[libvirt] [PATCH V1 5/5] node_device: fix possible non-terminated string

[Eric Blake]

On 05/04/2012 05:54 AM, Stefan Berger wrote:
> Error: STRING_NULL:
> /libvirt/src/node_device/node_device_linux_sysfs.c:80:
> string_null_argument: Function "saferead" does not terminate string "*buf".
> /libvirt/src/util/util.c:101:
> string_null_argument: Function "read" fills array "*buf" with a non-terminated string.
> /libvirt/src/node_device/node_device_linux_sysfs.c:87:
> string_null: Passing unterminated string "buf" to a function expecting a null-terminated string.
> 
> ---
>  src/node_device/node_device_linux_sysfs.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> Index: libvirt-acl/src/node_device/node_device_linux_sysfs.c
> ===================================================================
> --- libvirt-acl.orig/src/node_device/node_device_linux_sysfs.c
> +++ libvirt-acl/src/node_device/node_device_linux_sysfs.c
> @@ -69,20 +69,21 @@ out:
>  int read_wwn_linux(int host, const char *file, char **wwn)
>  {
>      char *p = NULL;
> -    int fd = -1, retval = 0;
> -    char buf[64];
> +    int fd = -1, retval = 0, len;
> +    char buf[65];

Here, I would write:

char buf[65] = "";

>  
>      if (open_wwn_file(LINUX_SYSFS_FC_HOST_PREFIX, host, file, &fd) < 0) {
>          goto out;
>      }
>  
> -    memset(buf, 0, sizeof(buf));

Then the memset is not necessary (the initialization took care of that
instead).

> -    if (saferead(fd, buf, sizeof(buf)) < 0) {
> +    len = saferead(fd, buf, sizeof(buf) - 1);

You are correct that you need to use sizeof(buf) - 1.  But if you
guarantee that buf was all NUL on entry, then you don't have to worry
about the resulting len,...

> +    if (len < 0) {
>          retval = -1;
>          VIR_DEBUG("Failed to read WWN for host%d '%s'",
>                    host, file);
>          goto out;
>      }
> +    buf[len] = '\0';

and therefore don't need this trailing assignment.  We definitely have
an off-by-one bug here, but I don't think we need quite as many changed
to fix the issue as what you have here.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120504/77aa2957/attachment-0001.sig>