linux-kernel - Re: [PATCH bpf v2 2/2] bpf: ensure main program has an extable

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: <33c52b4c-2a16-7578-f782-51267deff750@meta.com>
Date:   Thu, 8 Jun 2023 10:38:12 -0700
From:   Yonghong Song <yhs@...a.com>
To:     Krister Johansen <kjlx@...pleofstupid.com>, bpf@...r.kernel.org
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Song Liu <song@...nel.org>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,
        Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        stable@...r.kernel.org
Subject: Re: [PATCH bpf v2 2/2] bpf: ensure main program has an extable



On 6/7/23 2:04 PM, Krister Johansen wrote:
> When bpf subprograms are in use, the main program is not jit'd after the
> subprograms because jit_subprogs sets a value for prog->bpf_func upon
> success.  Subsequent calls to the JIT are bypassed when this value is
> non-NULL.  This leads to a situation where the main program and its
> func[0] counterpart are both in the bpf kallsyms tree, but only func[0]
> has an extable.  Extables are only created during JIT.  Now there are
> two nearly identical program ksym entries in the tree, but only one has
> an extable.  Depending upon how the entries are placed, there's a chance
> that a fault will call search_extable on the aux with the NULL entry.
> 
> Since jit_subprogs already copies state from func[0] to the main
> program, include the extable pointer in this state duplication.  The
> alternative is to skip adding the main program to the bpf_kallsyms
> table, but that would mean adding a check for subprograms into the
> middle of bpf_prog_load.

I think having two early identical program ksym entries is bad.
When people 'cat /proc/kallsyms | grep <their program name>',
they will find two programs with identical kernel address but different
hash value. This is just very confusing. I think removing the
duplicate in kallsyms is better from user's perspective.

> 
> Cc: stable@...r.kernel.org
> Fixes: 1c2a088a6626 ("bpf: x64: add JIT support for multi-function programs")
> Signed-off-by: Krister Johansen <kjlx@...pleofstupid.com>
> ---
>   kernel/bpf/verifier.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5871aa78d01a..d6939db9fbf9 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17242,6 +17242,7 @@ static int jit_subprogs(struct bpf_verifier_env *env)
>   	prog->jited = 1;
>   	prog->bpf_func = func[0]->bpf_func;
>   	prog->jited_len = func[0]->jited_len;
> +	prog->aux->extable = func[0]->aux->extable;
>   	prog->aux->func = func;
>   	prog->aux->func_cnt = env->subprog_cnt;
>   	bpf_prog_jit_attempt_done(prog);