netdev - Re: [PATCH bpf-next v4 2/6] bpf: introduce BPF dispatcher

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:   Thu, 12 Dec 2019 21:30:55 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Björn Töpel <bjorn.topel@...il.com>
Cc:     netdev@...r.kernel.org, ast@...nel.org, daniel@...earbox.net,
        Björn Töpel <bjorn.topel@...el.com>,
        bpf@...r.kernel.org, magnus.karlsson@...il.com,
        magnus.karlsson@...el.com, jonathan.lemon@...il.com,
        ecree@...arflare.com, thoiland@...hat.com, brouer@...hat.com,
        andrii.nakryiko@...il.com
Subject: Re: [PATCH bpf-next v4 2/6] bpf: introduce BPF dispatcher

On Wed, Dec 11, 2019 at 01:30:13PM +0100, Björn Töpel wrote:
> +
> +#define DEFINE_BPF_DISPATCHER(name)					\
> +	unsigned int name##func(					\
> +		const void *xdp_ctx,					\
> +		const struct bpf_insn *insnsi,				\
> +		unsigned int (*bpf_func)(const void *,			\
> +					 const struct bpf_insn *))	\
> +	{								\
> +		return bpf_func(xdp_ctx, insnsi);			\
> +	}								\
> +	EXPORT_SYMBOL(name##func);			\
> +	struct bpf_dispatcher name = BPF_DISPATCHER_INIT(name);

The dispatcher function is a normal function. EXPORT_SYMBOL doesn't make it
'noinline'. struct bpf_dispatcher takes a pointer to it and that address is
used for text_poke.

In patch 3 __BPF_PROG_RUN calls dfunc() from two places.
What stops compiler from inlining it? Or partially inlining it in one
or the other place?

I guess it works, because your compiler didn't inline it?
Could you share how asm looks for bpf_prog_run_xdp()
(where __BPF_PROG_RUN is called) and asm for name##func() ?

I hope my guess that compiler didn't inline it is correct. Then extra noinline
will not hurt and that's the only thing needed to avoid the issue.