[PATCH libdrm] xf86drm: fix valgrind warning in drmParsePciBusInfo

Nicholas Miell nmiell at gmail.com
Fri Jan 13 21:11:45 UTC 2017


On 01/13/2017 09:57 AM, Emil Velikov wrote:
> On 13 January 2017 at 11:34, Jan Vesely <jan.vesely at rutgers.edu> wrote:
>> 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 at gmail.com>
>>> 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';
>>
> We had this (better imho) patch a week or so ago. In either case the
> issue is virtually since (iirc) if the string is malformed we'll bail
> out either way.

Simpler, but potentially stops working in the future. It already stopped 
working once.

>> I think that dynamic memory allocation is still a more robust approach.
>>
> Yes that might be the better solution, or one could even use
> getline(). The latter might be pushing it's only POSIX 2008.

POSIX isn't relevant, this is a Linux-specific function.



More information about the dri-devel mailing list