[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] strncpy: best avoided (resend)
From: |
Jim Meyering |
Subject: |
[Qemu-devel] strncpy: best avoided (resend) |
Date: |
Wed, 9 May 2012 11:23:44 +0200 |
[Argh. First attempt omitted the most important address: qemu-devel.
Sorry to all who get two copies. ]
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
address@hidden
[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.
[PATCH 01/22] block: avoid buffer overrun by using pstrcpy, not
[PATCH 02/22] sheepdog: avoid a few buffer overruns
[PATCH 03/22] vmdk: relative_path: avoid buffer overrun
[PATCH 04/22] hw/9pfs: avoid buffer overrun
[PATCH 05/22] lm32: avoid buffer overrun
[PATCH 06/22] os-posix: avoid buffer overrun
[PATCH 07/22] ppc: avoid buffer overrun: use pstrcpy, not strncpy
[PATCH 08/22] linux-user: remove two unchecked uses of strdup
[PATCH 09/22] ui/vnc: simplify and avoid strncpy
[PATCH 10/22] bt: replace fragile snprintf use and unwarranted
[PATCH 11/22] virtio-9p: avoid unwarranted uses of strncpy
[PATCH 12/22] virtio-9p: avoid unwarranted use of strncpy
[PATCH 13/22] virtio-9p: avoid unwarranted use of strncpy
[PATCH 14/22] vscsi: avoid unwarranted strncpy
[PATCH 15/22] target-i386: use pstrcpy, not strncpy
[PATCH 16/22] qemu-ga: prefer pstrcpy: consistently NUL-terminate
[PATCH 17/22] libcacard/vcard_emul_nss: use pstrcpy in place of
[PATCH 18/22] acpi: remove strzcpy (strncpy-identical) function;
[PATCH 19/22] qcow2: mark this file's sole strncpy use as justified
[PATCH 20/22] hw/r2d: add comment: this strncpy use is ok
[PATCH 21/22] scsi: mark an strncpy use as valid
[PATCH 22/22] doc: update HACKING wrt strncpy/pstrcpy
- [Qemu-devel] strncpy: best avoided (resend),
Jim Meyering <=
- [Qemu-devel] [PATCH 09/22] ui/vnc: simplify and avoid strncpy, Jim Meyering, 2012/05/09
- [Qemu-devel] [PATCH 10/22] bt: replace fragile snprintf use and unwarranted strncpy, Jim Meyering, 2012/05/09
- [Qemu-devel] [PATCH 12/22] virtio-9p: avoid unwarranted use of strncpy, Jim Meyering, 2012/05/09
- [Qemu-devel] [PATCH 06/22] os-posix: avoid buffer overrun, Jim Meyering, 2012/05/09
- [Qemu-devel] [PATCH 01/22] block: avoid buffer overrun by using pstrcpy, not strncpy, Jim Meyering, 2012/05/09
- [Qemu-devel] [PATCH 18/22] acpi: remove strzcpy (strncpy-identical) function; just use strncpy, Jim Meyering, 2012/05/09