[PATCH] sysctl: drop the pointer dance in proc_put_char()

[PATCH] sysctl: drop the pointer dance in proc_put_char()

[Bradley Morgan]

proc_put_char() still drags around a char **buffer alias, writing
the char through it, advancing it, then copying it back into the
slot it was loaded from. That only made sense when the buffer was
__user and the char went through put_user() (which could fail).

Since commit 32927393dc1c ("sysctl: pass kernel pointers to
->proc_handler") the buffer is just a kernel pointer, so the alias
is dead weight. proc_put_long() and the skip helpers already advance
*buf directly, so do the same here. No functional change.

Signed-off-by: Bradley Morgan <include@grrlz.net>
---
 kernel/sysctl.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index f7b7598..2b92b30 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -350,12 +350,9 @@ static void proc_put_long(void **buf, size_t *size, unsigned long val, bool neg)
 static void proc_put_char(void **buf, size_t *size, char c)
 {
 	if (*size) {
-		char **buffer = (char **)buf;
-		**buffer = c;
-
+		*(char *)*buf = c;
 		(*size)--;
-		(*buffer)++;
-		*buf = *buffer;
+		(*buf)++;
 	}
 }
 
-- 
2.47.3

Re: [PATCH] sysctl: drop the pointer dance in proc_put_char()

[Joel Granados]

[-- Attachment #1: Type: text/plain, Size: 1426 bytes --]

On Sat, Aug 22, 2026 at 06:42:44PM +0000, Bradley Morgan wrote:
> proc_put_char() still drags around a char **buffer alias, writing
> the char through it, advancing it, then copying it back into the
> slot it was loaded from. That only made sense when the buffer was
> __user and the char went through put_user() (which could fail).
> 
> Since commit 32927393dc1c ("sysctl: pass kernel pointers to
> ->proc_handler") the buffer is just a kernel pointer, so the alias
> is dead weight. proc_put_long() and the skip helpers already advance
> *buf directly, so do the same here. No functional change.
> 
> Signed-off-by: Bradley Morgan <include@grrlz.net>
> ---
>  kernel/sysctl.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index f7b7598..2b92b30 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -350,12 +350,9 @@ static void proc_put_long(void **buf, size_t *size, unsigned long val, bool neg)
>  static void proc_put_char(void **buf, size_t *size, char c)
>  {
>  	if (*size) {
> -		char **buffer = (char **)buf;
> -		**buffer = c;
> -
> +		*(char *)*buf = c;
>  		(*size)--;
> -		(*buffer)++;
> -		*buf = *buffer;
> +		(*buf)++;
>  	}
>  }
>  
> -- 
> 2.47.3
> 

I was thinking more along these lines https://lore.kernel.org/all/20260916-lklm-sysctl-void-vs-char-v1-1-c3f8b2eb8b1e@kernel.org/

Best

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] sysctl: drop the pointer dance in proc_put_char()

[David Laight]

On Wed, 16 Sep 2026 13:46:40 +0200
Joel Granados <joel.granados@kernel.org> wrote:

> On Sat, Aug 22, 2026 at 06:42:44PM +0000, Bradley Morgan wrote:
> > proc_put_char() still drags around a char **buffer alias, writing
> > the char through it, advancing it, then copying it back into the
> > slot it was loaded from. That only made sense when the buffer was
> > __user and the char went through put_user() (which could fail).
> > 
> > Since commit 32927393dc1c ("sysctl: pass kernel pointers to  
> > ->proc_handler") the buffer is just a kernel pointer, so the alias  
> > is dead weight. proc_put_long() and the skip helpers already advance
> > *buf directly, so do the same here. No functional change.
> > 
> > Signed-off-by: Bradley Morgan <include@grrlz.net>
> > ---
> >  kernel/sysctl.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> > 
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index f7b7598..2b92b30 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -350,12 +350,9 @@ static void proc_put_long(void **buf, size_t *size, unsigned long val, bool neg)
> >  static void proc_put_char(void **buf, size_t *size, char c)
> >  {
> >  	if (*size) {
> > -		char **buffer = (char **)buf;
> > -		**buffer = c;
> > -
> > +		*(char *)*buf = c;
> >  		(*size)--;
> > -		(*buffer)++;
> > -		*buf = *buffer;
> > +		(*buf)++;
> >  	}
> >  }
> >  
> > -- 
> > 2.47.3
> >   
> 
> I was thinking more along these lines https://lore.kernel.org/all/20260916-lklm-sysctl-void-vs-char-v1-1-c3f8b2eb8b1e@kernel.org/

I think I'd decrement *size before writing to buf.
Should ensure the compiler never reloads *size.
Could also be worth an __always_inline - likely to make the code smaller.

David

> 
> Best