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@xxxxxxxxxx>
> ---
>
> 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@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
|