From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexei Starovoitov Subject: Re: [PATCH bpf] bpf: fix a rcu usage warning in bpf_prog_array_copy_core() Date: Wed, 15 Aug 2018 14:30:11 -0700 Message-ID: <20180815213009.t2nhtoxlqpd2sgk4@ast-mbp> References: <20180814180112.293058-1-yhs@fb.com> <20180814235944.5nufzmj6d2rkknvs@ast-mbp> <20180815000841.GA25304@castle.DHCP.thefacebook.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Yonghong Song , ast@fb.com, daniel@iogearbox.net, netdev@vger.kernel.org, kernel-team@fb.com To: Roman Gushchin Return-path: Received: from mail-pf1-f194.google.com ([209.85.210.194]:41214 "EHLO mail-pf1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727500AbeHPAYK (ORCPT ); Wed, 15 Aug 2018 20:24:10 -0400 Received: by mail-pf1-f194.google.com with SMTP id y10-v6so1023561pfn.8 for ; Wed, 15 Aug 2018 14:30:14 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20180815000841.GA25304@castle.DHCP.thefacebook.com> Sender: netdev-owner@vger.kernel.org List-ID: 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 > > > Signed-off-by: Yonghong Song > > > > makes sense to me > > Acked-by: Alexei Starovoitov > > > > 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.