netdev - Re: Accessing XDP packet memory from the end

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]

Message-ID: <87czhagxuw.fsf@toke.dk>
Date:   Thu, 21 Apr 2022 19:17:11 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Larysa Zaremba <larysa.zaremba@...el.com>,
        bpf <bpf@...r.kernel.org>
Cc:     Larysa Zaremba <larysa.zaremba@...el.com>,
        netdev <netdev@...r.kernel.org>,
        Andrii Nakryiko <andrii@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Magnus Karlsson <magnus.karlsson@...el.com>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
        Alexander Lobakin <alexandr.lobakin@...el.com>
Subject: Re: Accessing XDP packet memory from the end

Larysa Zaremba <larysa.zaremba@...el.com> writes:

> Dear all,
> Our team has encountered a need of accessing data_meta in a following way:
>
> int xdp_meta_prog(struct xdp_md *ctx)
> {
> 	void *data_meta_ptr = (void *)(long)ctx->data_meta;
> 	void *data_end = (void *)(long)ctx->data_end;
> 	void *data = (void *)(long)ctx->data;
> 	u64 data_size = sizeof(u32);
> 	u32 magic_meta;
> 	u8 offset;
>
> 	offset = (u8)((s64)data - (s64)data_meta_ptr);
> 	if (offset < data_size) {
> 		bpf_printk("invalid offset: %ld\n", offset);
> 		return XDP_DROP;
> 	}
>
> 	data_meta_ptr += offset;
> 	data_meta_ptr -= data_size;
>
> 	if (data_meta_ptr + data_size > data) {
> 		return XDP_DROP;
> 	}
> 		
> 	magic_meta = *((u32 *)data);
> 	bpf_printk("Magic: %d\n", magic_meta);
> 	return XDP_PASS;
> }
>
> Unfortunately, verifier claims this code attempts to access packet with
> an offset of -2 (a constant part) and negative offset is generally forbidden.
>
> For now we have 2 solutions, one is using bpf_xdp_adjust_meta(),
> which is pretty good, but not ideal for the hot path.
> The second one is the patch at the end.
>
> Do you see any other way of accessing memory from the end of data_meta/data?
> What do you think about both suggested solutions?

The problem is that the compiler is generating code that the verifier
doesn't understand. It's notoriously hard to get LLVM to produce code
that preserves the right bounds checks which is why projects like Cilium
use helpers with inline ASM to produce the right loads, like in [0].

Adapting that cilium helper to load from the metadata area, your example
can be rewritten as follows (which works just fine with no verifier
changes):

static __always_inline int
xdp_load_meta_bytes(const struct xdp_md *ctx, __u64 off, void *to, const __u64 len)
{
	void *from;
	int ret;
	/* LLVM tends to generate code that verifier doesn't understand,
	 * so force it the way we want it in order to open up a range
	 * on the reg.
	 */
	asm volatile("r1 = *(u32 *)(%[ctx] +8)\n\t"
		     "r2 = *(u32 *)(%[ctx] +0)\n\t"
		     "%[off] &= %[offmax]\n\t"
		     "r1 += %[off]\n\t"
		     "%[from] = r1\n\t"
		     "r1 += %[len]\n\t"
		     "if r1 > r2 goto +2\n\t"
		     "%[ret] = 0\n\t"
		     "goto +1\n\t"
		     "%[ret] = %[errno]\n\t"
		     : [ret]"=r"(ret), [from]"=r"(from)
		     : [ctx]"r"(ctx), [off]"r"(off), [len]"ri"(len),
		       [offmax]"i"(__CTX_OFF_MAX), [errno]"i"(-EINVAL)
		     : "r1", "r2");
	if (!ret)
		__builtin_memcpy(to, from, len);
	return ret;
}


SEC("xdp")
int xdp_meta_prog(struct xdp_md *ctx)
{
        void *data_meta_ptr = (void *)(long)ctx->data_meta;
        void *data = (void *)(long)ctx->data;
        __u32 magic_meta;
        __u8 offset;
	int ret;

        offset = (__u8)((__s64)data - (__s64)data_meta_ptr);
	ret = xdp_load_meta_bytes(ctx, offset - 4, &magic_meta, sizeof(magic_meta));
	if (ret) {
		bpf_printk("load bytes failed: %d\n", ret);
                return XDP_DROP;
	}

        bpf_printk("Magic: %d\n", magic_meta);
        return XDP_PASS;
}

-Toke


[0] https://github.com/cilium/cilium/blob/master/bpf/include/bpf/ctx/xdp.h#L35