* [PATCH bpf] bpf: fix a rcu usage warning in bpf_prog_array_copy_core()
@ 2018-08-14 18:01 Yonghong Song
2018-08-14 23:59 ` Alexei Starovoitov
0 siblings, 1 reply; 6+ messages in thread
From: Yonghong Song @ 2018-08-14 18:01 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team, Roman Gushchin
Commit 394e40a29788 ("bpf: extend bpf_prog_array to store pointers
to the cgroup storage") refactored the bpf_prog_array_copy_core()
to accommodate new structure bpf_prog_array_item which contains
bpf_prog array itself.
In the old code, we had
perf_event_query_prog_array():
mutex_lock(...)
bpf_prog_array_copy_call():
prog = rcu_dereference_check(array, 1)->progs
bpf_prog_array_copy_core(prog, ...)
mutex_unlock(...)
With the above commit, we had
perf_event_query_prog_array():
mutex_lock(...)
bpf_prog_array_copy_call():
bpf_prog_array_copy_core(array, ...):
item = rcu_dereference(array)->items;
...
mutex_unlock(...)
The new code will trigger a lockdep rcu checking warning.
The fix is to change rcu_dereference() to rcu_dereference_check()
to prevent such a warning.
Reported-by: syzbot+6e72317008eef84a216b@syzkaller.appspotmail.com
Fixes: 394e40a29788 ("bpf: extend bpf_prog_array to store pointers to the cgroup storage")
Cc: Roman Gushchin <guro@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 4d09e610777f..3f5bf1af0826 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1579,7 +1579,7 @@ static bool bpf_prog_array_copy_core(struct bpf_prog_array __rcu *array,
struct bpf_prog_array_item *item;
int i = 0;
- item = rcu_dereference(array)->items;
+ item = rcu_dereference_check(array, 1)->items;
for (; item->prog; item++) {
if (item->prog == &dummy_bpf_prog.prog)
continue;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH bpf] bpf: fix a rcu usage warning in bpf_prog_array_copy_core()
2018-08-14 18:01 [PATCH bpf] bpf: fix a rcu usage warning in bpf_prog_array_copy_core() Yonghong Song
@ 2018-08-14 23:59 ` Alexei Starovoitov
2018-08-15 0:08 ` Roman Gushchin
0 siblings, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2018-08-14 23:59 UTC (permalink / raw)
To: Yonghong Song; +Cc: ast, daniel, netdev, kernel-team, Roman Gushchin
On Tue, Aug 14, 2018 at 11:01:12AM -0700, Yonghong Song wrote:
> Commit 394e40a29788 ("bpf: extend bpf_prog_array to store pointers
> to the cgroup storage") refactored the bpf_prog_array_copy_core()
> to accommodate new structure bpf_prog_array_item which contains
> bpf_prog array itself.
>
> In the old code, we had
> perf_event_query_prog_array():
> mutex_lock(...)
> bpf_prog_array_copy_call():
> prog = rcu_dereference_check(array, 1)->progs
> bpf_prog_array_copy_core(prog, ...)
> mutex_unlock(...)
>
> With the above commit, we had
> perf_event_query_prog_array():
> mutex_lock(...)
> bpf_prog_array_copy_call():
> bpf_prog_array_copy_core(array, ...):
> item = rcu_dereference(array)->items;
> ...
> mutex_unlock(...)
>
> The new code will trigger a lockdep rcu checking warning.
> The fix is to change rcu_dereference() to rcu_dereference_check()
> to prevent such a warning.
>
> Reported-by: syzbot+6e72317008eef84a216b@syzkaller.appspotmail.com
> Fixes: 394e40a29788 ("bpf: extend bpf_prog_array to store pointers to the cgroup storage")
> Cc: Roman Gushchin <guro@fb.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>
makes sense to me
Acked-by: Alexei Starovoitov <ast@kernel.org>
Roman, would you agree?
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH bpf] bpf: fix a rcu usage warning in bpf_prog_array_copy_core()
2018-08-14 23:59 ` Alexei Starovoitov
@ 2018-08-15 0:08 ` Roman Gushchin
2018-08-15 21:30 ` Alexei Starovoitov
2018-08-16 0:17 ` Daniel Borkmann
0 siblings, 2 replies; 6+ messages in thread
From: Roman Gushchin @ 2018-08-15 0:08 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Yonghong Song, ast, daniel, netdev, kernel-team
On Tue, Aug 14, 2018 at 04:59:45PM -0700, Alexei Starovoitov wrote:
> On Tue, Aug 14, 2018 at 11:01:12AM -0700, Yonghong Song wrote:
> > Commit 394e40a29788 ("bpf: extend bpf_prog_array to store pointers
> > to the cgroup storage") refactored the bpf_prog_array_copy_core()
> > to accommodate new structure bpf_prog_array_item which contains
> > bpf_prog array itself.
> >
> > In the old code, we had
> > perf_event_query_prog_array():
> > mutex_lock(...)
> > bpf_prog_array_copy_call():
> > prog = rcu_dereference_check(array, 1)->progs
> > bpf_prog_array_copy_core(prog, ...)
> > mutex_unlock(...)
> >
> > With the above commit, we had
> > perf_event_query_prog_array():
> > mutex_lock(...)
> > bpf_prog_array_copy_call():
> > bpf_prog_array_copy_core(array, ...):
> > item = rcu_dereference(array)->items;
> > ...
> > mutex_unlock(...)
> >
> > The new code will trigger a lockdep rcu checking warning.
> > The fix is to change rcu_dereference() to rcu_dereference_check()
> > to prevent such a warning.
> >
> > Reported-by: syzbot+6e72317008eef84a216b@syzkaller.appspotmail.com
> > Fixes: 394e40a29788 ("bpf: extend bpf_prog_array to store pointers to the cgroup storage")
> > Cc: Roman Gushchin <guro@fb.com>
> > Signed-off-by: Yonghong Song <yhs@fb.com>
>
> makes sense to me
> Acked-by: Alexei Starovoitov <ast@kernel.org>
>
> Roman, would you agree?
>
rcu_dereference_check(<>, 1) always looks a bit strange to me,
but if it's the only reasonable way to silence the warning,
of course I'm fine with it.
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH bpf] bpf: fix a rcu usage warning in bpf_prog_array_copy_core()
2018-08-15 0:08 ` Roman Gushchin
@ 2018-08-15 21:30 ` Alexei Starovoitov
2018-08-15 21:37 ` Roman Gushchin
2018-08-16 0:17 ` Daniel Borkmann
1 sibling, 1 reply; 6+ messages in thread
From: Alexei Starovoitov @ 2018-08-15 21:30 UTC (permalink / raw)
To: Roman Gushchin; +Cc: Yonghong Song, ast, daniel, netdev, kernel-team
On Tue, Aug 14, 2018 at 05:08:44PM -0700, Roman Gushchin wrote:
> On Tue, Aug 14, 2018 at 04:59:45PM -0700, Alexei Starovoitov wrote:
> > On Tue, Aug 14, 2018 at 11:01:12AM -0700, Yonghong Song wrote:
> > > Commit 394e40a29788 ("bpf: extend bpf_prog_array to store pointers
> > > to the cgroup storage") refactored the bpf_prog_array_copy_core()
> > > to accommodate new structure bpf_prog_array_item which contains
> > > bpf_prog array itself.
> > >
> > > In the old code, we had
> > > perf_event_query_prog_array():
> > > mutex_lock(...)
> > > bpf_prog_array_copy_call():
> > > prog = rcu_dereference_check(array, 1)->progs
> > > bpf_prog_array_copy_core(prog, ...)
> > > mutex_unlock(...)
> > >
> > > With the above commit, we had
> > > perf_event_query_prog_array():
> > > mutex_lock(...)
> > > bpf_prog_array_copy_call():
> > > bpf_prog_array_copy_core(array, ...):
> > > item = rcu_dereference(array)->items;
> > > ...
> > > mutex_unlock(...)
> > >
> > > The new code will trigger a lockdep rcu checking warning.
> > > The fix is to change rcu_dereference() to rcu_dereference_check()
> > > to prevent such a warning.
> > >
> > > Reported-by: syzbot+6e72317008eef84a216b@syzkaller.appspotmail.com
> > > Fixes: 394e40a29788 ("bpf: extend bpf_prog_array to store pointers to the cgroup storage")
> > > Cc: Roman Gushchin <guro@fb.com>
> > > Signed-off-by: Yonghong Song <yhs@fb.com>
> >
> > makes sense to me
> > Acked-by: Alexei Starovoitov <ast@kernel.org>
> >
> > Roman, would you agree?
> >
>
> rcu_dereference_check(<>, 1) always looks a bit strange to me,
> but if it's the only reasonable way to silence the warning,
> of course I'm fine with it.
do you have better suggestion?
This patch is a fix for the regression introduced in your earlier patch,
so I think the only fair path forward is either to Ack it or
to send an alternative patch asap.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH bpf] bpf: fix a rcu usage warning in bpf_prog_array_copy_core()
2018-08-15 21:30 ` Alexei Starovoitov
@ 2018-08-15 21:37 ` Roman Gushchin
0 siblings, 0 replies; 6+ messages in thread
From: Roman Gushchin @ 2018-08-15 21:37 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Yonghong Song, ast, daniel, netdev, kernel-team
On Wed, Aug 15, 2018 at 02:30:11PM -0700, Alexei Starovoitov wrote:
> On Tue, Aug 14, 2018 at 05:08:44PM -0700, Roman Gushchin wrote:
> > On Tue, Aug 14, 2018 at 04:59:45PM -0700, Alexei Starovoitov wrote:
> > > On Tue, Aug 14, 2018 at 11:01:12AM -0700, Yonghong Song wrote:
> > > > Commit 394e40a29788 ("bpf: extend bpf_prog_array to store pointers
> > > > to the cgroup storage") refactored the bpf_prog_array_copy_core()
> > > > to accommodate new structure bpf_prog_array_item which contains
> > > > bpf_prog array itself.
> > > >
> > > > In the old code, we had
> > > > perf_event_query_prog_array():
> > > > mutex_lock(...)
> > > > bpf_prog_array_copy_call():
> > > > prog = rcu_dereference_check(array, 1)->progs
> > > > bpf_prog_array_copy_core(prog, ...)
> > > > mutex_unlock(...)
> > > >
> > > > With the above commit, we had
> > > > perf_event_query_prog_array():
> > > > mutex_lock(...)
> > > > bpf_prog_array_copy_call():
> > > > bpf_prog_array_copy_core(array, ...):
> > > > item = rcu_dereference(array)->items;
> > > > ...
> > > > mutex_unlock(...)
> > > >
> > > > The new code will trigger a lockdep rcu checking warning.
> > > > The fix is to change rcu_dereference() to rcu_dereference_check()
> > > > to prevent such a warning.
> > > >
> > > > Reported-by: syzbot+6e72317008eef84a216b@syzkaller.appspotmail.com
> > > > Fixes: 394e40a29788 ("bpf: extend bpf_prog_array to store pointers to the cgroup storage")
> > > > Cc: Roman Gushchin <guro@fb.com>
> > > > Signed-off-by: Yonghong Song <yhs@fb.com>
> > >
> > > makes sense to me
> > > Acked-by: Alexei Starovoitov <ast@kernel.org>
> > >
> > > Roman, would you agree?
> > >
> >
> > rcu_dereference_check(<>, 1) always looks a bit strange to me,
> > but if it's the only reasonable way to silence the warning,
> > of course I'm fine with it.
>
> do you have better suggestion?
> This patch is a fix for the regression introduced in your earlier patch,
> so I think the only fair path forward is either to Ack it or
> to send an alternative patch asap.
>
As I said, I've nothing against.
Acked-by: Roman Gushchin <guro@fb.com>
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] bpf: fix a rcu usage warning in bpf_prog_array_copy_core()
2018-08-15 0:08 ` Roman Gushchin
2018-08-15 21:30 ` Alexei Starovoitov
@ 2018-08-16 0:17 ` Daniel Borkmann
1 sibling, 0 replies; 6+ messages in thread
From: Daniel Borkmann @ 2018-08-16 0:17 UTC (permalink / raw)
To: Roman Gushchin, Alexei Starovoitov
Cc: Yonghong Song, ast, netdev, kernel-team
On 08/15/2018 02:08 AM, Roman Gushchin wrote:
> On Tue, Aug 14, 2018 at 04:59:45PM -0700, Alexei Starovoitov wrote:
>> On Tue, Aug 14, 2018 at 11:01:12AM -0700, Yonghong Song wrote:
>>> Commit 394e40a29788 ("bpf: extend bpf_prog_array to store pointers
>>> to the cgroup storage") refactored the bpf_prog_array_copy_core()
>>> to accommodate new structure bpf_prog_array_item which contains
>>> bpf_prog array itself.
>>>
>>> In the old code, we had
>>> perf_event_query_prog_array():
>>> mutex_lock(...)
>>> bpf_prog_array_copy_call():
>>> prog = rcu_dereference_check(array, 1)->progs
>>> bpf_prog_array_copy_core(prog, ...)
>>> mutex_unlock(...)
>>>
>>> With the above commit, we had
>>> perf_event_query_prog_array():
>>> mutex_lock(...)
>>> bpf_prog_array_copy_call():
>>> bpf_prog_array_copy_core(array, ...):
>>> item = rcu_dereference(array)->items;
>>> ...
>>> mutex_unlock(...)
>>>
>>> The new code will trigger a lockdep rcu checking warning.
>>> The fix is to change rcu_dereference() to rcu_dereference_check()
>>> to prevent such a warning.
>>>
>>> Reported-by: syzbot+6e72317008eef84a216b@syzkaller.appspotmail.com
>>> Fixes: 394e40a29788 ("bpf: extend bpf_prog_array to store pointers to the cgroup storage")
>>> Cc: Roman Gushchin <guro@fb.com>
>>> Signed-off-by: Yonghong Song <yhs@fb.com>
Applied to bpf, thanks Yonghong!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-16 3:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-14 18:01 [PATCH bpf] bpf: fix a rcu usage warning in bpf_prog_array_copy_core() Yonghong Song
2018-08-14 23:59 ` Alexei Starovoitov
2018-08-15 0:08 ` Roman Gushchin
2018-08-15 21:30 ` Alexei Starovoitov
2018-08-15 21:37 ` Roman Gushchin
2018-08-16 0:17 ` Daniel Borkmann
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).