Re: [PATCH] Fix unterminated buffer after readlink() call

[Thomas Jarosch <thomas jarosch intra2net com>]

Hi Dan,

On 10/14/2011 05:14 PM, Dan Williams wrote:
>> --- a/src/nm-device-ethernet.c
>> +++ b/src/nm-device-ethernet.c
>> @@ -343,6 +343,7 @@ _update_s390_subchannels (NMDeviceEthernet *self)
>>  	while ((item = g_dir_read_name (dir))) {
>>  		char buf[50];
>>  		char *cdev_path;
>> +		ssize_t link_len;
>>  
>>  		if (strncmp (item, "cdev", 4))
>>  			continue;  /* Not a subchannel link */
>> @@ -351,7 +352,9 @@ _update_s390_subchannels (NMDeviceEthernet *self)
>>  
>>  		memset (buf, 0, sizeof (buf));
> 
> My initial read is that the memset would terminate the buffer, since
> we're passing sizeof (buf) - 1 into the call, the last byte of the
> buffer will always be 0, no?  

Good catch, I missed the memset() call.

And if readlink() doesn't fill the buffer,
> the remaining bytes will already be set to 0 by the memset, AFAICT.  Are
> you seeing a crash here or was this from visual inspection?

Pure visual inspection, I was looking for readlink() usage patterns.

Still the buf[link_len] = '0'; is more explicit and we wouldn't need
the memset() call at all. What do you think?

Cheers,
Thomas