[PATCH net-next] fs: nfsd: Fix buffer overflow in write_pool_threads()

[PATCH net-next] fs: nfsd: Fix buffer overflow in write_pool_threads()

[David Laight]

write_pool_threads() writes the number of threads in each pool into a
caller-supplied 'almost PAGE_SIZE' buffer.
If there are enough pools to overflow the buffer the code continues
writing beynd its end.

Fix the overflow check so that it actually works.

Fixes: eed2965af1bae "knfsd: allow admin to set nthreads per node"
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---

I'm pretty sure this is 'root only' code.
So you'd have to try very hard to actually get the overflow.

 fs/nfsd/nfsctl.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
index 39e7012a60d8..b74048aa2402 100644
--- a/fs/nfsd/nfsctl.c
+++ b/fs/nfsd/nfsctl.c
@@ -483,8 +483,7 @@ static ssize_t write_pool_threads(struct file *file, char *buf, size_t size)
 		 * file, sorry.  Report zero threads.
 		 */
 		mutex_unlock(&nfsd_mutex);
-		strcpy(buf, "0\n");
-		return strlen(buf);
+		return strscpy(buf, "0\n", SIMPLE_TRANSACTION_LIMIT);
 	}
 
 	nthreads = kzalloc_objs(int, npools);
@@ -523,13 +522,14 @@ static ssize_t write_pool_threads(struct file *file, char *buf, size_t size)
 
 	mesg = buf;
 	size = SIMPLE_TRANSACTION_LIMIT;
-	for (i = 0; i < npools && size > 0; i++) {
-		snprintf(mesg, size, "%d%c", nthreads[i], (i == npools-1 ? '\n' : ' '));
-		len = strlen(mesg);
+	for (i = 0; i < npools; i++) {
+		len = scnprintf(mesg, size, "%d ", nthreads[i]);
 		size -= len;
 		mesg += len;
 	}
 	rv = mesg - buf;
+	if (rv != SIMPLE_TRANSACTION_LIMIT - 1)
+		msg[-1] = '\n';
 out_free:
 	kfree(nthreads);
 	mutex_unlock(&nfsd_mutex);
-- 
2.39.5

Re: [PATCH net-next] fs: nfsd: Fix buffer overflow in write_pool_threads()

[Chuck Lever]

On Wed, Aug 12, 2026, at 3:33 PM, David Laight wrote:
> write_pool_threads() writes the number of threads in each pool into a
> caller-supplied 'almost PAGE_SIZE' buffer.
> If there are enough pools to overflow the buffer the code continues
> writing beynd its end.
>
> Fix the overflow check so that it actually works.
>
> Fixes: eed2965af1bae "knfsd: allow admin to set nthreads per node"
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
>
> I'm pretty sure this is 'root only' code.
> So you'd have to try very hard to actually get the overflow.
>
>  fs/nfsd/nfsctl.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> index 39e7012a60d8..b74048aa2402 100644
> --- a/fs/nfsd/nfsctl.c
> +++ b/fs/nfsd/nfsctl.c
> @@ -483,8 +483,7 @@ static ssize_t write_pool_threads(struct file 
> *file, char *buf, size_t size)
>  		 * file, sorry.  Report zero threads.
>  		 */
>  		mutex_unlock(&nfsd_mutex);
> -		strcpy(buf, "0\n");
> -		return strlen(buf);
> +		return strscpy(buf, "0\n", SIMPLE_TRANSACTION_LIMIT);
>  	}
> 
>  	nthreads = kzalloc_objs(int, npools);
> @@ -523,13 +522,14 @@ static ssize_t write_pool_threads(struct file 
> *file, char *buf, size_t size)
> 
>  	mesg = buf;
>  	size = SIMPLE_TRANSACTION_LIMIT;
> -	for (i = 0; i < npools && size > 0; i++) {
> -		snprintf(mesg, size, "%d%c", nthreads[i], (i == npools-1 ? '\n' : ' '));
> -		len = strlen(mesg);
> +	for (i = 0; i < npools; i++) {
> +		len = scnprintf(mesg, size, "%d ", nthreads[i]);
>  		size -= len;
>  		mesg += len;
>  	}
>  	rv = mesg - buf;
> +	if (rv != SIMPLE_TRANSACTION_LIMIT - 1)
> +		msg[-1] = '\n';

Did you mean "mesg[-1] = '\n';" here?


>  out_free:
>  	kfree(nthreads);
>  	mutex_unlock(&nfsd_mutex);
> -- 
> 2.39.5

-- 
Chuck Lever

Re: [PATCH net-next] fs: nfsd: Fix buffer overflow in write_pool_threads()

[David Laight]

On Wed, 12 Aug 2026 16:19:22 -0400
"Chuck Lever" <cel@kernel.org> wrote:

> On Wed, Aug 12, 2026, at 3:33 PM, David Laight wrote:
> > write_pool_threads() writes the number of threads in each pool into a
> > caller-supplied 'almost PAGE_SIZE' buffer.
> > If there are enough pools to overflow the buffer the code continues
> > writing beynd its end.
> >
> > Fix the overflow check so that it actually works.
> >
> > Fixes: eed2965af1bae "knfsd: allow admin to set nthreads per node"
> > Signed-off-by: David Laight <david.laight.linux@gmail.com>
> > ---
> >
> > I'm pretty sure this is 'root only' code.
> > So you'd have to try very hard to actually get the overflow.
> >
> >  fs/nfsd/nfsctl.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/nfsd/nfsctl.c b/fs/nfsd/nfsctl.c
> > index 39e7012a60d8..b74048aa2402 100644
> > --- a/fs/nfsd/nfsctl.c
> > +++ b/fs/nfsd/nfsctl.c
> > @@ -483,8 +483,7 @@ static ssize_t write_pool_threads(struct file 
> > *file, char *buf, size_t size)
> >  		 * file, sorry.  Report zero threads.
> >  		 */
> >  		mutex_unlock(&nfsd_mutex);
> > -		strcpy(buf, "0\n");
> > -		return strlen(buf);
> > +		return strscpy(buf, "0\n", SIMPLE_TRANSACTION_LIMIT);
> >  	}
> > 
> >  	nthreads = kzalloc_objs(int, npools);
> > @@ -523,13 +522,14 @@ static ssize_t write_pool_threads(struct file 
> > *file, char *buf, size_t size)
> > 
> >  	mesg = buf;
> >  	size = SIMPLE_TRANSACTION_LIMIT;
> > -	for (i = 0; i < npools && size > 0; i++) {
> > -		snprintf(mesg, size, "%d%c", nthreads[i], (i == npools-1 ? '\n' : ' '));
> > -		len = strlen(mesg);
> > +	for (i = 0; i < npools; i++) {
> > +		len = scnprintf(mesg, size, "%d ", nthreads[i]);
> >  		size -= len;
> >  		mesg += len;
> >  	}
> >  	rv = mesg - buf;
> > +	if (rv != SIMPLE_TRANSACTION_LIMIT - 1)
> > +		msg[-1] = '\n';  
> 
> Did you mean "mesg[-1] = '\n';" here?

Yes - and I thought I'd compiled it ...
I did decide not to worry about the missing '\n' when the output 'just fits'.
After all you need over 1300 pools with 10 threads to get to 4k.
I'd bet something else dies first.

	David

(Oh I've replaced Linus's 20 year old email I copied from the commit with
his current one (he acked it) and deleted the broken one from the author.)


> 
> 
> >  out_free:
> >  	kfree(nthreads);
> >  	mutex_unlock(&nfsd_mutex);
> > -- 
> > 2.39.5  
>