netdev - Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 08 Sep 2017 12:34:28 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
CC:     davem@...emloft.net, ast@...com, john.fastabend@...il.com,
        andy@...yhouse.net, netdev@...r.kernel.org
Subject: Re: [PATCH net] bpf: don't select potentially stale ri->map from
 buggy xdp progs

On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote:
> On Fri,  8 Sep 2017 00:14:51 +0200
> Daniel Borkmann <daniel@...earbox.net> 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.