Re: [PATCH 36/40] NFSD: Prevent buffer overflow in write_threads()

["J. Bruce Fields" <bfields@fieldses.org>]

On Thu, Mar 12, 2009 at 12:11:30PM -0400, Chuck Lever wrote:
> Fix up write_threads() to eliminate the small possibility of
> overflowing its output buffer.

A gripe: could you be more specific about that "small possibility"?

Looks like the answer in this case is that the buffer overflow is
currently impossible, and that this is defensive programming against
someone making SIMPLE_TRANSACTION_LIMIT ridiculously small, or
something.  OK, but that should be stated very clearly in the commit
message.

(A lot of people may read a patch that begins "Prevent buffer overflow".
Let's be kind to them!  (A less alarming subject might also not be a bad
idea.))

Ditto for some of the other patches.

--b.

> 
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
> 
>  fs/nfsd/nfsctl.c |    9 +++++----
>  1 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index ce54d72..e8009c8 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -666,9 +666,9 @@ static ssize_t write_filehandle(struct file *file, char *buf, size_t size)
>  static ssize_t write_threads(struct file *file, char *buf, size_t size)
>  {
>  	char *mesg = buf;
> -	int rv;
> +
>  	if (size > 0) {
> -		int newthreads;
> +		int rv, newthreads;
>  		rv = get_int(&mesg, &newthreads);
>  		if (rv)
>  			return rv;
> @@ -678,8 +678,9 @@ static ssize_t write_threads(struct file *file, char *buf, size_t size)
>  		if (rv)
>  			return rv;
>  	}
> -	sprintf(buf, "%d\n", nfsd_nrthreads());
> -	return strlen(buf);
> +
> +	return scnprintf(buf, SIMPLE_TRANSACTION_LIMIT, "%d\n",
> +							nfsd_nrthreads());
>  }
>  
>  /**
>