Skip to content

Fix dead_code in rust KParamGuard. #448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: rust
Choose a base branch
from
Open

Fix dead_code in rust KParamGuard. #448

wants to merge 1 commit into from

Conversation

kloenk
Copy link
Member

@kloenk kloenk commented Jul 16, 2021

Noticed as part of #439 With disabled CONFIG_SYSFS

@kloenk kloenk added • lib Related to the `rust/` library. • misc Related to other topics (e.g. CI). labels Jul 16, 2021
@alex
Copy link
Member

alex commented Jul 16, 2021

Would it make more sense to put it behind a #[cfg]?

@kloenk
Copy link
Member Author

kloenk commented Jul 16, 2021

I don't think so. It makes the api the same, and so easier to handle. Just did not want a compiler warning

@bjorn3
Copy link
Member

bjorn3 commented Jul 16, 2021

Maybe put just the #[allow] behind a #[cfg]? Also why does that lint fire at all? The type is exported from the kernel crate, right?

@kloenk
Copy link
Member Author

kloenk commented Jul 16, 2021

I think because the internal type that is note used without the cfg.

@kloenk
Copy link
Member Author

kloenk commented Jul 17, 2021

I will remove the inner type, and replace it with a Zero Sized type. this has the advantage, that it is not creating a lint problem, but also the interface to the kernel crate stays the same with or without the CONFIG

@kloenk kloenk force-pushed the KParamGuard-dead_code branch 2 times, most recently from aaf5d63 to e632fe9 Compare July 18, 2021 09:26
bjorn3
bjorn3 reviewed Jul 18, 2021
View reviewed changes
@nbdd0121
Copy link
Member

This makes the KParamGuard's generics arity different depending on config, making it impossible to use with Rust 2018 idiom enforced.

@kloenk
Copy link
Member Author

kloenk commented Jul 19, 2021

@nbdd0121 does that mean it would be better to just add the allow dead_code if not CONFIG_SYSFS and be done with it?

@nbdd0121
Copy link
Member

Either that or just touch it using let _ = .... As @bjorn3 mentioned a Drop impl for KParamGuard is desirable anyway so you can just touch unused fields in drop.

@kloenk kloenk force-pushed the KParamGuard-dead_code branch from e632fe9 to 4b191e1 Compare July 23, 2021 12:12
@kloenk kloenk requested a review from bjorn3 July 23, 2021 12:13
bjorn3
bjorn3 reviewed Jul 23, 2021
View reviewed changes
@kloenk kloenk force-pushed the KParamGuard-dead_code branch from 4b191e1 to 70720d8 Compare July 25, 2021 08:29
@kloenk kloenk requested a review from bjorn3 July 25, 2021 08:29
nbdd0121
nbdd0121 reviewed Jul 25, 2021
View reviewed changes
@kloenk kloenk force-pushed the KParamGuard-dead_code branch from 70720d8 to c8fcdcf Compare August 27, 2021 11:28
@kloenk kloenk requested a review from nbdd0121 November 25, 2021 08:06
ojeda
ojeda reviewed Jan 25, 2022
View reviewed changes
@kloenk
Fix dead_code in rust KParamGuard.
6b55976 
Signed-off-by: Finn Behrens <me@kloenk.de>
@kloenk kloenk force-pushed the KParamGuard-dead_code branch from c8fcdcf to 6b55976 Compare January 27, 2022 12:11
@kloenk kloenk requested a review from ojeda January 27, 2022 12:11
ojeda pushed a commit that referenced this pull request Jun 11, 2024
@akpm00
vmalloc: check CONFIG_EXECMEM in is_vmalloc_or_module_addr()
0105eaa 
After commit 2c9e5d4 ("bpf: remove CONFIG_BPF_JIT dependency on
CONFIG_MODULES of") CONFIG_BPF_JIT does not depend on CONFIG_MODULES any
more and bpf jit also uses the [MODULES_VADDR, MODULES_END] memory region.
But is_vmalloc_or_module_addr() still checks CONFIG_MODULES, which then
returns false for a bpf jit memory region when CONFIG_MODULES is not
defined.  It leads to the following kernel BUG:

[    1.567023] ------------[ cut here ]------------
[    1.567883] kernel BUG at mm/vmalloc.c:745!
[    1.568477] Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN NOPTI
[    1.569367] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.9.0+ #448
[    1.570247] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.15.0-1 04/01/2014
[    1.570786] RIP: 0010:vmalloc_to_page+0x48/0x1ec
[    1.570786] Code: 0f 00 00 e8 eb 1a 05 00 b8 37 00 00 00 48 ba fe ff ff ff ff 1f 00 00 4c 03 25 76 49 c6 02 48 c1 e0 28 48 01 e8 48 39 d0 76 02 <0f> 0b 4c 89 e7 e8 bf 1a 05 00 49 8b 04 24 48 a9 9f ff ff ff 0f 84
[    1.570786] RSP: 0018:ffff888007787960 EFLAGS: 00010212
[    1.570786] RAX: 000036ffa0000000 RBX: 0000000000000640 RCX: ffffffff8147e93c
[    1.570786] RDX: 00001ffffffffffe RSI: dffffc0000000000 RDI: ffffffff840e32c8
[    1.570786] RBP: ffffffffa0000000 R08: 0000000000000000 R09: 0000000000000000
[    1.570786] R10: ffff888007787a88 R11: ffffffff8475d8e7 R12: ffffffff83e80ff8
[    1.570786] R13: 0000000000000640 R14: 0000000000000640 R15: 0000000000000640
[    1.570786] FS:  0000000000000000(0000) GS:ffff88806cc00000(0000) knlGS:0000000000000000
[    1.570786] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    1.570786] CR2: ffff888006a01000 CR3: 0000000003e80000 CR4: 0000000000350ef0
[    1.570786] Call Trace:
[    1.570786]  <TASK>
[    1.570786]  ? __die_body+0x1b/0x58
[    1.570786]  ? die+0x31/0x4b
[    1.570786]  ? do_trap+0x9d/0x138
[    1.570786]  ? vmalloc_to_page+0x48/0x1ec
[    1.570786]  ? do_error_trap+0xcd/0x102
[    1.570786]  ? vmalloc_to_page+0x48/0x1ec
[    1.570786]  ? vmalloc_to_page+0x48/0x1ec
[    1.570786]  ? handle_invalid_op+0x2f/0x38
[    1.570786]  ? vmalloc_to_page+0x48/0x1ec
[    1.570786]  ? exc_invalid_op+0x2b/0x41
[    1.570786]  ? asm_exc_invalid_op+0x16/0x20
[    1.570786]  ? vmalloc_to_page+0x26/0x1ec
[    1.570786]  ? vmalloc_to_page+0x48/0x1ec
[    1.570786]  __text_poke+0xb6/0x458
[    1.570786]  ? __pfx_text_poke_memcpy+0x10/0x10
[    1.570786]  ? __pfx___mutex_lock+0x10/0x10
[    1.570786]  ? __pfx___text_poke+0x10/0x10
[    1.570786]  ? __pfx_get_random_u32+0x10/0x10
[    1.570786]  ? srso_return_thunk+0x5/0x5f
[    1.570786]  text_poke_copy_locked+0x70/0x84
[    1.570786]  text_poke_copy+0x32/0x4f
[    1.570786]  bpf_arch_text_copy+0xf/0x27
[    1.570786]  bpf_jit_binary_pack_finalize+0x26/0x5a
[    1.570786]  bpf_int_jit_compile+0x576/0x8ad
[    1.570786]  ? __pfx_bpf_int_jit_compile+0x10/0x10
[    1.570786]  ? srso_return_thunk+0x5/0x5f
[    1.570786]  ? __kmalloc_node_track_caller+0x2b5/0x2e0
[    1.570786]  bpf_prog_select_runtime+0x7c/0x199
[    1.570786]  bpf_prepare_filter+0x1e9/0x25b
[    1.570786]  ? __pfx_bpf_prepare_filter+0x10/0x10
[    1.570786]  ? srso_return_thunk+0x5/0x5f
[    1.570786]  ? _find_next_bit+0x29/0x7e
[    1.570786]  bpf_prog_create+0xb8/0xe0
[    1.570786]  ptp_classifier_init+0x75/0xa1
[    1.570786]  ? __pfx_ptp_classifier_init+0x10/0x10
[    1.570786]  ? srso_return_thunk+0x5/0x5f
[    1.570786]  ? register_pernet_subsys+0x36/0x42
[    1.570786]  ? srso_return_thunk+0x5/0x5f
[    1.570786]  sock_init+0x99/0xa3
[    1.570786]  ? __pfx_sock_init+0x10/0x10
[    1.570786]  do_one_initcall+0x104/0x2c4
[    1.570786]  ? __pfx_do_one_initcall+0x10/0x10
[    1.570786]  ? parameq+0x25/0x2d
[    1.570786]  ? rcu_is_watching+0x1c/0x3c
[    1.570786]  ? trace_kmalloc+0x81/0xb2
[    1.570786]  ? srso_return_thunk+0x5/0x5f
[    1.570786]  ? __kmalloc+0x29c/0x2c7
[    1.570786]  ? srso_return_thunk+0x5/0x5f
[    1.570786]  do_initcalls+0xf9/0x123
[    1.570786]  kernel_init_freeable+0x24f/0x289
[    1.570786]  ? __pfx_kernel_init+0x10/0x10
[    1.570786]  kernel_init+0x19/0x13a
[    1.570786]  ret_from_fork+0x24/0x41
[    1.570786]  ? __pfx_kernel_init+0x10/0x10
[    1.570786]  ret_from_fork_asm+0x1a/0x30
[    1.570786]  </TASK>
[    1.570819] ---[ end trace 0000000000000000 ]---
[    1.571463] RIP: 0010:vmalloc_to_page+0x48/0x1ec
[    1.572111] Code: 0f 00 00 e8 eb 1a 05 00 b8 37 00 00 00 48 ba fe ff ff ff ff 1f 00 00 4c 03 25 76 49 c6 02 48 c1 e0 28 48 01 e8 48 39 d0 76 02 <0f> 0b 4c 89 e7 e8 bf 1a 05 00 49 8b 04 24 48 a9 9f ff ff ff 0f 84
[    1.574632] RSP: 0018:ffff888007787960 EFLAGS: 00010212
[    1.575129] RAX: 000036ffa0000000 RBX: 0000000000000640 RCX: ffffffff8147e93c
[    1.576097] RDX: 00001ffffffffffe RSI: dffffc0000000000 RDI: ffffffff840e32c8
[    1.577084] RBP: ffffffffa0000000 R08: 0000000000000000 R09: 0000000000000000
[    1.578077] R10: ffff888007787a88 R11: ffffffff8475d8e7 R12: ffffffff83e80ff8
[    1.578810] R13: 0000000000000640 R14: 0000000000000640 R15: 0000000000000640
[    1.579823] FS:  0000000000000000(0000) GS:ffff88806cc00000(0000) knlGS:0000000000000000
[    1.580992] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    1.581869] CR2: ffff888006a01000 CR3: 0000000003e80000 CR4: 0000000000350ef0
[    1.582800] Kernel panic - not syncing: Fatal exception
[    1.583765] ---[ end Kernel panic - not syncing: Fatal exception ]---

Fix this by checking CONFIG_EXECMEM instead.

Link: https://lkml.kernel.org/r/20240528160838.102223-1-xiyou.wangcong@gmail.com
Fixes: 2c9e5d4 ("bpf: remove CONFIG_BPF_JIT dependency on CONFIG_MODULES of")
Signed-off-by: Cong Wang <cong.wang@bytedance.com>
Acked-by: Mike Rapoport (IBM) <rppt@kernel.org>
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nbdd0121 nbdd0121 Awaiting requested review from nbdd0121

@ojeda ojeda Awaiting requested review from ojeda

1 more reviewer

@bjorn3 bjorn3 bjorn3 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

• lib Related to the `rust/` library. • misc Related to other topics (e.g. CI).

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kloenk @alex @bjorn3 @nbdd0121 @ojeda