netdev - Re: [PATCH net-next] xdp: implement xdp_redirect

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:   Wed, 06 Sep 2017 19:02:49 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jesper Dangaard Brouer <brouer@...hat.com>, netdev@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>
CC:     John Fastabend <john.fastabend@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>
Subject: Re: [PATCH net-next] xdp: implement xdp_redirect_map for generic
 XDP

On 09/06/2017 06:24 PM, Daniel Borkmann wrote:
[...]
> Otherwise it looks good, but note that it also doesn't really
> resolve the issue you mention wrt stale map pointers by the
> way. This would need a different way to clear out the pointers
> from redirect_info, I'm thinking when we have devmap dismantle
> time after RCU grace period we should check whether there are
> still stale pointers from this map around and clear them under
> disabled preemption, but need to brainstorm a bit more on that
> first.

Scratch that approach, doesn't work. So thinking bit more on
this, what we could do here is the following: verifier knows
we called bpf_xdp_redirect_map() helper, so it could do a small
insn rewrite in the sense that it fills R4 with a pointer to
the bpf_prog. We have that at verification time anyway and R4
is allowed to be populated since we scratch it per convention.
Then, the helper would store the prog pointer in struct redirect_info.
Later in xdp_do_*_redirect() we check whether the redirect_info's
prog pointer is the same as passed xdp_prog pointer, and if
that's the case then all good, since the prog holds a ref on
the map anyway, if they are not equal in the unlikely case, it
means stale pointer, so we bail out right there. That would
work imo, will see to code it up and check it out.