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

[buermarc]

From: Marc Buerg <buermarc@xxxxxxxxxxxxxx>

Hi,

On Tue, 24 Mar 2026 08:44:13 +0100 Joel Granados, wrote:

> 1. len is calculated by doing len = p - tmp
> 2. tmp is the beginning of the buffer
> 3. p is the first character that is not a digit.
>
> There will always be at least one ('\0') character that is not a digit.

Yes.

> So len will be less than size at least by one (could be more if the
> string contains non digit chars). When it is parsed to the end,
> len will be less than size by one.

I don't think this is the case. See below.

> TL;DR

I'll try to make it shorter. size is the length of the user buffer, even
after the copy to the kernel buffer. The added '\0' does not increment
size in proc_sys_call_handler(). Following the example for
write(fd,"123",3). In the beginning of proc_get_long():

size = 3

@:012 3
-------
 "123\0"
  |
 tmp=@0
  |
  p=@0

After strtoul_lenient():

@:012 3
-------
 "123\0"
  |   |
 tmp=@0
      |
      p=@3

@3 - @0 = 3
len = 3

(len < size) => (3 < 3) => false

And size must be 3, proc_sys_call_handler() does not increment it. If we
provide something like write(fd,"123\0",4) size will be 4, but now we will
return -EINVAL.

If len < size is true, we call memchk(). memchk will not find a trailing
character as p points to '\0'. This means in this case we would return
-EINVAL for the first proc_get_long() call in the while loop. Reason is
'\0' is not in tr_a and therefore not a valid delimiter for the first
proc_get_long() call.

I hope this get's my point across.

> All this holds unless the trailing '\0' is modified in some way in that
> BPF call.

To my knowledge there was no bpf tracing enabled on the affected host at
the time that the behavior was observed.

Finally: the fix which adds a check for left introduces a regression.
"123-" is wrongly accepted. Not sure if you saw my other message. It
should not be used.

Best Regards,
Marc