public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] bpftool: un-const bpf_func_info to fix it for llvm 17 and newer
@ 2024-05-20 22:51 Ivan Babrou
  2024-05-20 23:02 ` Nick Desaulniers
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ivan Babrou @ 2024-05-20 22:51 UTC (permalink / raw)
  To: bpf
  Cc: linux-kernel, kernel-team, Ivan Babrou, Quentin Monnet,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Nathan Chancellor, Nick Desaulniers, Tom Rix, Raman Shukhau

LLVM 17 started treating const structs as constants:

* https://github.com/llvm/llvm-project/commit/0b2d5b967d98

Combined with pointer laundering via ptr_to_u64, which takes a const ptr,
but in reality treats the underlying memory as mutable, this makes clang
always pass zero to btf__type_by_id, which breaks full name resolution.

Disassembly before (LLVM 16) and after (LLVM 17):

    -    8b 75 cc                 mov    -0x34(%rbp),%esi
    -    e8 47 8d 02 00           call   3f5b0 <btf__type_by_id>
    +    31 f6                    xor    %esi,%esi
    +    e8 a9 8c 02 00           call   3f510 <btf__type_by_id>

It's a bigger project to fix this properly (and a question whether LLVM
itself should detect this), but for right now let's just fix bpftool.

For more information, see this thread in bpf mailing list:

* https://lore.kernel.org/bpf/CABWYdi0ymezpYsQsPv7qzpx2fWuTkoD1-wG1eT-9x-TSREFrQg@mail.gmail.com/T/

Fixes: b662000aff84 ("bpftool: Adding support for BTF program names")
Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
---
 tools/bpf/bpftool/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index cc6e6aae2447..6d8bbc3ec603 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -338,7 +338,7 @@ void get_prog_full_name(const struct bpf_prog_info *prog_info, int prog_fd,
 {
 	const char *prog_name = prog_info->name;
 	const struct btf_type *func_type;
-	const struct bpf_func_info finfo = {};
+	struct bpf_func_info finfo = {};
 	struct bpf_prog_info info = {};
 	__u32 info_len = sizeof(info);
 	struct btf *prog_btf = NULL;
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] bpftool: un-const bpf_func_info to fix it for llvm 17 and newer
  2024-05-20 22:51 [PATCH] bpftool: un-const bpf_func_info to fix it for llvm 17 and newer Ivan Babrou
@ 2024-05-20 23:02 ` Nick Desaulniers
  2024-05-21  2:12 ` Yonghong Song
  2024-05-21 17:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Nick Desaulniers @ 2024-05-20 23:02 UTC (permalink / raw)
  To: Ivan Babrou
  Cc: bpf, linux-kernel, kernel-team, Quentin Monnet,
	Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Nathan Chancellor, Tom Rix, Raman Shukhau

On Mon, May 20, 2024 at 3:52 PM Ivan Babrou <ivan@cloudflare.com> wrote:
>
> LLVM 17 started treating const structs as constants:
>
> * https://github.com/llvm/llvm-project/commit/0b2d5b967d98
>
> Combined with pointer laundering via ptr_to_u64, which takes a const ptr,
> but in reality treats the underlying memory as mutable, this makes clang
> always pass zero to btf__type_by_id, which breaks full name resolution.
>
> Disassembly before (LLVM 16) and after (LLVM 17):
>
>     -    8b 75 cc                 mov    -0x34(%rbp),%esi
>     -    e8 47 8d 02 00           call   3f5b0 <btf__type_by_id>
>     +    31 f6                    xor    %esi,%esi
>     +    e8 a9 8c 02 00           call   3f510 <btf__type_by_id>
>
> It's a bigger project to fix this properly (and a question whether LLVM
> itself should detect this), but for right now let's just fix bpftool.

Right, looks like we don't currently have UBSan instrumentation for "I
modified an object declared const."  Instead, it just leads to fun
bugs like "why was my write elided" or in this case "why did my
constant folding change between compiler versions?"  Such are the joys
of UB.

Acked-by: Nick Desaulniers <ndesaulniers@google.com>

>
> For more information, see this thread in bpf mailing list:
>
> * https://lore.kernel.org/bpf/CABWYdi0ymezpYsQsPv7qzpx2fWuTkoD1-wG1eT-9x-TSREFrQg@mail.gmail.com/T/
>
> Fixes: b662000aff84 ("bpftool: Adding support for BTF program names")
> Signed-off-by: Ivan Babrou <ivan@cloudflare.com>
> ---
>  tools/bpf/bpftool/common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> index cc6e6aae2447..6d8bbc3ec603 100644
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> @@ -338,7 +338,7 @@ void get_prog_full_name(const struct bpf_prog_info *prog_info, int prog_fd,
>  {
>         const char *prog_name = prog_info->name;
>         const struct btf_type *func_type;
> -       const struct bpf_func_info finfo = {};
> +       struct bpf_func_info finfo = {};
>         struct bpf_prog_info info = {};
>         __u32 info_len = sizeof(info);
>         struct btf *prog_btf = NULL;
> --
> 2.44.0
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bpftool: un-const bpf_func_info to fix it for llvm 17 and newer
  2024-05-20 22:51 [PATCH] bpftool: un-const bpf_func_info to fix it for llvm 17 and newer Ivan Babrou
  2024-05-20 23:02 ` Nick Desaulniers
@ 2024-05-21  2:12 ` Yonghong Song
  2024-05-21 17:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2024-05-21  2:12 UTC (permalink / raw)
  To: Ivan Babrou, bpf
  Cc: linux-kernel, kernel-team, Quentin Monnet, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau, Song Liu,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Nathan Chancellor, Nick Desaulniers, Tom Rix, Raman Shukhau


On 5/20/24 4:51 PM, Ivan Babrou wrote:
> LLVM 17 started treating const structs as constants:
>
> * https://github.com/llvm/llvm-project/commit/0b2d5b967d98
>
> Combined with pointer laundering via ptr_to_u64, which takes a const ptr,
> but in reality treats the underlying memory as mutable, this makes clang
> always pass zero to btf__type_by_id, which breaks full name resolution.
>
> Disassembly before (LLVM 16) and after (LLVM 17):
>
>      -    8b 75 cc                 mov    -0x34(%rbp),%esi
>      -    e8 47 8d 02 00           call   3f5b0 <btf__type_by_id>
>      +    31 f6                    xor    %esi,%esi
>      +    e8 a9 8c 02 00           call   3f510 <btf__type_by_id>
>
> It's a bigger project to fix this properly (and a question whether LLVM
> itself should detect this), but for right now let's just fix bpftool.
>
> For more information, see this thread in bpf mailing list:
>
> * https://lore.kernel.org/bpf/CABWYdi0ymezpYsQsPv7qzpx2fWuTkoD1-wG1eT-9x-TSREFrQg@mail.gmail.com/T/
>
> Fixes: b662000aff84 ("bpftool: Adding support for BTF program names")
> Signed-off-by: Ivan Babrou <ivan@cloudflare.com>

Acked-by: Yonghong Song <yonghong.song@linux.dev>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] bpftool: un-const bpf_func_info to fix it for llvm 17 and newer
  2024-05-20 22:51 [PATCH] bpftool: un-const bpf_func_info to fix it for llvm 17 and newer Ivan Babrou
  2024-05-20 23:02 ` Nick Desaulniers
  2024-05-21  2:12 ` Yonghong Song
@ 2024-05-21 17:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-05-21 17:40 UTC (permalink / raw)
  To: Ivan Babrou
  Cc: bpf, linux-kernel, kernel-team, quentin, ast, daniel, andrii,
	martin.lau, song, yonghong.song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa, nathan, ndesaulniers, trix, ramasha

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Mon, 20 May 2024 15:51:49 -0700 you wrote:
> LLVM 17 started treating const structs as constants:
> 
> * https://github.com/llvm/llvm-project/commit/0b2d5b967d98
> 
> Combined with pointer laundering via ptr_to_u64, which takes a const ptr,
> but in reality treats the underlying memory as mutable, this makes clang
> always pass zero to btf__type_by_id, which breaks full name resolution.
> 
> [...]

Here is the summary with links:
  - bpftool: un-const bpf_func_info to fix it for llvm 17 and newer
    https://git.kernel.org/bpf/bpf-next/c/f4aba3471cfb

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-05-21 17:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-20 22:51 [PATCH] bpftool: un-const bpf_func_info to fix it for llvm 17 and newer Ivan Babrou
2024-05-20 23:02 ` Nick Desaulniers
2024-05-21  2:12 ` Yonghong Song
2024-05-21 17:40 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox