Messages in this thread | | | Date | Tue, 9 Apr 2024 10:59:17 -0700 | Subject | Re: [PATCH] bpf: fix uninit-value in strnchr | From | Martin KaFai Lau <> |
| |
On 4/9/24 4:37 AM, Edward Adam Davis wrote: > According to the context in bpf_bprintf_prepare(), this is checking if fmt ends > with a NUL word. Therefore, strnchrnul() should be used for validation instead > of strnchr().
As your another email, this is not fixing the uninit KMSAN report.
If there was a separate bug, please post a separate patch instead of replying to an unrelated thread and confuse syzbot.
> > Reported-by: syzbot+9b8be5e35747291236c8@syzkaller.appspotmail.com > Signed-off-by: Edward Adam Davis <eadavis@qq.com> > --- > kernel/bpf/helpers.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > 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)
> return -EINVAL; > fmt_size = fmt_end - fmt;
| |