On Thu, 2017-01-12 at 18:16 -0800, Nicholas Miell wrote:
> From 714d07f600db39498c87d7816f4dd3a7e6d9bbca Mon Sep 17 00:00:00 2001
> From: Nicholas Miell <nmiell@xxxxxxxxx>
> Date: Thu, 12 Jan 2017 15:43:07 -0800
> Subject: [PATCH libdrm] xf86drm: fix valgrind warning in drmParsePciBusInfo
>
> The current implementation reads (up to) 513 bytes, overwrites the 513th
> byte with '\0' and then passes the buffer off to strstr() and sscanf()
> without ever initializing the middle bytes. This causes valgrind
> warnings and potentially fails to parse PCI_SLOT_NAME if the uevent is
> unexpectedly large.
a simpler fix should also get rid of the valgrind warning:
- ret = read(fd, data, sizeof(data));
- data[sizeof(data)-1] = '\0';
+ ret = read(fd, data, sizeof(data) - 1);
close(fd);
if (ret < 0)
return -errno
+ data[ret] = '\0';
I think that dynamic memory allocation is still a more robust approach.
regards,
Jan
>
> Rewrite it using getline() to fix the valgrind errors and future-proof
> the function against uevent files larger than 513 bytes.
>
> Signed-off-by: Nicholas Miell <nmiell@xxxxxxxxx>
> ---
> xf86drm.c | 34 +++++++++++++++++-----------------
> 1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index b8b2cfe..33261ac 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2919,31 +2919,31 @@ static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
> {
> #ifdef __linux__
> char path[PATH_MAX + 1];
> - char data[512 + 1];
> - char *str;
> + FILE *uevent;
> + char *line = NULL;
> + size_t n = 0;
> + bool found = false;
> int domain, bus, dev, func;
> - int fd, ret;
>
> snprintf(path, PATH_MAX, "/sys/dev/char/%d:%d/device/uevent", maj, min);
> - fd = open(path, O_RDONLY);
> - if (fd < 0)
> + uevent = fopen(path, "r");
> + if (uevent == NULL)
> return -errno;
>
> - ret = read(fd, data, sizeof(data));
> - data[sizeof(data)-1] = '\0';
> - close(fd);
> - if (ret < 0)
> - return -errno;
> + while (getline(&line, &n, uevent) != -1) {
> + if (sscanf(line, "PCI_SLOT_NAME=%04x:%02x:%02x.%1u",
> + &domain, &bus, &dev, &func) == 4)
> + {
> + found = true;
> + break;
> + }
> + }
> + free(line);
>
> -#define TAG "PCI_SLOT_NAME="
> - str = strstr(data, TAG);
> - if (str == NULL)
> - return -EINVAL;
> + fclose(uevent);
>
> - if (sscanf(str, TAG "%04x:%02x:%02x.%1u",
> - &domain, &bus, &dev, &func) != 4)
> + if (!found)
> return -EINVAL;
> -#undef TAG
>
> info->domain = domain;
> info->bus = bus;
--
Jan Vesely <jan.vesely@xxxxxxxxxxx>Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
