[PATCH] sysctl: fix uninitialized variable in proc_do_large_bitmap

[PATCH] sysctl: fix uninitialized variable in proc_do_large_bitmap

[Marc Buerg]

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.

Initialize c to 0 to prevent returning -EINVAL on valid input.

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

The proc_get_long() function can set c if the length of the parsed long
is smaller than the given size of the buffer containing the user input.
This is not the case if the buffer only contains the port value (e.g.
"123") and sets the size exactly to that (3). Meaning if there is no
trailing character, c will not be set.

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.

Initializing c to 0 will solve the 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 disappears, which would fit the current explanation as a
root cause for the observed behavior.

Fixes: 9f977fb7ae9d ("sysctl: add proc_do_large_bitmap")
Signed-off-by: Marc Buerg <buermarc@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..c9efb17cc255 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1118,7 +1118,7 @@ int proc_do_large_bitmap(const struct ctl_table *table, int dir,
 	unsigned long bitmap_len = table->maxlen;
 	unsigned long *bitmap = *(unsigned long **) table->data;
 	unsigned long *tmp_bitmap = NULL;
-	char tr_a[] = { '-', ',', '\n' }, tr_b[] = { ',', '\n', 0 }, c;
+	char tr_a[] = { '-', ',', '\n' }, tr_b[] = { ',', '\n', 0 }, c = 0;
 
 	if (!bitmap || !bitmap_len || !left || (*ppos && SYSCTL_KERN_TO_USER(dir))) {
 		*lenp = 0;

---
base-commit: 80234b5ab240f52fa45d201e899e207b9265ef91
change-id: 20260312-fix-uninitialized-variable-in-proc_do_large_bitmap-30c6ef4ac1c5

Best regards,
-- 
Marc Buerg <buermarc@googlemail.com>

Re: [PATCH] sysctl: fix uninitialized variable in proc_do_large_bitmap

[Peter Seiderer]

Hello Marc,

On Thu, 12 Mar 2026 16:42:19 +0100, Marc Buerg <buermarc@googlemail.com> 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.
> 
> Initialize c to 0 to prevent returning -EINVAL on valid input.
> 
> ---
> 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 an 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.
> 
> The proc_get_long() function can set c if the length of the parsed long
> is smaller than the given size of the buffer containing the user input.
> This is not the case if the buffer only contains the port value (e.g.
> "123") and sets the size exactly to that (3). Meaning if there is no
> trailing character, c will not be set.
> 
> 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.
> 
> Initializing c to 0 will solve the 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 disappears, which would fit the current explanation as a
> root cause for the observed behavior.
> 
> Fixes: 9f977fb7ae9d ("sysctl: add proc_do_large_bitmap")
> Signed-off-by: Marc Buerg <buermarc@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..c9efb17cc255 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1118,7 +1118,7 @@ int proc_do_large_bitmap(const struct ctl_table *table, int dir,
>  	unsigned long bitmap_len = table->maxlen;
>  	unsigned long *bitmap = *(unsigned long **) table->data;
>  	unsigned long *tmp_bitmap = NULL;
> -	char tr_a[] = { '-', ',', '\n' }, tr_b[] = { ',', '\n', 0 }, c;
> +	char tr_a[] = { '-', ',', '\n' }, tr_b[] = { ',', '\n', 0 }, c = 0;
>  
>  	if (!bitmap || !bitmap_len || !left || (*ppos && SYSCTL_KERN_TO_USER(dir))) {
>  		*lenp = 0;
> 

Given the description of proc_get_long() regarding @tr:

   * @tr: pointer to store the trailer character
   *
   * In case of success %0 is returned and @buf and @size are updated with
   * the amount of bytes read. If @tr is non-NULL and a trailing
   * character exists (size is non-zero after returning from this
   * function), @tr is updated with the trailing character.

Would the better fix be:

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 354a2d294f52..89db88552987 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1427,7 +1427,7 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,
 				left--;
 			}
 
-			if (c == '-') {
+			if (left && c == '-') {
 				err = proc_get_long(&p, &left, &val_b,
 						     &neg, tr_b, sizeof(tr_b),
 						     &c);

Regards,
Peter

> ---
> base-commit: 80234b5ab240f52fa45d201e899e207b9265ef91
> change-id: 20260312-fix-uninitialized-variable-in-proc_do_large_bitmap-30c6ef4ac1c5
> 
> Best regards,

Re: [PATCH ] sysctl: fix uninitialized variable in proc_do_large_bitmap

[Marc Buerg]

Hello Peter,

Thanks for your feedback and the idea. You are correct proc_get_long()
does not set @tr if @size is zero, therefore, left in
proc_do_large_bitmap() should be zero when we expect @tr to not be
written to and c still being uninitialized.

> Would the better fix be:
> 
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 354a2d294f52..89db88552987 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1427,7 +1427,7 @@ int proc_do_large_bitmap(struct ctl_table *table, in=
> t write,
>  				left--;
>  			}
>  
> -			if (c == '-') {
> +			if (left && c == '-') {
>  				err = proc_get_long(&p, &left, &val_b,
>  						     &neg, tr_b, sizeof(tr_b),
>  						     &c);

This would explicitly fix the problem as it enforces that we only check
if we know c contains what we want to check for. Fixing it like you
proposed seems better to me.

I am somewhat conflicted because leaving c uninitialized allows that a
similar problematic access of c could be made in the future.
Initializing c could prevent that. I also do not see an immediate
downside, but that could just be my naivety. Further, that part would
now behave similar to when we apply the default hardening configuration,
if my understanding is correct.

On the other hand, we do not read c later on, and I do not see a reason
why the function would change significantly. Still, it feels more
defensive to me to also set c to 0.

In the end, I am not so used to the kernel coding style. Is there
anything that can be argued against providing both? If you think this is
unnecessary I am happy to follow your reasoning and go with only the
check for left being non-zero.

Kind Regards,
Marc

Re: [PATCH ] sysctl: fix uninitialized variable in proc_do_large_bitmap

[Peter Seiderer]

Hello Marc,

On Sat, 14 Mar 2026 10:37:25 +0100, Marc Buerg <buermarc@googlemail.com> wrote:

> Hello Peter,
> 
> Thanks for your feedback and the idea. You are correct proc_get_long()
> does not set @tr if @size is zero, therefore, left in
> proc_do_large_bitmap() should be zero when we expect @tr to not be
> written to and c still being uninitialized.
> 
> > Would the better fix be:
> > 
> > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > index 354a2d294f52..89db88552987 100644
> > --- a/kernel/sysctl.c
> > +++ b/kernel/sysctl.c
> > @@ -1427,7 +1427,7 @@ int proc_do_large_bitmap(struct ctl_table *table, in=
> > t write,
> >  				left--;
> >  			}
> >  
> > -			if (c == '-') {
> > +			if (left && c == '-') {
> >  				err = proc_get_long(&p, &left, &val_b,
> >  						     &neg, tr_b, sizeof(tr_b),
> >  						     &c);  
> 
> This would explicitly fix the problem as it enforces that we only check
> if we know c contains what we want to check for. Fixing it like you
> proposed seems better to me.
> 
> I am somewhat conflicted because leaving c uninitialized allows that a
> similar problematic access of c could be made in the future.
> Initializing c could prevent that. I also do not see an immediate
> downside, but that could just be my naivety. Further, that part would
> now behave similar to when we apply the default hardening configuration,
> if my understanding is correct.

Your 'initialize c' (outside of the while loop) approach will only fix the
problem at the first iteration of the loop, on further iterations c will be
overwritten by the prior proc_get_long() calls..., for me the 'check c only
if valid' seems the better approach.... ;-)

Regards,
Peter

> 
> On the other hand, we do not read c later on, and I do not see a reason
> why the function would change significantly. Still, it feels more
> defensive to me to also set c to 0.
> 
> In the end, I am not so used to the kernel coding style. Is there
> anything that can be argued against providing both? If you think this is
> unnecessary I am happy to follow your reasoning and go with only the
> check for left being non-zero.
> 
> Kind Regards,
> Marc

Re: [PATCH ] sysctl: fix uninitialized variable in proc_do_large_bitmap

[buermarc]

From: Marc Buerg <buermarc@googlemail.com>

Hello Peter,

c will be overwritten by the previous proc_get_long() calls but that
will not be a problem. We only check c again if we had another trailing
char in either tr_a or tr_b[] = { ',', '\n', 0 }, minus '-'. If we find
a first '-' we will only reach the check again after hitting a trailing
char in tr_b. This ensures c must not be '-' before we might encounter a
new '-'. Meaning c should not contain a problematic value in future
iterations, but only in the first.

I agree: checking for left, as you proposed, should be done. I'll drop
setting c to zero and create a new patch which checks left.

Kind Regards,
Marc