Re: [PATCH v3] sysctl: fix check against uninitialized variable in proc_do_large_bitmap

[Peter Seiderer <ps.report@gmx.net>]

Hello Marc, Kees,

On Fri, 20 Mar 2026 11:30:29 -0700, Kees Cook <kees@kernel.org> wrote:

> On Thu, Mar 19, 2026 at 11:50:50PM +0100, Marc Buerg wrote:
> > proc_do_large_bitmap() does not initialize variable c, which is expected
> > to be set to a trailing character by proc_get_long().
> > 
> > However, proc_get_long() only sets c when the input buffer contains a
> > trailing character after the parsed value.
> > 
> > If c is not initialized it may happen to contain a '-'. If this is the
> > case proc_do_large_bitmap() expects to be able to parse a second part of
> > the input buffer. If there is no second part an unjustified -EINVAL will
> > be returned.
> > 
> > Add check that left is non-zero before checking c, as proc_get_long()
> > ensures that the passed left is non-zero, if a trailing character
> > exists.
> > 
> > ---  
> 
> Please don't include "---" as a separator here: we want to keep your
> entire commit log (including the SoB and other tags).

+1 for this one...

> 
> Also, I think the explicit zero-init of 'c' would be nice to keep just
> for robustness: the compiler can elide it if it decides it's a duplicate
> store. There's only upside to including it.

Sorry, disagree on this one, not a fan of this 'add unneeded code just in case...'
pattern hiding the real fix and/or logic of the code, but just the
opinion of a sporadic contributor ;-)

@Marc: in case you go this way, please remove my Reviewed-by tag on next
patch iteration...

Regards,
Peter

> 
> -Kees
> 
> > When writing to /proc/sys/net/ipv4/ip_local_reserved_ports it is
> > possible to receive an -EINVAL for a valid value.
> > 
> > This happens due to a check of a potentially uninitialized variable in
> > the proc_do_large_bitmap() function, namely char c. To trigger this
> > behavior the variable has to contain the later explicitly checked '-'
> > char by chance.
> > 
> > In proc_do_large_bitmap() it is expected that the variable might be
> > filled by the proc_get_long() function with the trailing character of
> > the given input. But only if a trailing character exists within the
> > passed size of the buffer.
> > 
> > If no trailing character is present we still do a c == '-' check. If the
> > uninitialized variable contains this char the function continues
> > parsing. It will now set err to -EINVAL in the next proc_get_long()
> > call, as there is nothing more to parse.
> > 
> > proc_do_large_bitmap() passes left to the proc_get_long() call. left
> > will only be non-zero, if a trailing character has been written.
> > Therefore, checking that left is non-zero before accessing c fixes this
> > problem.
> > 
> > The problem will only arise sporadically, as the variable must contain
> > '-' by chance. On the affected system CONFIG_INIT_STACK_NONE=y was
> > enabled. Further, when enabling eBPF tracing to dump contents of the
> > stack the issue disappeared.
> > 
> > Fixes: 9f977fb7ae9d ("sysctl: add proc_do_large_bitmap")
> > Signed-off-by: Marc Buerg <buermarc@googlemail.com>
> > Reviewed-by: Peter Seiderer <ps.report@gmx.net>
> > ---
> > Changes in v3:
> > - Add Reviewed-by: Peter Seiderer <ps.report@gmx.net>
> > - Re-include bug context into cover letter
> > - Link to v2: https://lore.kernel.org/r/20260317-fix-uninitialized-variable-in-proc_do_large_bitmap-v2-1-6dfb1aefa287@googlemail.com
> > 
> > Changes in v2:
> > - Drop initialization of c to 0
> > - Include checking that left is non-zero before checking against c
> > - Link to v1: https://lore.kernel.org/r/20260312-fix-uninitialized-variable-in-proc_do_large_bitmap-v1-1-35ad2dddaf21@googlemail.com
> > ---
> >  kernel/sysctl.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index 9d3a666ffde1..dd337a63da41 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -1171,7 +1171,7 @@ int proc_do_large_bitmap(const struct ctl_table *table, int dir,
> >  				left--;
> >  			}
> >  
> > -			if (c == '-') {
> > +			if (left && c == '-') {
> >  				err = proc_get_long(&p, &left, &val_b,
> >  						     &neg, tr_b, sizeof(tr_b),
> >  						     &c);
> > 
> > ---
> > base-commit: 80234b5ab240f52fa45d201e899e207b9265ef91
> > change-id: 20260312-fix-uninitialized-variable-in-proc_do_large_bitmap-30c6ef4ac1c5
> > 
> > Best regards,
> > -- 
> > buermarc <buermarc@googlemail.com>
> >   
>