Add Rust int param types #87

adamrk · 2021-02-13T05:40:51Z

Adds module param types for i8, i64, isize, and usize. This is done using a trait ModuleParam which will be expanded to work for custom param types as well. Addresses part of #11.

rust/kernel/module_param.rs

rust/Makefile

rust/kernel/module_param.rs

adamrk · 2021-02-26T19:53:28Z

Rebased onto the latest changes. Does this need a second review?

rust/Makefile

rust/kernel/buffer.rs

rust/kernel/module_param.rs

ojeda · 2021-02-27T03:28:08Z

Rebased onto the latest changes.

Yeah sorry, we should have merged this before -- anyway this gives a chance to review the docs; the person who writes some code is the best one that can write the documentation anyway :)

adamrk · 2021-02-27T20:13:47Z

Yeah sorry, we should have merged this before -- anyway this gives a chance to review the docs; the person who writes some code is the best one that can write the documentation anyway :)

No worries.

ojeda · 2021-02-28T07:35:18Z

rust/kernel/buffer.rs

+//! Struct for writing to a pre-allocated buffer with the [`write!`] macro.
+//!
+//! [`write!`]: https://doc.rust-lang.org/core/macro.write.html


Suggested change

//! Struct for writing to a pre-allocated buffer with the [`write!`] macro.

//!

//! [`write!`]: https://doc.rust-lang.org/core/macro.write.html

//! Struct for writing to a pre-allocated buffer with the [`write!`] macro.

Intra-doc links (for core and alloc, not std since we don't use it) should work automatically (or did you try and it was not working?).

Ah, I didn't try it. Was just following the pattern for existing links.

Looks like items in core and alloc link to doc.rust-lang.org even without explicitly setting them. We're not generating our own docs for core and alloc - at least I don't see any rules for them in the Makefile and they don't seem to be generated by make rustdoc.

Yeah, they go to the official docs at the moment, but perhaps we should generate them too. I can look into that. Let me create an issue.

rust/kernel/lib.rs

ojeda · 2021-02-28T07:41:10Z

rust/kernel/module_param.rs

+/// Note that displaying the type in `sysfs` will fail if
+/// [`alloc::string::ToString::to_string`] (as implemented through the
+/// [`core::fmt::Display`] trait) writes more than `kernel::PAGE_SIZE`
+/// bytes (including an additional null terminator).
+///
+/// [`alloc::string::ToString::to_string`]: https://doc.rust-lang.org/alloc/string/trait.ToString.html#tymethod.to_string
+/// [`core::fmt::Display`]: https://doc.rust-lang.org/core/fmt/trait.Display.html


Suggested change

/// Note that displaying the type in `sysfs` will fail if

/// [`alloc::string::ToString::to_string`] (as implemented through the

/// [`core::fmt::Display`] trait) writes more than `kernel::PAGE_SIZE`

/// bytes (including an additional null terminator).

///

/// [`alloc::string::ToString::to_string`]: https://doc.rust-lang.org/alloc/string/trait.ToString.html#tymethod.to_string

/// [`core::fmt::Display`]: https://doc.rust-lang.org/core/fmt/trait.Display.html

/// Note that displaying the type in `sysfs` will fail if

/// [`alloc::string::ToString::to_string`] (as implemented through the

/// [`core::fmt::Display`] trait) writes more than [`PAGE_SIZE`]

/// bytes (including an additional null terminator).

Ditto: i.e. the Rust intra-doc links are found automatically by rustdoc.

I think PAGE_SIZE will also work without kernel::, but please double-check.

Removed the links rust-lang.org, but PAGE_SIZE needs to be referenced as crate::PAGE_SIZE.

Co-authored-by: Miguel Ojeda <[email protected]>

ojeda · 2021-03-03T10:00:30Z

Let's get this in!

Thanks a lot for all the work and enduring the reviews!

During testing of f263a81 ("bpf: Track subprog poke descriptors correctly and fix use-after-free") under various failure conditions, for example, when jit_subprogs() fails and tries to clean up the program to be run under the interpreter, we ran into the following freeze: [...] #127/8 tailcall_bpf2bpf_3:FAIL [...] [ 92.041251] BUG: KASAN: slab-out-of-bounds in ___bpf_prog_run+0x1b9d/0x2e20 [ 92.042408] Read of size 8 at addr ffff88800da67f68 by task test_progs/682 [ 92.043707] [ 92.044030] CPU: 1 PID: 682 Comm: test_progs Tainted: G O 5.13.0-53301-ge6c08cb33a30-dirty #87 [ 92.045542] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1 04/01/2014 [ 92.046785] Call Trace: [ 92.047171] ? __bpf_prog_run_args64+0xc0/0xc0 [ 92.047773] ? __bpf_prog_run_args32+0x8b/0xb0 [ 92.048389] ? __bpf_prog_run_args64+0xc0/0xc0 [ 92.049019] ? ktime_get+0x117/0x130 [...] // few hundred [similar] lines more [ 92.659025] ? ktime_get+0x117/0x130 [ 92.659845] ? __bpf_prog_run_args64+0xc0/0xc0 [ 92.660738] ? __bpf_prog_run_args32+0x8b/0xb0 [ 92.661528] ? __bpf_prog_run_args64+0xc0/0xc0 [ 92.662378] ? print_usage_bug+0x50/0x50 [ 92.663221] ? print_usage_bug+0x50/0x50 [ 92.664077] ? bpf_ksym_find+0x9c/0xe0 [ 92.664887] ? ktime_get+0x117/0x130 [ 92.665624] ? kernel_text_address+0xf5/0x100 [ 92.666529] ? __kernel_text_address+0xe/0x30 [ 92.667725] ? unwind_get_return_address+0x2f/0x50 [ 92.668854] ? ___bpf_prog_run+0x15d4/0x2e20 [ 92.670185] ? ktime_get+0x117/0x130 [ 92.671130] ? __bpf_prog_run_args64+0xc0/0xc0 [ 92.672020] ? __bpf_prog_run_args32+0x8b/0xb0 [ 92.672860] ? __bpf_prog_run_args64+0xc0/0xc0 [ 92.675159] ? ktime_get+0x117/0x130 [ 92.677074] ? lock_is_held_type+0xd5/0x130 [ 92.678662] ? ___bpf_prog_run+0x15d4/0x2e20 [ 92.680046] ? ktime_get+0x117/0x130 [ 92.681285] ? __bpf_prog_run32+0x6b/0x90 [ 92.682601] ? __bpf_prog_run64+0x90/0x90 [ 92.683636] ? lock_downgrade+0x370/0x370 [ 92.684647] ? mark_held_locks+0x44/0x90 [ 92.685652] ? ktime_get+0x117/0x130 [ 92.686752] ? lockdep_hardirqs_on+0x79/0x100 [ 92.688004] ? ktime_get+0x117/0x130 [ 92.688573] ? __cant_migrate+0x2b/0x80 [ 92.689192] ? bpf_test_run+0x2f4/0x510 [ 92.689869] ? bpf_test_timer_continue+0x1c0/0x1c0 [ 92.690856] ? rcu_read_lock_bh_held+0x90/0x90 [ 92.691506] ? __kasan_slab_alloc+0x61/0x80 [ 92.692128] ? eth_type_trans+0x128/0x240 [ 92.692737] ? __build_skb+0x46/0x50 [ 92.693252] ? bpf_prog_test_run_skb+0x65e/0xc50 [ 92.693954] ? bpf_prog_test_run_raw_tp+0x2d0/0x2d0 [ 92.694639] ? __fget_light+0xa1/0x100 [ 92.695162] ? bpf_prog_inc+0x23/0x30 [ 92.695685] ? __sys_bpf+0xb40/0x2c80 [ 92.696324] ? bpf_link_get_from_fd+0x90/0x90 [ 92.697150] ? mark_held_locks+0x24/0x90 [ 92.698007] ? lockdep_hardirqs_on_prepare+0x124/0x220 [ 92.699045] ? finish_task_switch+0xe6/0x370 [ 92.700072] ? lockdep_hardirqs_on+0x79/0x100 [ 92.701233] ? finish_task_switch+0x11d/0x370 [ 92.702264] ? __switch_to+0x2c0/0x740 [ 92.703148] ? mark_held_locks+0x24/0x90 [ 92.704155] ? __x64_sys_bpf+0x45/0x50 [ 92.705146] ? do_syscall_64+0x35/0x80 [ 92.706953] ? entry_SYSCALL_64_after_hwframe+0x44/0xae [...] Turns out that the program rejection from e411901 ("bpf: allow for tailcalls in BPF subprograms for x64 JIT") is buggy since env->prog->aux->tail_call_reachable is never true. Commit ebf7d1f ("bpf, x64: rework pro/epilogue and tailcall handling in JIT") added a tracker into check_max_stack_depth() which propagates the tail_call_reachable condition throughout the subprograms. This info is then assigned to the subprogram's func[i]->aux->tail_call_reachable. However, in the case of the rejection check upon JIT failure, env->prog->aux->tail_call_reachable is used. func[0]->aux->tail_call_reachable which represents the main program's information did not propagate this to the outer env->prog->aux, though. Add this propagation into check_max_stack_depth() where it needs to belong so that the check can be done reliably. Fixes: ebf7d1f ("bpf, x64: rework pro/epilogue and tailcall handling in JIT") Fixes: e411901 ("bpf: allow for tailcalls in BPF subprograms for x64 JIT") Co-developed-by: John Fastabend <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]> Signed-off-by: John Fastabend <[email protected]> Signed-off-by: Alexei Starovoitov <[email protected]> Acked-by: Maciej Fijalkowski <[email protected]> Link: https://lore.kernel.org/bpf/618c34e3163ad1a36b1e82377576a6081e182f25.1626123173.git.daniel@iogearbox.net

If open_cached_dir() encounters an error parsing the lease from the server, the error handling may race with receiving a lease break, resulting in open_cached_dir() freeing the cfid while the queued work is pending. Update open_cached_dir() to drop refs rather than directly freeing the cfid. Have cached_dir_lease_break(), cfids_laundromat_worker(), and invalidate_all_cached_dirs() clear has_lease immediately while still holding cfids->cfid_list_lock, and then use this to also simplify the reference counting in cfids_laundromat_worker() and invalidate_all_cached_dirs(). Fixes this KASAN splat (which manually injects an error and lease break in open_cached_dir()): ================================================================== BUG: KASAN: slab-use-after-free in smb2_cached_lease_break+0x27/0xb0 Read of size 8 at addr ffff88811cc24c10 by task kworker/3:1/65 CPU: 3 UID: 0 PID: 65 Comm: kworker/3:1 Not tainted 6.12.0-rc6-g255cf264e6e5-dirty #87 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 11/12/2020 Workqueue: cifsiod smb2_cached_lease_break Call Trace: <TASK> dump_stack_lvl+0x77/0xb0 print_report+0xce/0x660 kasan_report+0xd3/0x110 smb2_cached_lease_break+0x27/0xb0 process_one_work+0x50a/0xc50 worker_thread+0x2ba/0x530 kthread+0x17c/0x1c0 ret_from_fork+0x34/0x60 ret_from_fork_asm+0x1a/0x30 </TASK> Allocated by task 2464: kasan_save_stack+0x33/0x60 kasan_save_track+0x14/0x30 __kasan_kmalloc+0xaa/0xb0 open_cached_dir+0xa7d/0x1fb0 smb2_query_path_info+0x43c/0x6e0 cifs_get_fattr+0x346/0xf10 cifs_get_inode_info+0x157/0x210 cifs_revalidate_dentry_attr+0x2d1/0x460 cifs_getattr+0x173/0x470 vfs_statx_path+0x10f/0x160 vfs_statx+0xe9/0x150 vfs_fstatat+0x5e/0xc0 __do_sys_newfstatat+0x91/0xf0 do_syscall_64+0x95/0x1a0 entry_SYSCALL_64_after_hwframe+0x76/0x7e Freed by task 2464: kasan_save_stack+0x33/0x60 kasan_save_track+0x14/0x30 kasan_save_free_info+0x3b/0x60 __kasan_slab_free+0x51/0x70 kfree+0x174/0x520 open_cached_dir+0x97f/0x1fb0 smb2_query_path_info+0x43c/0x6e0 cifs_get_fattr+0x346/0xf10 cifs_get_inode_info+0x157/0x210 cifs_revalidate_dentry_attr+0x2d1/0x460 cifs_getattr+0x173/0x470 vfs_statx_path+0x10f/0x160 vfs_statx+0xe9/0x150 vfs_fstatat+0x5e/0xc0 __do_sys_newfstatat+0x91/0xf0 do_syscall_64+0x95/0x1a0 entry_SYSCALL_64_after_hwframe+0x76/0x7e Last potentially related work creation: kasan_save_stack+0x33/0x60 __kasan_record_aux_stack+0xad/0xc0 insert_work+0x32/0x100 __queue_work+0x5c9/0x870 queue_work_on+0x82/0x90 open_cached_dir+0x1369/0x1fb0 smb2_query_path_info+0x43c/0x6e0 cifs_get_fattr+0x346/0xf10 cifs_get_inode_info+0x157/0x210 cifs_revalidate_dentry_attr+0x2d1/0x460 cifs_getattr+0x173/0x470 vfs_statx_path+0x10f/0x160 vfs_statx+0xe9/0x150 vfs_fstatat+0x5e/0xc0 __do_sys_newfstatat+0x91/0xf0 do_syscall_64+0x95/0x1a0 entry_SYSCALL_64_after_hwframe+0x76/0x7e The buggy address belongs to the object at ffff88811cc24c00 which belongs to the cache kmalloc-1k of size 1024 The buggy address is located 16 bytes inside of freed 1024-byte region [ffff88811cc24c00, ffff88811cc25000) Cc: [email protected] Signed-off-by: Paul Aurich <[email protected]> Signed-off-by: Steve French <[email protected]>

ojeda reviewed Feb 13, 2021

View reviewed changes

rust/kernel/module_param.rs Outdated Show resolved Hide resolved

rust/Makefile Outdated Show resolved Hide resolved

adamrk force-pushed the rust-int-params branch from 1f96a0f to eef24ea Compare February 15, 2021 17:28

ojeda reviewed Feb 16, 2021

View reviewed changes

rust/kernel/module_param.rs Outdated Show resolved Hide resolved

rust/kernel/module_param.rs Outdated Show resolved Hide resolved

ojeda reviewed Feb 16, 2021

View reviewed changes

rust/kernel/module_param.rs Outdated Show resolved Hide resolved

adamrk force-pushed the rust-int-params branch from 33001ee to dc30d16 Compare February 17, 2021 20:47

ojeda reviewed Feb 17, 2021

View reviewed changes

rust/kernel/module_param.rs Outdated Show resolved Hide resolved

rust/kernel/module_param.rs Outdated Show resolved Hide resolved

adamrk force-pushed the rust-int-params branch from a9ce588 to 6c9036e Compare February 18, 2021 08:05

ojeda approved these changes Feb 18, 2021

View reviewed changes

adamrk force-pushed the rust-int-params branch from 6c9036e to 947bebf Compare February 26, 2021 19:51

adamrk added 3 commits February 26, 2021 21:04

Add ModuleParam trait and new int params

4021d94

Add usize param to examples

0fbeb9f

Add NO_ARG and bool ops

901cd01

adamrk force-pushed the rust-int-params branch from 947bebf to 901cd01 Compare February 26, 2021 20:04

adamrk added 2 commits February 26, 2021 21:55

Add docs

8c8b479

rustfmt

923540b

ojeda reviewed Feb 27, 2021

View reviewed changes

Improve docs

edf5833

adamrk force-pushed the rust-int-params branch from 65fe9b0 to edf5833 Compare February 27, 2021 20:09

clean up export symbol regex

fce463d

ojeda reviewed Feb 28, 2021

View reviewed changes

adamrk and others added 3 commits March 2, 2021 21:45

remove doc.rust-lang.org links

5d8e7af

fix PAGE_SIZE doc link

41325ef

add link to PAGE_SHIFT in C

3ec22aa

Co-authored-by: Miguel Ojeda <[email protected]>

ojeda merged commit 304ee69 into Rust-for-Linux:rust Mar 3, 2021

ojeda mentioned this pull request Mar 8, 2021

Complete kernel params support #11

Open

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Rust int param types #87

Add Rust int param types #87

adamrk commented Feb 13, 2021 •

edited

Loading

adamrk commented Feb 26, 2021

ojeda commented Feb 27, 2021 •

edited

Loading

adamrk commented Feb 27, 2021

ojeda Feb 28, 2021

adamrk Mar 2, 2021

ojeda Mar 3, 2021

ojeda Mar 3, 2021

ojeda Feb 28, 2021 •

edited

Loading

adamrk Mar 2, 2021

ojeda commented Mar 3, 2021

Add Rust int param types #87

Add Rust int param types #87

Conversation

adamrk commented Feb 13, 2021 • edited Loading

adamrk commented Feb 26, 2021

ojeda commented Feb 27, 2021 • edited Loading

adamrk commented Feb 27, 2021

ojeda Feb 28, 2021

Choose a reason for hiding this comment

adamrk Mar 2, 2021

Choose a reason for hiding this comment

ojeda Mar 3, 2021

Choose a reason for hiding this comment

ojeda Mar 3, 2021

Choose a reason for hiding this comment

ojeda Feb 28, 2021 • edited Loading

Choose a reason for hiding this comment

adamrk Mar 2, 2021

Choose a reason for hiding this comment

ojeda commented Mar 3, 2021

adamrk commented Feb 13, 2021 •

edited

Loading

ojeda commented Feb 27, 2021 •

edited

Loading

ojeda Feb 28, 2021 •

edited

Loading