Re: [Qemu-devel] [PATCH 2/3] Disk serial number string copy function: change strncpy() to pstrcpy()

[Markus Armbruster]

Floris Bos <address@hidden> writes:

> Cc: address@hidden
> Signed-off-by: Floris Bos <address@hidden>
> ---
>  blockdev.c    |    5 +++--
>  hw/ide/core.c |    2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/blockdev.c b/blockdev.c
> index d78aa51..e52449e 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -532,8 +532,9 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
>      dinfo->unit = unit_id;
>      dinfo->opts = opts;
>      dinfo->refcount = 1;
> -    if (serial)
> -        strncpy(dinfo->serial, serial, sizeof(dinfo->serial) - 1);
> +    if (serial) {
> +        pstrcpy(dinfo->serial, sizeof(dinfo->serial), serial);
> +    }
>      QTAILQ_INSERT_TAIL(&drives, dinfo, next);
>  
>      bdrv_set_on_error(dinfo->bdrv, on_read_error, on_write_error);

The old code works because dinfo->serial[]'s last byte is set to zero by
g_malloc0().  The new code achieves the same result in a less subtle
manner.

> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index c9a0e24..3e50c52 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -1880,7 +1880,7 @@ int ide_init_drive(IDEState *s, BlockDriverState *bs, 
> IDEDriveKind kind,
>          }
>      }
>      if (serial) {
> -        strncpy(s->drive_serial_str, serial, sizeof(s->drive_serial_str));
> +        pstrcpy(s->drive_serial_str, sizeof(s->drive_serial_str), serial);
>      } else {
>          snprintf(s->drive_serial_str, sizeof(s->drive_serial_str),
>                   "QM%05d", s->drive_serial);

This actually fixes a latent bug.  The old code fails to zero-terminate
s->drive_serial_str when serial is longer than 21 characters.
ide_identify(), ide_atapi_identify() and ide_cfata_identify() don't
care, but ide_dev_initfn() passes it to g_strdup().  Fortunately, we
reach that only when !dev->serial, and then the value copied into
s->drive_serial_str[] comes from DriveInfo member serial[], which has
space for just 20 characters plus terminating zero.

Thanks!

Two down, 34 uses of strncpy() left in the code.