Re: [PATCH] xfsprogs: properly terminate string in quota's restore_file()

[Brian Foster <bfoster@redhat.com>]

On Tue, Aug 25, 2015 at 11:39:29AM -0500, Eric Sandeen wrote:
> This code copies up to the entire size of devbuffer, and then
> tries to use "strlen" to null terminate it.
> 
> But strlen works by *finding* the null, so it's at best a
> no-op, and at worst not properly terminating the string.
> 
> Fix this by placing the null at the last byte of the buffer.
> 
> Addresses-Coverity-Id: 1297519
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/quota/edit.c b/quota/edit.c
> index d226e89..a53a7e6 100644
> --- a/quota/edit.c
> +++ b/quota/edit.c
> @@ -385,7 +385,7 @@ restore_file(
>  	while (fgets(buffer, sizeof(buffer), fp) != NULL) {
>  		if (strncmp("fs = ", buffer, 5) == 0) {
>  			dev = strncpy(devbuffer, buffer+5, sizeof(devbuffer));
> -			dev[strlen(dev) - 1] = '\0';
> +			dev[sizeof(devbuffer) - 1] = '\0';

According to the man page, fgets() NULL terminates the provided buffer.
Next, we attempt to strncpy() just the device name part of the string
(copying up to 512 bytes from a 512-5 byte buffer). I'm not quite sure,
but it looks like the above line could be trying to replace a newline
with a NULL terminator..? E.g., it expects the last character in an
already NULL terminated line to be a newline.

Brian

>  			continue;
>  		}
>  		rtbsoft = rtbhard = 0;
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs