[PATCH libdrm] xf86drm: fix valgrind warning in drmParsePciBusInfo

[Emil Velikov]

On 13 January 2017 at 21:11, Nicholas Miell <nmiell at gmail.com> wrote:
> 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.
>
Stopped working = it never worked since it was introduced (initially
in mesa) circa 2014 ;-)

That aside, Thierry has some helper(s) which we can reuse here.

>>> 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.
>
I'm well aware of that, it was me who added the guard or reviewed &
pushed the commit ;-)

As you may be aware other platforms also have sysfs - FreeBSD (and
derivatives?), GNU Hurd and perhaps others. Things are kept Linux only
since almost (nobody) running !Linux platform has bothered looking in
libdrm for a long time. And it's not like I haven't poked people on a
number of occasions :-P

Emil