public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Seiderer <ps.report@gmx.net>
To: Marc Buerg <buermarc@googlemail.com>
Cc: Kees Cook <kees@kernel.org>,
	Joel Granados <joel.granados@kernel.org>,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	stable@vger.kernel.org, Elias Oezcan <elias.rw2@gmail.com>
Subject: Re: [PATCH] sysctl: fix uninitialized variable in proc_do_large_bitmap
Date: Fri, 13 Mar 2026 12:17:08 +0100	[thread overview]
Message-ID: <20260313121708.137dae22@pc-1> (raw)
In-Reply-To: <20260312-fix-uninitialized-variable-in-proc_do_large_bitmap-v1-1-35ad2dddaf21@googlemail.com>

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,


  reply	other threads:[~2026-03-13 11:17 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-12 15:42 [PATCH] sysctl: fix uninitialized variable in proc_do_large_bitmap Marc Buerg
2026-03-13 11:17 ` Peter Seiderer [this message]
2026-03-14  9:37   ` [PATCH ] " Marc Buerg
2026-03-15 14:26     ` Peter Seiderer
2026-03-17 21:32       ` buermarc

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=20260313121708.137dae22@pc-1 \
    --to=ps.report@gmx.net \
    --cc=buermarc@googlemail.com \
    --cc=elias.rw2@gmail.com \
    --cc=joel.granados@kernel.org \
    --cc=kees@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox