1. Patchew
  2. linux
  3. [PATCH bpf v2 0/2] bpf: enhance validation of pointer formatting
  4. View patch

[PATCH bpf v2 1/2] bpf: fix %p% runtime check in bpf_bprintf_prepare

Ilya Shchipletsov posted 2 patches 3 weeks, 6 days ago
There is a newer version of this series
[PATCH bpf v2 1/2] bpf: fix %p% runtime check in bpf_bprintf_prepare
Posted by Ilya Shchipletsov 3 weeks, 6 days ago 
Fuzzing reports a warning in format_decode()

Please remove unsupported %� in format string
WARNING: CPU: 0 PID: 5091 at lib/vsprintf.c:2680 format_decode+0x1193/0x1bb0 lib/vsprintf.c:2680
Modules linked in:
CPU: 0 PID: 5091 Comm: syz-executor879 Not tainted 6.10.0-rc1-syzkaller-00021-ge0cce98fe279 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/02/2024
RIP: 0010:format_decode+0x1193/0x1bb0 lib/vsprintf.c:2680
Call Trace:
 <TASK>
 bstr_printf+0x137/0x1210 lib/vsprintf.c:3253
 ____bpf_trace_printk kernel/trace/bpf_trace.c:390 [inline]
 bpf_trace_printk+0x1a1/0x230 kernel/trace/bpf_trace.c:375
 bpf_prog_21da1b68f62e1237+0x36/0x41
 bpf_dispatcher_nop_func include/linux/bpf.h:1243 [inline]
 __bpf_prog_run include/linux/filter.h:691 [inline]
 bpf_prog_run include/linux/filter.h:698 [inline]
 bpf_test_run+0x40b/0x910 net/bpf/test_run.c:425
 bpf_prog_test_run_skb+0xafa/0x13a0 net/bpf/test_run.c:1066
 bpf_prog_test_run+0x33c/0x3b0 kernel/bpf/syscall.c:4291
 __sys_bpf+0x48d/0x810 kernel/bpf/syscall.c:5705
 __do_sys_bpf kernel/bpf/syscall.c:5794 [inline]
 __se_sys_bpf kernel/bpf/syscall.c:5792 [inline]
 __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5792
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

The problem occurs when trying to pass %p% at the end of format string,
which would result in skipping last % and passing invalid format string
down to format_decode() that would cause warning because of invalid
character after %.

Fix issue by advancing pointer only if next char is format modifier.
If next char is null/space/punct, then just accept formatting as is,
without advancing the pointer.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Reported-by: syzbot+e2c932aec5c8a6e1d31c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e2c932aec5c8a6e1d31c
Fixes: 48cac3f4a96d ("bpf: Implement formatted output helpers with bstr_printf")
Co-developed-by: Nikita Marushkin <hfggklm@gmail.com>
Signed-off-by: Nikita Marushkin <hfggklm@gmail.com>
Signed-off-by: Ilya Shchipletsov <rabbelkin@mail.ru>
---
 kernel/bpf/helpers.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 1a43d06eab28..3efa8b04999a 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -889,10 +889,8 @@ int bpf_bprintf_prepare(char *fmt, u32 fmt_size, const u64 *raw_args,
 				goto fmt_str;
 			}
 
-			if (fmt[i + 1] == 0 || isspace(fmt[i + 1]) ||
-			    ispunct(fmt[i + 1]) || fmt[i + 1] == 'K' ||
-			    fmt[i + 1] == 'x' || fmt[i + 1] == 's' ||
-			    fmt[i + 1] == 'S') {
+			if (fmt[i + 1] == 'K' || fmt[i + 1] == 'x' ||
+			    fmt[i + 1] == 's' || fmt[i + 1] == 'S') {
 				/* just kernel pointers */
 				if (tmp_buf)
 					cur_arg = raw_args[num_spec];
@@ -900,6 +898,13 @@ int bpf_bprintf_prepare(char *fmt, u32 fmt_size, const u64 *raw_args,
 				goto nocopy_fmt;
 			}
 
+			if (fmt[i + 1] == 0 || isspace(fmt[i + 1]) ||
+			    ispunct(fmt[i + 1])) {
+				if (tmp_buf)
+					cur_arg = raw_args[num_spec];
+				goto nocopy_fmt;
+			}
+
 			if (fmt[i + 1] == 'B') {
 				if (tmp_buf)  {
 					err = snprintf(tmp_buf,
-- 
2.43.0
Re: [PATCH bpf v2 1/2] bpf: fix %p% runtime check in bpf_bprintf_prepare
Posted by Yonghong Song 3 weeks, 6 days ago 
On 10/28/24 12:53 PM, Ilya Shchipletsov wrote:
> Fuzzing reports a warning in format_decode()
>
> Please remove unsupported %� in format string
> WARNING: CPU: 0 PID: 5091 at lib/vsprintf.c:2680 format_decode+0x1193/0x1bb0 lib/vsprintf.c:2680
> Modules linked in:
> CPU: 0 PID: 5091 Comm: syz-executor879 Not tainted 6.10.0-rc1-syzkaller-00021-ge0cce98fe279 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/02/2024
> RIP: 0010:format_decode+0x1193/0x1bb0 lib/vsprintf.c:2680
> Call Trace:
>   <TASK>
>   bstr_printf+0x137/0x1210 lib/vsprintf.c:3253
>   ____bpf_trace_printk kernel/trace/bpf_trace.c:390 [inline]
>   bpf_trace_printk+0x1a1/0x230 kernel/trace/bpf_trace.c:375
>   bpf_prog_21da1b68f62e1237+0x36/0x41
>   bpf_dispatcher_nop_func include/linux/bpf.h:1243 [inline]
>   __bpf_prog_run include/linux/filter.h:691 [inline]
>   bpf_prog_run include/linux/filter.h:698 [inline]
>   bpf_test_run+0x40b/0x910 net/bpf/test_run.c:425
>   bpf_prog_test_run_skb+0xafa/0x13a0 net/bpf/test_run.c:1066
>   bpf_prog_test_run+0x33c/0x3b0 kernel/bpf/syscall.c:4291
>   __sys_bpf+0x48d/0x810 kernel/bpf/syscall.c:5705
>   __do_sys_bpf kernel/bpf/syscall.c:5794 [inline]
>   __se_sys_bpf kernel/bpf/syscall.c:5792 [inline]
>   __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5792
>   do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>   do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
>   entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> The problem occurs when trying to pass %p% at the end of format string,
> which would result in skipping last % and passing invalid format string
> down to format_decode() that would cause warning because of invalid
> character after %.
>
> Fix issue by advancing pointer only if next char is format modifier.
> If next char is null/space/punct, then just accept formatting as is,
> without advancing the pointer.
>
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
>
> Reported-by: syzbot+e2c932aec5c8a6e1d31c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e2c932aec5c8a6e1d31c
> Fixes: 48cac3f4a96d ("bpf: Implement formatted output helpers with bstr_printf")
> Co-developed-by: Nikita Marushkin <hfggklm@gmail.com>
> Signed-off-by: Nikita Marushkin <hfggklm@gmail.com>
> Signed-off-by: Ilya Shchipletsov <rabbelkin@mail.ru>

Acked-by: Yonghong Song <yonghong.song@linux.dev>
Re: [PATCH bpf v2 1/2] bpf: fix %p% runtime check in bpf_bprintf_prepare
Posted by Florent Revest 3 weeks, 3 days ago 
Acked-by: Florent Revest <revest@chromium.org>

On Tue, Oct 29, 2024 at 7:18 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
> On 10/28/24 12:53 PM, Ilya Shchipletsov wrote:
> > Fuzzing reports a warning in format_decode()
> >
> > Please remove unsupported %� in format string
> > WARNING: CPU: 0 PID: 5091 at lib/vsprintf.c:2680 format_decode+0x1193/0x1bb0 lib/vsprintf.c:2680
> > Modules linked in:
> > CPU: 0 PID: 5091 Comm: syz-executor879 Not tainted 6.10.0-rc1-syzkaller-00021-ge0cce98fe279 #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/02/2024
> > RIP: 0010:format_decode+0x1193/0x1bb0 lib/vsprintf.c:2680
> > Call Trace:
> >   <TASK>
> >   bstr_printf+0x137/0x1210 lib/vsprintf.c:3253
> >   ____bpf_trace_printk kernel/trace/bpf_trace.c:390 [inline]
> >   bpf_trace_printk+0x1a1/0x230 kernel/trace/bpf_trace.c:375
> >   bpf_prog_21da1b68f62e1237+0x36/0x41
> >   bpf_dispatcher_nop_func include/linux/bpf.h:1243 [inline]
> >   __bpf_prog_run include/linux/filter.h:691 [inline]
> >   bpf_prog_run include/linux/filter.h:698 [inline]
> >   bpf_test_run+0x40b/0x910 net/bpf/test_run.c:425
> >   bpf_prog_test_run_skb+0xafa/0x13a0 net/bpf/test_run.c:1066
> >   bpf_prog_test_run+0x33c/0x3b0 kernel/bpf/syscall.c:4291
> >   __sys_bpf+0x48d/0x810 kernel/bpf/syscall.c:5705
> >   __do_sys_bpf kernel/bpf/syscall.c:5794 [inline]
> >   __se_sys_bpf kernel/bpf/syscall.c:5792 [inline]
> >   __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5792
> >   do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> >   do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> >   entry_SYSCALL_64_after_hwframe+0x77/0x7f
> >
> > The problem occurs when trying to pass %p% at the end of format string,
> > which would result in skipping last % and passing invalid format string
> > down to format_decode() that would cause warning because of invalid
> > character after %.
> >
> > Fix issue by advancing pointer only if next char is format modifier.
> > If next char is null/space/punct, then just accept formatting as is,
> > without advancing the pointer.
> >
> > Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> >
> > Reported-by: syzbot+e2c932aec5c8a6e1d31c@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=e2c932aec5c8a6e1d31c
> > Fixes: 48cac3f4a96d ("bpf: Implement formatted output helpers with bstr_printf")
> > Co-developed-by: Nikita Marushkin <hfggklm@gmail.com>
> > Signed-off-by: Nikita Marushkin <hfggklm@gmail.com>
> > Signed-off-by: Ilya Shchipletsov <rabbelkin@mail.ru>
>
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
>
Re: [PATCH bpf v2 1/2] bpf: fix %p% runtime check in bpf_bprintf_prepare
Posted by Fedor Pchelkin 2 weeks, 4 days ago 
On Thu, 31. Oct 17:17, Florent Revest wrote:
> Acked-by: Florent Revest <revest@chromium.org>
> 
> On Tue, Oct 29, 2024 at 7:18 AM Yonghong Song <yonghong.song@linux.dev> wrote:
> > On 10/28/24 12:53 PM, Ilya Shchipletsov wrote:
> > > Fuzzing reports a warning in format_decode()
> > >
> > > Please remove unsupported %� in format string
> > > WARNING: CPU: 0 PID: 5091 at lib/vsprintf.c:2680 format_decode+0x1193/0x1bb0 lib/vsprintf.c:2680
> > > Modules linked in:
> > > CPU: 0 PID: 5091 Comm: syz-executor879 Not tainted 6.10.0-rc1-syzkaller-00021-ge0cce98fe279 #0
> > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/02/2024
> > > RIP: 0010:format_decode+0x1193/0x1bb0 lib/vsprintf.c:2680
> > > Call Trace:
> > >   <TASK>
> > >   bstr_printf+0x137/0x1210 lib/vsprintf.c:3253
> > >   ____bpf_trace_printk kernel/trace/bpf_trace.c:390 [inline]
> > >   bpf_trace_printk+0x1a1/0x230 kernel/trace/bpf_trace.c:375
> > >   bpf_prog_21da1b68f62e1237+0x36/0x41
> > >   bpf_dispatcher_nop_func include/linux/bpf.h:1243 [inline]
> > >   __bpf_prog_run include/linux/filter.h:691 [inline]
> > >   bpf_prog_run include/linux/filter.h:698 [inline]
> > >   bpf_test_run+0x40b/0x910 net/bpf/test_run.c:425
> > >   bpf_prog_test_run_skb+0xafa/0x13a0 net/bpf/test_run.c:1066
> > >   bpf_prog_test_run+0x33c/0x3b0 kernel/bpf/syscall.c:4291
> > >   __sys_bpf+0x48d/0x810 kernel/bpf/syscall.c:5705
> > >   __do_sys_bpf kernel/bpf/syscall.c:5794 [inline]
> > >   __se_sys_bpf kernel/bpf/syscall.c:5792 [inline]
> > >   __x64_sys_bpf+0x7c/0x90 kernel/bpf/syscall.c:5792
> > >   do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > >   do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
> > >   entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > >
> > > The problem occurs when trying to pass %p% at the end of format string,
> > > which would result in skipping last % and passing invalid format string
> > > down to format_decode() that would cause warning because of invalid
> > > character after %.
> > >
> > > Fix issue by advancing pointer only if next char is format modifier.
> > > If next char is null/space/punct, then just accept formatting as is,
> > > without advancing the pointer.
> > >
> > > Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> > >
> > > Reported-by: syzbot+e2c932aec5c8a6e1d31c@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=e2c932aec5c8a6e1d31c
> > > Fixes: 48cac3f4a96d ("bpf: Implement formatted output helpers with bstr_printf")
> > > Co-developed-by: Nikita Marushkin <hfggklm@gmail.com>
> > > Signed-off-by: Nikita Marushkin <hfggklm@gmail.com>
> > > Signed-off-by: Ilya Shchipletsov <rabbelkin@mail.ru>
> >
> > Acked-by: Yonghong Song <yonghong.song@linux.dev>
> >

The patches of the series are marked as "Changes Requested" in Patchwork.
They've been acked twice. And there've been no additional comment posted
on mailing lists explaining the "Changes Required" status.

Is there anything to improve in the series? Or it can be applied as is?

--
Thanks,
Fedor

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.