* [PATCH net] flow_dissector: disable preemption around BPF calls
@ 2019-05-13 16:38 Eric Dumazet
2019-05-13 16:55 ` David Miller
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Eric Dumazet @ 2019-05-13 16:38 UTC (permalink / raw)
To: David S . Miller
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Eric Dumazet,
Eric Dumazet, bpf, syzbot, Petar Penkov, Stanislav Fomichev
Various things in eBPF really require us to disable preemption
before running an eBPF program.
syzbot reported :
BUG: assuming atomic context at net/core/flow_dissector.c:737
in_atomic(): 0, irqs_disabled(): 0, pid: 24710, name: syz-executor.3
2 locks held by syz-executor.3/24710:
#0: 00000000e81a4bf1 (&tfile->napi_mutex){+.+.}, at: tun_get_user+0x168e/0x3ff0 drivers/net/tun.c:1850
#1: 00000000254afebd (rcu_read_lock){....}, at: __skb_flow_dissect+0x1e1/0x4bb0 net/core/flow_dissector.c:822
CPU: 1 PID: 24710 Comm: syz-executor.3 Not tainted 5.1.0+ #6
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
__cant_sleep kernel/sched/core.c:6165 [inline]
__cant_sleep.cold+0xa3/0xbb kernel/sched/core.c:6142
bpf_flow_dissect+0xfe/0x390 net/core/flow_dissector.c:737
__skb_flow_dissect+0x362/0x4bb0 net/core/flow_dissector.c:853
skb_flow_dissect_flow_keys_basic include/linux/skbuff.h:1322 [inline]
skb_probe_transport_header include/linux/skbuff.h:2500 [inline]
skb_probe_transport_header include/linux/skbuff.h:2493 [inline]
tun_get_user+0x2cfe/0x3ff0 drivers/net/tun.c:1940
tun_chr_write_iter+0xbd/0x156 drivers/net/tun.c:2037
call_write_iter include/linux/fs.h:1872 [inline]
do_iter_readv_writev+0x5fd/0x900 fs/read_write.c:693
do_iter_write fs/read_write.c:970 [inline]
do_iter_write+0x184/0x610 fs/read_write.c:951
vfs_writev+0x1b3/0x2f0 fs/read_write.c:1015
do_writev+0x15b/0x330 fs/read_write.c:1058
__do_sys_writev fs/read_write.c:1131 [inline]
__se_sys_writev fs/read_write.c:1128 [inline]
__x64_sys_writev+0x75/0xb0 fs/read_write.c:1128
do_syscall_64+0x103/0x670 arch/x86/entry/common.c:298
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Petar Penkov <ppenkov@google.com>
Cc: Stanislav Fomichev <sdf@google.com>
---
net/core/flow_dissector.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 9ca784c592ac8c9c58282289a81889fbe4658a9e..548f39dde30711ac5be9e921993a6d8e53f74161 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -734,7 +734,9 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
flow_keys->nhoff = nhoff;
flow_keys->thoff = flow_keys->nhoff;
+ preempt_disable();
result = BPF_PROG_RUN(prog, ctx);
+ preempt_enable();
flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, nhoff, hlen);
flow_keys->thoff = clamp_t(u16, flow_keys->thoff,
--
2.21.0.1020.gf2820cf01a-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH net] flow_dissector: disable preemption around BPF calls
2019-05-13 16:38 [PATCH net] flow_dissector: disable preemption around BPF calls Eric Dumazet
@ 2019-05-13 16:55 ` David Miller
2019-05-13 16:56 ` Stanislav Fomichev
2019-05-13 17:17 ` Mark Rutland
2 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2019-05-13 16:55 UTC (permalink / raw)
To: edumazet; +Cc: ast, daniel, netdev, eric.dumazet, bpf, syzkaller, ppenkov, sdf
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 13 May 2019 09:38:55 -0700
> Various things in eBPF really require us to disable preemption
> before running an eBPF program.
>
> syzbot reported :
. ..
> Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
Applied and queued up for -stable.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] flow_dissector: disable preemption around BPF calls
2019-05-13 16:38 [PATCH net] flow_dissector: disable preemption around BPF calls Eric Dumazet
2019-05-13 16:55 ` David Miller
@ 2019-05-13 16:56 ` Stanislav Fomichev
2019-05-13 17:17 ` Mark Rutland
2 siblings, 0 replies; 7+ messages in thread
From: Stanislav Fomichev @ 2019-05-13 16:56 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Alexei Starovoitov, Daniel Borkmann, netdev,
Eric Dumazet, bpf, syzbot, Petar Penkov, Stanislav Fomichev
On 05/13, Eric Dumazet wrote:
> Various things in eBPF really require us to disable preemption
> before running an eBPF program.
>
> syzbot reported :
>
> BUG: assuming atomic context at net/core/flow_dissector.c:737
> in_atomic(): 0, irqs_disabled(): 0, pid: 24710, name: syz-executor.3
> 2 locks held by syz-executor.3/24710:
> #0: 00000000e81a4bf1 (&tfile->napi_mutex){+.+.}, at: tun_get_user+0x168e/0x3ff0 drivers/net/tun.c:1850
> #1: 00000000254afebd (rcu_read_lock){....}, at: __skb_flow_dissect+0x1e1/0x4bb0 net/core/flow_dissector.c:822
> CPU: 1 PID: 24710 Comm: syz-executor.3 Not tainted 5.1.0+ #6
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> __cant_sleep kernel/sched/core.c:6165 [inline]
> __cant_sleep.cold+0xa3/0xbb kernel/sched/core.c:6142
> bpf_flow_dissect+0xfe/0x390 net/core/flow_dissector.c:737
> __skb_flow_dissect+0x362/0x4bb0 net/core/flow_dissector.c:853
> skb_flow_dissect_flow_keys_basic include/linux/skbuff.h:1322 [inline]
> skb_probe_transport_header include/linux/skbuff.h:2500 [inline]
> skb_probe_transport_header include/linux/skbuff.h:2493 [inline]
> tun_get_user+0x2cfe/0x3ff0 drivers/net/tun.c:1940
> tun_chr_write_iter+0xbd/0x156 drivers/net/tun.c:2037
> call_write_iter include/linux/fs.h:1872 [inline]
> do_iter_readv_writev+0x5fd/0x900 fs/read_write.c:693
> do_iter_write fs/read_write.c:970 [inline]
> do_iter_write+0x184/0x610 fs/read_write.c:951
> vfs_writev+0x1b3/0x2f0 fs/read_write.c:1015
> do_writev+0x15b/0x330 fs/read_write.c:1058
> __do_sys_writev fs/read_write.c:1131 [inline]
> __se_sys_writev fs/read_write.c:1128 [inline]
> __x64_sys_writev+0x75/0xb0 fs/read_write.c:1128
> do_syscall_64+0x103/0x670 arch/x86/entry/common.c:298
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Cc: Petar Penkov <ppenkov@google.com>
> Cc: Stanislav Fomichev <sdf@google.com>
> ---
> net/core/flow_dissector.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 9ca784c592ac8c9c58282289a81889fbe4658a9e..548f39dde30711ac5be9e921993a6d8e53f74161 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -734,7 +734,9 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
> flow_keys->nhoff = nhoff;
> flow_keys->thoff = flow_keys->nhoff;
>
> + preempt_disable();
> result = BPF_PROG_RUN(prog, ctx);
> + preempt_enable();
>
> flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, nhoff, hlen);
> flow_keys->thoff = clamp_t(u16, flow_keys->thoff,
> --
> 2.21.0.1020.gf2820cf01a-goog
>
Reviewed-by: Stanislav Fomichev <sdf@google.com>
Thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] flow_dissector: disable preemption around BPF calls
2019-05-13 16:38 [PATCH net] flow_dissector: disable preemption around BPF calls Eric Dumazet
2019-05-13 16:55 ` David Miller
2019-05-13 16:56 ` Stanislav Fomichev
@ 2019-05-13 17:17 ` Mark Rutland
2019-05-13 17:20 ` Eric Dumazet
2 siblings, 1 reply; 7+ messages in thread
From: Mark Rutland @ 2019-05-13 17:17 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Alexei Starovoitov, Daniel Borkmann, netdev,
Eric Dumazet, bpf, syzbot, Petar Penkov, Stanislav Fomichev
On Mon, May 13, 2019 at 09:38:55AM -0700, 'Eric Dumazet' via syzkaller wrote:
> Various things in eBPF really require us to disable preemption
> before running an eBPF program.
Is that true for all eBPF uses? I note that we don't disable preemption
in the lib/test_bpf.c module, for example.
If it's a general requirement, perhaps it's worth an assertion within
BPF_PROG_RUN()?
Thanks,
Mark.
>
> syzbot reported :
>
> BUG: assuming atomic context at net/core/flow_dissector.c:737
> in_atomic(): 0, irqs_disabled(): 0, pid: 24710, name: syz-executor.3
> 2 locks held by syz-executor.3/24710:
> #0: 00000000e81a4bf1 (&tfile->napi_mutex){+.+.}, at: tun_get_user+0x168e/0x3ff0 drivers/net/tun.c:1850
> #1: 00000000254afebd (rcu_read_lock){....}, at: __skb_flow_dissect+0x1e1/0x4bb0 net/core/flow_dissector.c:822
> CPU: 1 PID: 24710 Comm: syz-executor.3 Not tainted 5.1.0+ #6
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
> __dump_stack lib/dump_stack.c:77 [inline]
> dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> __cant_sleep kernel/sched/core.c:6165 [inline]
> __cant_sleep.cold+0xa3/0xbb kernel/sched/core.c:6142
> bpf_flow_dissect+0xfe/0x390 net/core/flow_dissector.c:737
> __skb_flow_dissect+0x362/0x4bb0 net/core/flow_dissector.c:853
> skb_flow_dissect_flow_keys_basic include/linux/skbuff.h:1322 [inline]
> skb_probe_transport_header include/linux/skbuff.h:2500 [inline]
> skb_probe_transport_header include/linux/skbuff.h:2493 [inline]
> tun_get_user+0x2cfe/0x3ff0 drivers/net/tun.c:1940
> tun_chr_write_iter+0xbd/0x156 drivers/net/tun.c:2037
> call_write_iter include/linux/fs.h:1872 [inline]
> do_iter_readv_writev+0x5fd/0x900 fs/read_write.c:693
> do_iter_write fs/read_write.c:970 [inline]
> do_iter_write+0x184/0x610 fs/read_write.c:951
> vfs_writev+0x1b3/0x2f0 fs/read_write.c:1015
> do_writev+0x15b/0x330 fs/read_write.c:1058
> __do_sys_writev fs/read_write.c:1131 [inline]
> __se_sys_writev fs/read_write.c:1128 [inline]
> __x64_sys_writev+0x75/0xb0 fs/read_write.c:1128
> do_syscall_64+0x103/0x670 arch/x86/entry/common.c:298
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Cc: Petar Penkov <ppenkov@google.com>
> Cc: Stanislav Fomichev <sdf@google.com>
> ---
> net/core/flow_dissector.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 9ca784c592ac8c9c58282289a81889fbe4658a9e..548f39dde30711ac5be9e921993a6d8e53f74161 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -734,7 +734,9 @@ bool bpf_flow_dissect(struct bpf_prog *prog, struct bpf_flow_dissector *ctx,
> flow_keys->nhoff = nhoff;
> flow_keys->thoff = flow_keys->nhoff;
>
> + preempt_disable();
> result = BPF_PROG_RUN(prog, ctx);
> + preempt_enable();
>
> flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, nhoff, hlen);
> flow_keys->thoff = clamp_t(u16, flow_keys->thoff,
> --
> 2.21.0.1020.gf2820cf01a-goog
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller/20190513163855.225489-1-edumazet%40google.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH net] flow_dissector: disable preemption around BPF calls
2019-05-13 17:17 ` Mark Rutland
@ 2019-05-13 17:20 ` Eric Dumazet
2019-05-13 17:25 ` Mark Rutland
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2019-05-13 17:20 UTC (permalink / raw)
To: Mark Rutland
Cc: David S . Miller, Alexei Starovoitov, Daniel Borkmann, netdev,
Eric Dumazet, bpf, syzbot, Petar Penkov, Stanislav Fomichev
On Mon, May 13, 2019 at 10:17 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Mon, May 13, 2019 at 09:38:55AM -0700, 'Eric Dumazet' via syzkaller wrote:
> > Various things in eBPF really require us to disable preemption
> > before running an eBPF program.
>
> Is that true for all eBPF uses? I note that we don't disable preemption
> in the lib/test_bpf.c module, for example.
>
> If it's a general requirement, perhaps it's worth an assertion within
> BPF_PROG_RUN()?
The assertion is already there :)
This is how syzbot triggered the report.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] flow_dissector: disable preemption around BPF calls
2019-05-13 17:20 ` Eric Dumazet
@ 2019-05-13 17:25 ` Mark Rutland
2019-05-13 17:52 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Mark Rutland @ 2019-05-13 17:25 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S . Miller, Alexei Starovoitov, Daniel Borkmann, netdev,
Eric Dumazet, bpf, syzbot, Petar Penkov, Stanislav Fomichev
On Mon, May 13, 2019 at 10:20:19AM -0700, 'Eric Dumazet' via syzkaller wrote:
> On Mon, May 13, 2019 at 10:17 AM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Mon, May 13, 2019 at 09:38:55AM -0700, 'Eric Dumazet' via syzkaller wrote:
> > > Various things in eBPF really require us to disable preemption
> > > before running an eBPF program.
> >
> > Is that true for all eBPF uses? I note that we don't disable preemption
> > in the lib/test_bpf.c module, for example.
> >
> > If it's a general requirement, perhaps it's worth an assertion within
> > BPF_PROG_RUN()?
>
> The assertion is already there :)
>
> This is how syzbot triggered the report.
Ah! :)
I also see I'm wrong about test_bpf.c, so sorry for the noise on both
counts!
Thanks,
Mark.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net] flow_dissector: disable preemption around BPF calls
2019-05-13 17:25 ` Mark Rutland
@ 2019-05-13 17:52 ` Eric Dumazet
0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2019-05-13 17:52 UTC (permalink / raw)
To: Mark Rutland, Eric Dumazet
Cc: David S . Miller, Alexei Starovoitov, Daniel Borkmann, netdev,
bpf, syzbot, Petar Penkov, Stanislav Fomichev
On 5/13/19 10:25 AM, Mark Rutland wrote:
> On Mon, May 13, 2019 at 10:20:19AM -0700, 'Eric Dumazet' via syzkaller wrote:
>> On Mon, May 13, 2019 at 10:17 AM Mark Rutland <mark.rutland@arm.com> wrote:
>>>
>>> On Mon, May 13, 2019 at 09:38:55AM -0700, 'Eric Dumazet' via syzkaller wrote:
>>>> Various things in eBPF really require us to disable preemption
>>>> before running an eBPF program.
>>>
>>> Is that true for all eBPF uses? I note that we don't disable preemption
>>> in the lib/test_bpf.c module, for example.
>>>
>>> If it's a general requirement, perhaps it's worth an assertion within
>>> BPF_PROG_RUN()?
>>
>> The assertion is already there :)
>>
>> This is how syzbot triggered the report.
>
> Ah! :)
>
> I also see I'm wrong about test_bpf.c, so sorry for the noise on both
> counts!
No worries, thanks for reviewing !
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-05-13 17:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-13 16:38 [PATCH net] flow_dissector: disable preemption around BPF calls Eric Dumazet
2019-05-13 16:55 ` David Miller
2019-05-13 16:56 ` Stanislav Fomichev
2019-05-13 17:17 ` Mark Rutland
2019-05-13 17:20 ` Eric Dumazet
2019-05-13 17:25 ` Mark Rutland
2019-05-13 17:52 ` Eric Dumazet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).