[Qemu-devel] [PATCHv2 00/22] strncpy: best avoided

[Jim Meyering]

From: Jim Meyering <address@hidden>

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).

I included a quick classification of these change sets for the original series,
(see https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg01151.html)
but note that a few have changed as the result of review feedback.

Jim Meyering (22):
  scsi,pci,qdev,isa-bus,sysbus: don't let *_get_fw_dev_path return NULL
  sparc: use g_strdup in place of unchecked strdup
  block: avoid buffer overrun by using pstrcpy, not strncpy
  sheepdog: avoid a few buffer overruns
  vmdk: relative_path: use pstrcpy in place of strncpy
  hw/9pfs: avoid buffer overrun
  lm32: avoid buffer overrun
  os-posix: avoid buffer overrun
  ppc: avoid buffer overrun: use pstrcpy, not strncpy
  linux-user: remove two unchecked uses of strdup
  ui/vnc: simplify and avoid strncpy
  bt: replace fragile snprintf use and unwarranted strncpy
  virtio-9p: avoid unwarranted uses of strncpy
  vscsi: avoid unwarranted strncpy
  target-i386: use pstrcpy, not strncpy
  qemu-ga: prefer pstrcpy: consistently NUL-terminate ifreq.ifr_name
  libcacard/vcard_emul_nss: use pstrcpy in place of strncpy
  acpi: remove strzcpy (strncpy-identical) function; just use strncpy
  qcow2: mark this file's sole strncpy use as justified
  hw/r2d: add comment: this strncpy use is ok
  scsi: mark an strncpy use as valid
  doc: update HACKING wrt strncpy/pstrcpy

 HACKING                        |  9 +++++----
 block.c                        |  5 +++--
 block/qcow2.c                  |  1 +
 block/sheepdog.c               | 34 ++++++++++++++++++++++------------
 block/vmdk.c                   |  3 +--
 hw/9pfs/virtio-9p-posix-acl.c  |  6 ++++--
 hw/9pfs/virtio-9p-synth.c      |  4 ++--
 hw/9pfs/virtio-9p-xattr-user.c |  3 ++-
 hw/9pfs/virtio-9p-xattr.c      |  3 ++-
 hw/acpi.c                      | 24 ++++++++----------------
 hw/bt-hci.c                    |  7 ++-----
 hw/ide/qdev.c                  |  2 +-
 hw/isa-bus.c                   |  2 +-
 hw/lm32_hwsetup.h              |  2 +-
 hw/pci.c                       |  2 +-
 hw/qdev.c                      |  2 +-
 hw/r2d.c                       |  2 ++
 hw/scsi-bus.c                  |  9 +++------
 hw/spapr_vscsi.c               |  2 +-
 hw/sysbus.c                    |  2 +-
 libcacard/Makefile             |  2 +-
 libcacard/vcard_emul_nss.c     |  3 +--
 linux-user/elfload.c           | 12 +++++++-----
 os-posix.c                     |  3 +--
 qga/commands-posix.c           |  2 +-
 target-i386/cpu.c              |  2 +-
 target-ppc/kvm.c               |  2 +-
 target-sparc/cpu.c             |  4 ++--
 ui/vnc-auth-sasl.c             |  4 +---
 29 files changed, 80 insertions(+), 78 deletions(-)

--
1.7.10.2.605.gbefc5ed