[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

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


[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

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]