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

[Dan Williams <dcbw redhat com>]

On Fri, 2011-10-14 at 13:17 +0200, Thomas Jarosch wrote:
> Patch is not compile tested as I lack some
> dependencies to build NM on this box.
> 
> Signed-off-by: Thomas Jarosch <thomas jarosch intra2net com>
> ---
>  src/nm-device-ethernet.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/src/nm-device-ethernet.c b/src/nm-device-ethernet.c
> index 8556c5b..87cd57e 100644
> --- 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?  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?

Dan

>  		errno = 0;
> -		if (readlink (cdev_path, &buf[0], sizeof (buf) - 1) >= 0) {
> +		if ((link_len = readlink (cdev_path, buf, sizeof (buf) - 1)) >= 0) {
> +			buf[link_len] = 0;
> +
>  			if (!priv->subchan1)
>  				priv->subchan1 = g_path_get_basename (buf);
>  			else if (!priv->subchan2)