Re: [PATCH] bpf: fix uninit-value in strnchr

From: Edward Adam Davis
Date: Tue Apr 09 2024 - 20:28:22 EST


On Tue, 9 Apr 2024 10:59:17 -0700, Martin KaFai Lau wrote:
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index 449b9a5d3fe3..07490eba24fe 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -826,7 +826,7 @@ int bpf_bprintf_prepare(char *fmt, u32 fmt_size, const u64 *raw_args,
> > u64 cur_arg;
> > char fmt_ptype, cur_ip[16], ip_spec[] = "%pXX";
> >
> > - fmt_end = strnchr(fmt, fmt_size, 0);
> > + fmt_end = strnchrnul(fmt, fmt_size, 0);
>
> I don't think it is correct either.
>
> > if (!fmt_end)
>
> e.g. what will strnchrnul return if fmt is not NULL terminated?
>
> The current code is correct as is. Comment snippet from strnchr:
>
> /*
> * ...
> *
> * Note that the %NUL-terminator is considered part of the string, and can
> * be searched for.
> */
> char *strnchr(const char *s, size_t count, int c)
lib/string.c
9 /**
8 * strnchr - Find a character in a length limited string
7 * @s: The string to be searched
6 * @count: The number of characters to be searched
5 * @c: The character to search for
4 *
3 * Note that the %NUL-terminator is considered part of the string, and can
2 * be searched for.
1 */
384 char *strnchr(const char *s, size_t count, int c)
1 {
2 while (count--) {
3 if (*s == (char)c) // Only when the length of s is 1, can NUL char be obtained
4 return (char *)s;
5 if (*s++ == '\0') // When the length of s is greater than 1, the loop will terminate and return NULL, without obtaining a pointer to a NUL char
6 break;
7 }
8 return NULL;
9 }
>
>
> > return -EINVAL;
> > fmt_size = fmt_end - fmt;