From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Borkmann Subject: Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs Date: Fri, 08 Sep 2017 12:34:28 +0200 Message-ID: <59B27234.4050703@iogearbox.net> References: <01d69254802986ad3a8b18a8650c45df3df95def.1504821825.git.daniel@iogearbox.net> <20170908070610.4b8e1df1@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: davem@davemloft.net, ast@fb.com, john.fastabend@gmail.com, andy@greyhouse.net, netdev@vger.kernel.org To: Jesper Dangaard Brouer Return-path: Received: from www62.your-server.de ([213.133.104.62]:51024 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751675AbdIHKeh (ORCPT ); Fri, 8 Sep 2017 06:34:37 -0400 In-Reply-To: <20170908070610.4b8e1df1@redhat.com> Sender: netdev-owner@vger.kernel.org List-ID: On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote: > On Fri, 8 Sep 2017 00:14:51 +0200 > Daniel Borkmann wrote: > >> + /* This is really only caused by a deliberately crappy >> + * BPF program, normally we would never hit that case, >> + * so no need to inform someone via tracepoints either, >> + * just bail out. >> + */ >> + if (unlikely(map_owner != xdp_prog)) >> + return -EINVAL; > > IMHO we do need to call the tracepoint here. It is not just crappy > BPF-progs that cause this situation, it is also drivers not implementing > XDP_REDIRECT yet (which is all but ixgbe). Due to the level XDP > operates at, tracepoints are the only way users can runtime troubleshoot > their XDP programs. Drivers not implementing XDP_REDIRECT don't even get there in the first place. What they will do is to hit the 'default' case when they check for the action code from the BPF program. Then call into bpf_warn_invalid_xdp_action(act), and fall-through to hit the tracepoint at trace_xdp_exception() which is also triggered by XDP_ABORTED usually. So when that happens we do complain loudly and call a tracepoint already. We should probably tweak the bpf_warn_invalid_xdp_action() message a little to make it clear that the action could also just be unsupported by the driver instead of being illegal.