Skip to content
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

Move unfit code out of rust/kernel/types.rs #1117

Closed
y86-dev opened this issue Sep 13, 2024 · 5 comments
Closed

Move unfit code out of rust/kernel/types.rs #1117

y86-dev opened this issue Sep 13, 2024 · 5 comments
Assignees
@plinkr
Labels
good first issue Good for newcomers

Comments

@y86-dev
Copy link
Member

y86-dev commented Sep 13, 2024 •
edited
Loading

rust/kernel/types.rs is a rather non-descriptive, general file that contains code that might be better suited somewhere else:

  • ARef<T> and AlwaysRefCounted should be in rust/kernel/sync/aref.rs.
  • FromBytes and AsBytes traits and implementations should be in rust/kernel/transmute.rs.

Please create two separate patches for the above. Also take care to fix any imports and rustdoc examples.

This requires submitting a proper patch to the LKML and the Rust for Linux mailing list. Please recall to test your changes (including generating the documentation if changed, running the Rust doctests if changed, etc.), to use a proper title for the commit, to sign your commit under the Developer's Certificate of Origin and to add a Suggested-by: tag and a Link: tag to this issue. Please see https://rust-for-linux.com/contributing for details.

Please take this issue only if you are new to the kernel development process and you would like to use it as a test to submit your first patch to the kernel. Please do not take it if you do not plan to make other contributions to the kernel.
@y86-dev y86-dev added the good first issue Good for newcomers label Sep 13, 2024
@nbdd0121
Copy link
Member

I feel like convert doesn't coney the semantics of these traits are only performing byte-level transmutation. Maybe transmute.rs?

@plinkr
Copy link

plinkr commented Sep 16, 2024

Hey! I would like to give this issue a try if no one is working on it

intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Sep 18, 2024
@plinkr @intel-lab-lkp
rust: Move FromBytes and AsBytes traits to a new transmute module
b2bb147 
This patch refactors the `FromBytes` and `AsBytes` traits from `types.rs`
into a new `transmute.rs` module.
The traits and their implementations remain unchanged.
 - Adds `rust/kernel/transmute.rs` with the definitions of `FromBytes`
    and `AsBytes`
 - Removes the same trait definitions from `rust/kernel/types.rs`
 - Updates `rust/kernel/uaccess.rs` to import `AsBytes` and `FromBytes`
    from `transmute.rs`

Suggested-by: Benno Lossin <benno.lossin@proton.me>

Link: Rust-for-Linux#1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this issue Sep 18, 2024
@plinkr @intel-lab-lkp
rust: Move ARef<T> and AlwaysRefCounted to new sync::aref module
adebc2a 
This patch refactors the `ARef` type and `AlwaysRefCounted` trait from
`types.rs` into a new `sync/aref.rs` module.
The type and trait definitions remain unchanged.
 - Adds `rust/kernel/sync/aref.rs` with the definitions of `ARef` and
    `AlwaysRefCounted`
 - Removes the same type and trait definitions from `rust/kernel/types.rs`
 - Updates relevant files to import `ARef` and `AlwaysRefCounted` from
    `sync/aref.rs`

Suggested-by: Benno Lossin <benno.lossin@proton.me>

Link: Rust-for-Linux#1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
@plinkr
Copy link

plinkr commented Sep 18, 2024

I sent the patches: https://lore.kernel.org/rust-for-linux/20240918225115.2309224-1-aliet.exposito@gmail.com/T/#t
Thank you very much for the opportunity to work on a good first issue

@sergiocollado
Copy link

sergiocollado commented Sep 19, 2024 •
edited
Loading

The patch looks good .... but transmute.rs for me it feels a bit too eldritch and arcane .... convert or convertBytes or something similar sounded better, at least for me. Otherwise looks great!

ojeda pushed a commit that referenced this issue Oct 5, 2024
@plinkr @ojeda
rust: kernel: move FromBytes and AsBytes traits to a new `transmu…
590530e 
…te` module

Refactor the `FromBytes` and `AsBytes` traits from `types.rs` into a new
`transmute.rs` module:

 - Add `rust/kernel/transmute.rs` with the definitions of `FromBytes`
   and `AsBytes`.

 - Remove the same trait definitions from `rust/kernel/types.rs`.

 - Update `rust/kernel/uaccess.rs` to import `AsBytes` and `FromBytes`
   from `transmute.rs`.

The traits and their implementations remain unchanged.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: #1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20240918225115.2309224-2-aliet.exposito@gmail.com
[ Rebased on top of the lints series and slightly reworded. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit that referenced this issue Oct 5, 2024
@plinkr @ojeda
rust: kernel: move ARef<T> and AlwaysRefCounted to new `sync::are…
743c57a 
…f` module

Refactor the `ARef` type and `AlwaysRefCounted` trait from `types.rs`
into a new `sync/aref.rs` module:

 - Add `rust/kernel/sync/aref.rs` with the definitions of `ARef` and
   `AlwaysRefCounted`.

 - Remove the same type and trait definitions from `rust/kernel/types.rs`.

 - Update relevant files to import `ARef` and `AlwaysRefCounted` from
   `sync/aref.rs`.

The type and trait definitions remain unchanged.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: #1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20240918225115.2309224-3-aliet.exposito@gmail.com
[ Rebased on top of the lints series, slightly reworded and removed
  unneeded `use AlwaysRefCounted` in example. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
@ojeda
Copy link
Member

ojeda commented Oct 5, 2024

@sergiocollado I assume it is following std::mem::transmute.

Applied to rust-next -- thanks everyone!

@ojeda ojeda closed this as completed Oct 5, 2024
ojeda pushed a commit that referenced this issue Oct 7, 2024
@plinkr @ojeda
rust: kernel: move FromBytes and AsBytes traits to a new `transmu…
96653f8 
…te` module

Refactor the `FromBytes` and `AsBytes` traits from `types.rs` into a new
`transmute.rs` module:

 - Add `rust/kernel/transmute.rs` with the definitions of `FromBytes`
   and `AsBytes`.

 - Remove the same trait definitions from `rust/kernel/types.rs`.

 - Update `rust/kernel/uaccess.rs` to import `AsBytes` and `FromBytes`
   from `transmute.rs`.

The traits and their implementations remain unchanged.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: #1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20240918225115.2309224-2-aliet.exposito@gmail.com
[ Rebased on top of the lints series and slightly reworded. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit that referenced this issue Oct 7, 2024
@plinkr @ojeda
rust: kernel: move ARef<T> and AlwaysRefCounted to new `sync::are…
38d2e7d 
…f` module

Refactor the `ARef` type and `AlwaysRefCounted` trait from `types.rs`
into a new `sync/aref.rs` module:

 - Add `rust/kernel/sync/aref.rs` with the definitions of `ARef` and
   `AlwaysRefCounted`.

 - Remove the same type and trait definitions from `rust/kernel/types.rs`.

 - Update relevant files to import `ARef` and `AlwaysRefCounted` from
   `sync/aref.rs`.

The type and trait definitions remain unchanged.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: #1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20240918225115.2309224-3-aliet.exposito@gmail.com
[ Rebased on top of the lints series, slightly reworded and removed
  unneeded `use AlwaysRefCounted` in example. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit that referenced this issue Oct 7, 2024
@plinkr @ojeda
rust: kernel: move FromBytes and AsBytes traits to a new `transmu…
67b3fa4 
…te` module

Refactor the `FromBytes` and `AsBytes` traits from `types.rs` into a new
`transmute.rs` module:

 - Add `rust/kernel/transmute.rs` with the definitions of `FromBytes`
   and `AsBytes`.

 - Remove the same trait definitions from `rust/kernel/types.rs`.

 - Update `rust/kernel/uaccess.rs` to import `AsBytes` and `FromBytes`
   from `transmute.rs`.

The traits and their implementations remain unchanged.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: #1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20240918225115.2309224-2-aliet.exposito@gmail.com
[ Rebased on top of the lints series and slightly reworded. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit that referenced this issue Oct 7, 2024
@plinkr @ojeda
rust: kernel: move ARef<T> and AlwaysRefCounted to new `sync::are…
1f0433d 
…f` module

Refactor the `ARef` type and `AlwaysRefCounted` trait from `types.rs`
into a new `sync/aref.rs` module:

 - Add `rust/kernel/sync/aref.rs` with the definitions of `ARef` and
   `AlwaysRefCounted`.

 - Remove the same type and trait definitions from `rust/kernel/types.rs`.

 - Update relevant files to import `ARef` and `AlwaysRefCounted` from
   `sync/aref.rs`.

The type and trait definitions remain unchanged.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: #1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20240918225115.2309224-3-aliet.exposito@gmail.com
[ Rebased on top of the lints series, slightly reworded and removed
  unneeded `use AlwaysRefCounted` in example. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit that referenced this issue Oct 9, 2024
@plinkr @ojeda
rust: kernel: move FromBytes and AsBytes traits to a new `transmu…
ce1c54f 
…te` module

Refactor the `FromBytes` and `AsBytes` traits from `types.rs` into a new
`transmute.rs` module:

 - Add `rust/kernel/transmute.rs` with the definitions of `FromBytes`
   and `AsBytes`.

 - Remove the same trait definitions from `rust/kernel/types.rs`.

 - Update `rust/kernel/uaccess.rs` to import `AsBytes` and `FromBytes`
   from `transmute.rs`.

The traits and their implementations remain unchanged.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: #1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20240918225115.2309224-2-aliet.exposito@gmail.com
[ Rebased on top of the lints series and slightly reworded. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda pushed a commit that referenced this issue Oct 21, 2024
@kuba-moo
net/sched: accept TCA_STAB only for root qdisc
3cb7cf1 
Most qdiscs maintain their backlog using qdisc_pkt_len(skb)
on the assumption it is invariant between the enqueue()
and dequeue() handlers.

Unfortunately syzbot can crash a host rather easily using
a TBF + SFQ combination, with an STAB on SFQ [1]

We can't support TCA_STAB on arbitrary level, this would
require to maintain per-qdisc storage.

[1]
[   88.796496] BUG: kernel NULL pointer dereference, address: 0000000000000000
[   88.798611] #PF: supervisor read access in kernel mode
[   88.799014] #PF: error_code(0x0000) - not-present page
[   88.799506] PGD 0 P4D 0
[   88.799829] Oops: Oops: 0000 [#1] SMP NOPTI
[   88.800569] CPU: 14 UID: 0 PID: 2053 Comm: b371744477 Not tainted 6.12.0-rc1-virtme #1117
[   88.801107] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   88.801779] RIP: 0010:sfq_dequeue (net/sched/sch_sfq.c:272 net/sched/sch_sfq.c:499) sch_sfq
[ 88.802544] Code: 0f b7 50 12 48 8d 04 d5 00 00 00 00 48 89 d6 48 29 d0 48 8b 91 c0 01 00 00 48 c1 e0 03 48 01 c2 66 83 7a 1a 00 7e c0 48 8b 3a <4c> 8b 07 4c 89 02 49 89 50 08 48 c7 47 08 00 00 00 00 48 c7 07 00
All code
========
   0:	0f b7 50 12          	movzwl 0x12(%rax),%edx
   4:	48 8d 04 d5 00 00 00 	lea    0x0(,%rdx,8),%rax
   b:	00
   c:	48 89 d6             	mov    %rdx,%rsi
   f:	48 29 d0             	sub    %rdx,%rax
  12:	48 8b 91 c0 01 00 00 	mov    0x1c0(%rcx),%rdx
  19:	48 c1 e0 03          	shl    $0x3,%rax
  1d:	48 01 c2             	add    %rax,%rdx
  20:	66 83 7a 1a 00       	cmpw   $0x0,0x1a(%rdx)
  25:	7e c0                	jle    0xffffffffffffffe7
  27:	48 8b 3a             	mov    (%rdx),%rdi
  2a:*	4c 8b 07             	mov    (%rdi),%r8		<-- trapping instruction
  2d:	4c 89 02             	mov    %r8,(%rdx)
  30:	49 89 50 08          	mov    %rdx,0x8(%r8)
  34:	48 c7 47 08 00 00 00 	movq   $0x0,0x8(%rdi)
  3b:	00
  3c:	48                   	rex.W
  3d:	c7                   	.byte 0xc7
  3e:	07                   	(bad)
	...

Code starting with the faulting instruction
===========================================
   0:	4c 8b 07             	mov    (%rdi),%r8
   3:	4c 89 02             	mov    %r8,(%rdx)
   6:	49 89 50 08          	mov    %rdx,0x8(%r8)
   a:	48 c7 47 08 00 00 00 	movq   $0x0,0x8(%rdi)
  11:	00
  12:	48                   	rex.W
  13:	c7                   	.byte 0xc7
  14:	07                   	(bad)
	...
[   88.803721] RSP: 0018:ffff9a1f892b7d58 EFLAGS: 00000206
[   88.804032] RAX: 0000000000000000 RBX: ffff9a1f8420c800 RCX: ffff9a1f8420c800
[   88.804560] RDX: ffff9a1f81bc1440 RSI: 0000000000000000 RDI: 0000000000000000
[   88.805056] RBP: ffffffffc04bb0e0 R08: 0000000000000001 R09: 00000000ff7f9a1f
[   88.805473] R10: 000000000001001b R11: 0000000000009a1f R12: 0000000000000140
[   88.806194] R13: 0000000000000001 R14: ffff9a1f886df400 R15: ffff9a1f886df4ac
[   88.806734] FS:  00007f445601a740(0000) GS:ffff9a2e7fd80000(0000) knlGS:0000000000000000
[   88.807225] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   88.807672] CR2: 0000000000000000 CR3: 000000050cc46000 CR4: 00000000000006f0
[   88.808165] Call Trace:
[   88.808459]  <TASK>
[   88.808710] ? __die (arch/x86/kernel/dumpstack.c:421 arch/x86/kernel/dumpstack.c:434)
[   88.809261] ? page_fault_oops (arch/x86/mm/fault.c:715)
[   88.809561] ? exc_page_fault (./arch/x86/include/asm/irqflags.h:26 ./arch/x86/include/asm/irqflags.h:87 ./arch/x86/include/asm/irqflags.h:147 arch/x86/mm/fault.c:1489 arch/x86/mm/fault.c:1539)
[   88.809806] ? asm_exc_page_fault (./arch/x86/include/asm/idtentry.h:623)
[   88.810074] ? sfq_dequeue (net/sched/sch_sfq.c:272 net/sched/sch_sfq.c:499) sch_sfq
[   88.810411] sfq_reset (net/sched/sch_sfq.c:525) sch_sfq
[   88.810671] qdisc_reset (./include/linux/skbuff.h:2135 ./include/linux/skbuff.h:2441 ./include/linux/skbuff.h:3304 ./include/linux/skbuff.h:3310 net/sched/sch_generic.c:1036)
[   88.810950] tbf_reset (./include/linux/timekeeping.h:169 net/sched/sch_tbf.c:334) sch_tbf
[   88.811208] qdisc_reset (./include/linux/skbuff.h:2135 ./include/linux/skbuff.h:2441 ./include/linux/skbuff.h:3304 ./include/linux/skbuff.h:3310 net/sched/sch_generic.c:1036)
[   88.811484] netif_set_real_num_tx_queues (./include/linux/spinlock.h:396 ./include/net/sch_generic.h:768 net/core/dev.c:2958)
[   88.811870] __tun_detach (drivers/net/tun.c:590 drivers/net/tun.c:673)
[   88.812271] tun_chr_close (drivers/net/tun.c:702 drivers/net/tun.c:3517)
[   88.812505] __fput (fs/file_table.c:432 (discriminator 1))
[   88.812735] task_work_run (kernel/task_work.c:230)
[   88.813016] do_exit (kernel/exit.c:940)
[   88.813372] ? trace_hardirqs_on (kernel/trace/trace_preemptirq.c:58 (discriminator 4))
[   88.813639] ? handle_mm_fault (./arch/x86/include/asm/irqflags.h:42 ./arch/x86/include/asm/irqflags.h:97 ./arch/x86/include/asm/irqflags.h:155 ./include/linux/memcontrol.h:1022 ./include/linux/memcontrol.h:1045 ./include/linux/memcontrol.h:1052 mm/memory.c:5928 mm/memory.c:6088)
[   88.813867] do_group_exit (kernel/exit.c:1070)
[   88.814138] __x64_sys_exit_group (kernel/exit.c:1099)
[   88.814490] x64_sys_call (??:?)
[   88.814791] do_syscall_64 (arch/x86/entry/common.c:52 (discriminator 1) arch/x86/entry/common.c:83 (discriminator 1))
[   88.815012] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
[   88.815495] RIP: 0033:0x7f44560f1975

Fixes: 175f9c1 ("net_sched: Add size table for qdiscs")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Link: https://patch.msgid.link/20241007184130.3960565-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
alistair23 pushed a commit to alistair23/linux that referenced this issue Nov 11, 2024
@plinkr @alistair23
rust: kernel: move FromBytes and AsBytes traits to a new `transmu…
9b605b3 
…te` module

Refactor the `FromBytes` and `AsBytes` traits from `types.rs` into a new
`transmute.rs` module:

 - Add `rust/kernel/transmute.rs` with the definitions of `FromBytes`
   and `AsBytes`.

 - Remove the same trait definitions from `rust/kernel/types.rs`.

 - Update `rust/kernel/uaccess.rs` to import `AsBytes` and `FromBytes`
   from `transmute.rs`.

The traits and their implementations remain unchanged.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: Rust-for-Linux#1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20240918225115.2309224-2-aliet.exposito@gmail.com
[ Rebased on top of the lints series and slightly reworded. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
alistair23 pushed a commit to alistair23/linux that referenced this issue Nov 12, 2024
@plinkr @alistair23
rust: kernel: move FromBytes and AsBytes traits to a new `transmu…
3609935 
…te` module

Refactor the `FromBytes` and `AsBytes` traits from `types.rs` into a new
`transmute.rs` module:

 - Add `rust/kernel/transmute.rs` with the definitions of `FromBytes`
   and `AsBytes`.

 - Remove the same trait definitions from `rust/kernel/types.rs`.

 - Update `rust/kernel/uaccess.rs` to import `AsBytes` and `FromBytes`
   from `transmute.rs`.

The traits and their implementations remain unchanged.

Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: Rust-for-Linux#1117
Signed-off-by: Aliet Exposito Garcia <aliet.exposito@gmail.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/r/20240918225115.2309224-2-aliet.exposito@gmail.com
[ Rebased on top of the lints series and slightly reworded. - Miguel ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@plinkr plinkr

Labels
good first issue Good for newcomers
Milestone
No milestone
Development

No branches or pull requests
5 participants
@ojeda @nbdd0121 @sergiocollado @plinkr @y86-dev