netdev - Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 22 Feb 2019 13:37:34 -0800
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>
Subject: Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT
 to a device

On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:
> Jakub Kicinski <jakub.kicinski@...ronome.com> writes:
> > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:  
> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >> index b63bc77af2d1..629661db36ee 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -7527,6 +7527,12 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env)
> >>  			prog->dst_needed = 1;
> >>  		if (insn->imm == BPF_FUNC_get_prandom_u32)
> >>  			bpf_user_rnd_init_once();
> >> +		if (insn->imm == BPF_FUNC_redirect) {
> >> +			int err = dev_map_alloc_default_map();
> >> +
> >> +			if (err)
> >> +				return err;
> >> +		}
> >>  		if (insn->imm == BPF_FUNC_override_return)
> >>  			prog->kprobe_override = 1;
> >>  		if (insn->imm == BPF_FUNC_tail_call) {  
> >  
> >> +int dev_map_alloc_default_map(void)
> >> +{
> >> +	struct net *net = current->nsproxy->net_ns;
> >> +	struct bpf_dtab *dtab, *old_dtab;
> >> +	struct net_device *netdev;
> >> +	union bpf_attr attr = {};
> >> +	u32 idx;
> >> +	int err;  
> >
> > BPF programs don't obey by netns boundaries.  The fact the program is
> > verified in one ns doesn't mean this is the only ns it will be used in :(
> > Meaning if any program is using the redirect map you may need a secret
> > map in every ns.. no?  
> 
> Ah, yes, good point. Totally didn't think about the fact that load and
> attach are decoupled. Hmm, guess I'll just have to move the call to
> alloc_default_map() to the point where the program is attached to an
> interface, then...

Possibly.. and you also need to handle the case where interface with a
program attached is moved, no?

> I trust it's safe to skip the allocation in case the program is
> offloaded to hardware, right? I.e., an offloaded program will never need
> to fall back to the kernel helper?

We will cross that bridge when we get there ;)  I'd definitely want the
ability to do a redirect to a non-offloaded netdev (e.g. redirect to a
veth) via some fallback, but the plan is to try to only add support for
the map version of redirect on offload, anyway.