From: David Laight <david.laight.linux@gmail.com>
To: Joel Granados <joel.granados@kernel.org>
Cc: Bradley Morgan <include@grrlz.net>, Kees Cook <kees@kernel.org>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] sysctl: drop the pointer dance in proc_put_char()
Date: Wed, 16 Sep 2026 13:47:00 +0100 [thread overview]
Message-ID: <20260916134700.3e0d17cd@pumpkin> (raw)
In-Reply-To: <adblkub2acrgjlgmjs2uu5644zm5fjghjzr5zkyga7uatrfeif@jsumbzlthyfb>
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
next prev parent reply other threads:[~2026-09-16 12:47 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-22 18:42 [PATCH] sysctl: drop the pointer dance in proc_put_char() Bradley Morgan
2026-09-16 11:46 ` Joel Granados
2026-09-16 12:47 ` David Laight [this message]
2026-09-25 13:16 ` Joel Granados
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260916134700.3e0d17cd@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=include@grrlz.net \
--cc=joel.granados@kernel.org \
--cc=kees@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.