[Hmm... This was supposed to be the first in the series] Given qemu's HACKING comments, I'm sure many here have read "man strncpy", where it indicates it is often not the best function to use.
However, many of the uses of strncpy in qemu mistakenly fail to ensure that the destination buffer is NUL-terminated. The first 7 c-sets fix a dozen or so buffer overrun errors due to the lack of NUL-termination in buffers that are later used in a context that requires the terminating NUL. I audited all of the strndup uses in qemu and have replaced many with uses of qemu's pstrcpy function (it guarantees NUL-termination and does not zero-fill). A few are easily/cleanly replaced by uses of memcpy, and for the few remaining uses that are justified, I added comments marking the use as justified, explaining that it's ok because uses of the destination buffer (currently) do not require NUL-termination. But see the note[0] below. Some of these changes definitely count as trivial, while others look trivial but required that I look into kernel sources to confirm that NUL-termination is ok, but not required (e.g., for the SIOCGIFHWADDR ioctl's ifr.ifr_name input: linux clears its last byte, up front). Here's a Q&D classification of these change sets: overrun block.c several overruns block/sheepdog.c overrun block/vmdk.c 2 overruns hw/9pfs/virtio-9p-synth.c overrun hw/lm32_hwsetup.h overrun os-posix.c overrun target-ppc/kvm.c 2-unchecked-strdup+comment linux-user/elfload.c simplify/avoid strncpy ui/vnc-auth-sasl.c snprintf+pstrcpy fragile hw/bt-hci.c 2 x s/strncpy/memcpy/ hw/9pfs/virtio-9p-posix-acl.c s/strncpy/memcpy/ hw/9pfs/virtio-9p-xattr-user.c s/strncpy/memcpy/ hw/9pfs/virtio-9p-xattr.c s/strncpy/memcpy/ hw/spapr_vscsi.c s/strncpy/pstrcpy/ +Makefile libcacard/vcard_emul_nss.c s/strncpy/pstrcpy qga/commands-posix.c s/strncpy/pstrcpy target-i386/cpu.c remove strzcpy definition hw/acpi.c comment block/qcow2.c comment hw/r2d.c comment hw/scsi-bus.c comment HACKING qemu-triv...@nongnu.org [0] I have mixed feelings about marking with comments the remaining, justifiable uses of strncpy. Note: I didn't finish that job. On one hand, it's good to document what I've done, presumably saving others the work of auditing those uses. On the other, it's ugly and not very maintainable. After writing the few comment-adding patches that merely say "this strncpy use is justified...", I realized it might be better to use a different symbol to indicate when one really does intend to use strncpy and realizes the implications. That would make it trivial to prohibit (mechanically) any new direct use of "strncpy". Then all "safe/intended" uses would use the new symbol, say strNcpy or STRNCPY (with a comment explaining the issues near the definition of the macro or static inline function), and we wouldn't have to pollute the code with comments marking each use. If there's interest (or no objection) I'll do that separately.
