[libvirt] [PATCH] node_device_driver.c: don't write beyond EOB for 4K-byte symlink

[Jim Meyering]

Chris Lalancette wrote:

> On 12/14/2009 02:48 PM, Jim Meyering wrote:
>> From: Jim Meyering <meyering at redhat.com>
>> Date: Mon, 14 Dec 2009 12:05:38 +0100
>> Subject: [PATCH] node_device_driver.c: don't write beyond EOB for 4K-byte symlink
>>
>> * src/node_device/node_device_driver.c (update_driver_name): The
>> previous code would write one byte beyond the end of the 4KiB
>> stack buffer when presented with a symlink value of exactly that
>> length (very unlikely).  Remove the automatic buffer and use
>> virFileResolveLink in place of readlink.  Suggested by Daniel Veillard.
>> ---
>>  src/node_device/node_device_driver.c |    7 +++----
>>  1 files changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c
>> index f083f16..ecbac0f 100644
>> --- a/src/node_device/node_device_driver.c
>> +++ b/src/node_device/node_device_driver.c
>> @@ -78,10 +78,9 @@ static int update_driver_name(virConnectPtr conn,
>>                                virNodeDeviceObjPtr dev)
>>  {
>>      char *driver_link = NULL;
>> -    char devpath[PATH_MAX];
>> +    char *devpath;
>>      char *p;
>>      int ret = -1;
>> -    int n;
>>
>>      VIR_FREE(dev->def->driver);
>>
>> @@ -97,12 +96,11 @@ static int update_driver_name(virConnectPtr conn,
>>          goto cleanup;
>>      }
>>
>> -    if ((n = readlink(driver_link, devpath, sizeof devpath)) < 0) {
>> +    if (virFileResolveLink(driver_link, &devpath) < 0) {
>
> FYI; I found out today that virFileResolveLink() doesn't work on filesystems
> backed by sysfs (which unfortunately this code path is).  The problem is that
> sysfs doesn't follow the POSIX-specified behavior of placing the size
> of the name of the real path in st.st_size; instead, on sysfs, st.st_size for
> symlinks is *always* 0 (at least on my F-12 box here).  So this code path is
> probably broken now.  DV said he will take a look at putting a patch together
> to make virFileResolveLink() handle the 0 case.

Good point.  Don't bother rolling your own.
I'll prepare a patch to use gnulib's areadlink module.