netdev - Re: [iovisor-dev] bpf_redirect_map not working after tail call

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] [day] [month] [year] [list]

Date:   Mon, 4 Jun 2018 14:21:22 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jesper Dangaard Brouer <brouer@...hat.com>,
        iovisor-dev <iovisor-dev@...ts.iovisor.org>,
        Daniel Borkmann <borkmann@...earbox.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [iovisor-dev] bpf_redirect_map not working after tail call

On 06/04/2018 01:04 PM, Jesper Dangaard Brouer via iovisor-dev wrote:
> On Fri, 1 Jun 2018 14:15:58 +0200
> Sebastiano Miano via iovisor-dev <iovisor-dev@...ts.iovisor.org> wrote:
> 
>> Dear all,
>>
>> We have noticed that the bpf_redirect_map returns an error when it is
>> called after a tail call.
>> The xdp_redirect_map program (under sample/bpf) works fine, but if we
>> modify it as shown in the following diff, it doesn't work anymore.
>> I have debugged it with the xdp_monitor application and the error
>> returned is EFAULT.
>> Is this a known issue? Am I doing something wrong?
> 
> Argh, this is likely an issue/bug due to the check xdp_map_invalid(),
> that was introduced in commit 7c3001313396 ("bpf: fix ri->map_owner
> pointer on bpf_prog_realloc").
> 
> To Daniel, I don't know how to solve this, could you give some advice?
> 
> 
> 
>  static inline bool xdp_map_invalid(const struct bpf_prog *xdp_prog,
> 				   unsigned long aux)
>  {
> 	return (unsigned long)xdp_prog->aux != aux;
>  }
> 
>  static int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
> 			       struct bpf_prog *xdp_prog)
>  {
> 	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
> 	unsigned long map_owner = ri->map_owner;
> 	struct bpf_map *map = ri->map;
> 	u32 index = ri->ifindex;
> 	void *fwd = NULL;
> 	int err;
> 
> 	[...]
> 	if (unlikely(xdp_map_invalid(xdp_prog, map_owner))) {
> 		err = -EFAULT;
> 		map = NULL;
> 		goto err;
> 	}
> 	[...]

Argh, I see the issue. Working on a fix after checking the syzkaller reports.

Thanks for the report!