Re: [PATCH] libhandle: fix potential unterminated string problem

[Eric Sandeen <sandeen@sandeen.net>]

On 10/7/20 10:58 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> gcc 10.2 complains about the strncpy call here, since it's possible that
> the source string is so long that the fspath inside the fdhash structure
> will end up without a null terminator.  Work around strncpy braindamage
> yet again by forcing the string to be terminated properly.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> Unless this is supposed to be a memcpy?  But it doesn't look like it.

so FWIW, coverity has complained about this one forever, CID 1297520

I think the thing that kept me from addressing it before was uncertainty
around lopping one byte off of PATH_MAX / MAXPATHLEN, essentially.

In reality, I think (?) PATH_MAX is a bit of a fiction anyway, and
there's probably nothing special about that one byte, and I think (?)
this is always a path to a mountpoint (?) which is unlikely to be 4096
or 4095 chars long anyway (?) so ... um ... sure.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  libhandle/handle.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libhandle/handle.c b/libhandle/handle.c
> index eb099f43791e..5c1686b3968d 100644
> --- a/libhandle/handle.c
> +++ b/libhandle/handle.c
> @@ -107,7 +107,8 @@ path_to_fshandle(
>  		}
>  
>  		fdhp->fsfd = fd;
> -		strncpy(fdhp->fspath, fspath, sizeof(fdhp->fspath));
> +		strncpy(fdhp->fspath, fspath, sizeof(fdhp->fspath) - 1);
> +		fdhp->fspath[sizeof(fdhp->fspath) - 1] = 0;
>  		memcpy(fdhp->fsh, *fshanp, FSIDSIZE);
>  
>  		fdhp->fnxt = fdhash_head;
>