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

[Joel Granados <joel.granados@kernel.org>]

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

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().
Ack.

> 
> However, proc_get_long() only sets c when the input buffer contains a
> trailing character after the parsed value.
Ack.

> 
> If c is not initialized it may happen to contain a '-'. If this is the
Here I have a question:

How is it possible that the buffer in proc_do_large_bitmap is not
terminated with a '\0' when writing?

The proc_do_large_bitmap gets called from proc_sys_call_handler and
before running the table->proc_handler call, the end of the buffer gets
set with '\0'. This means that when the last value is parsed, the
trailing char is '\0'. Which, in turn, means that 'c' will always get a
value.

*Unless*, the buffer is changed in BPF_CGROUP_RUN_PROG_SYSCTL by an
external (to the kernel) program.

Are you running a BPF program on this hook? Do you see the same behavior
if you turn off BPF?

> 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.
> 
> ---
I'll repeat the comments from the other reviewers. Careful with these
separators. When I used b4 to bring all this into my testing env, All
your trailers were ignored.

best

> 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>
> 

-- 

Joel Granados

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