Skip to content

Commit 0de10ea

Browse files
nbdd0121nbdd0121
committed
rust: CStr overhaul
`CStr` is overhauled to make using it more similar to use a `str`. `c_str` macro can now check if the literal supplied have any internal NUL (which would violate CStr contract). Signed-off-by: Gary Guo <[email protected]>
1 parent 392e448 commit 0de10ea

File tree

15 files changed

+207
-54
lines changed

15 files changed

+207
-54
lines changed

‎drivers/android/rust_binder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
use alloc::{boxed::Box, sync::Arc};
1111
use core::pin::Pin;
1212
use kernel::{
13-
cstr,
13+
c_str,
1414
io_buffer::IoBufferWriter,
1515
linked_list::{GetLinks, GetLinksWrapped, Links},
1616
miscdev::Registration,
@@ -119,7 +119,7 @@ impl KernelModule for BinderModule {
119119
let pinned_ctx = Context::new()?;
120120
let ctx = unsafe { Pin::into_inner_unchecked(pinned_ctx) };
121121
let reg = Registration::<Arc<Context>>::new_pinned::<process::Process>(
122-
cstr!("rust_binder"),
122+
c_str!("rust_binder"),
123123
None,
124124
ctx,
125125
)?;

‎rust/Makefile

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
obj-$(CONFIG_RUST) += core.o compiler_builtins.o helpers.o
44
extra-$(CONFIG_RUST) += exports_core_generated.h
55

6-
extra-$(CONFIG_RUST) += libmodule.so
6+
extra-$(CONFIG_RUST) += libmodule.so libc_str_check.so
77

88
extra-$(CONFIG_RUST) += bindings_generated.rs
99
obj-$(CONFIG_RUST) += alloc.o kernel.o
@@ -33,9 +33,11 @@ rustdoc-compiler_builtins: $(srctree)/rust/compiler_builtins.rs FORCE
3333

3434
rustdoc-kernel: private rustdoc_target_flags = --extern alloc \
3535
--extern link_time_panic \
36-
--extern module=$(objtree)/rust/libmodule.so
36+
--extern module=$(objtree)/rust/libmodule.so \
37+
--extern c_str_check=$(objtree)/rust/libc_str_check.so
3738
rustdoc-kernel: $(srctree)/rust/kernel/lib.rs rustdoc-module \
38-
$(objtree)/rust/libmodule.so $(objtree)/rust/bindings_generated.rs FORCE
39+
$(objtree)/rust/libmodule.so $(objtree)/rust/libc_str_check.so \
40+
$(objtree)/rust/bindings_generated.rs FORCE
3941
$(call if_changed,rustdoc)
4042

4143
ifdef CONFIG_CC_IS_CLANG
@@ -130,6 +132,11 @@ $(objtree)/rust/libmodule.so: $(srctree)/rust/module.rs \
130132
$(objtree)/rust/libparse.rlib FORCE
131133
$(call if_changed_dep,rustc_procmacro)
132134

135+
$(objtree)/rust/libc_str_check.so: private rustc_target_flags = --extern parse
136+
$(objtree)/rust/libc_str_check.so: $(srctree)/rust/c_str_check.rs \
137+
$(objtree)/rust/libparse.rlib FORCE
138+
$(call if_changed_dep,rustc_procmacro)
139+
133140
quiet_cmd_rustc_library = $(if $(skip_clippy),RUSTC,$(RUSTC_OR_CLIPPY_QUIET)) L $@
134141
cmd_rustc_library = \
135142
RUST_BINDINGS_FILE=$(abspath $(objtree)/rust/bindings_generated.rs) \
@@ -172,8 +179,10 @@ $(objtree)/rust/link_time_panic.o: $(srctree)/rust/link_time_panic.rs \
172179
# ICE on `--extern module`: https://github.com/rust-lang/rust/issues/56935
173180
$(objtree)/rust/kernel.o: private rustc_target_flags = --extern alloc \
174181
--extern link_time_panic \
175-
--extern module=$(objtree)/rust/libmodule.so
182+
--extern module=$(objtree)/rust/libmodule.so \
183+
--extern c_str_check=$(objtree)/rust/libc_str_check.so
176184
$(objtree)/rust/kernel.o: $(srctree)/rust/kernel/lib.rs $(objtree)/rust/alloc.o \
177185
$(objtree)/rust/link_time_panic.o \
178-
$(objtree)/rust/libmodule.so $(objtree)/rust/bindings_generated.rs FORCE
186+
$(objtree)/rust/libmodule.so $(objtree)/rust/libc_str_check.so \
187+
$(objtree)/rust/bindings_generated.rs FORCE
179188
$(call if_changed_dep,rustc_library)

‎rust/c_str_check.rs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
3+
//! Proc macro crate implementing the [`c_str!`] macro.
4+
5+
use proc_macro::{token_stream, Delimiter, Literal, TokenStream, TokenTree};
6+
7+
fn try_literal(it: &mut token_stream::IntoIter) -> Option<Literal> {
8+
if let Some(TokenTree::Literal(literal)) = it.next() {
9+
Some(literal)
10+
} else {
11+
None
12+
}
13+
}
14+
15+
fn try_any_string(it: &mut token_stream::IntoIter) -> Option<Vec<u8>> {
16+
let lit = try_literal(it)?.to_string();
17+
let (content, suffix) = match parse::byte(&lit, 0) {
18+
b'"' | b'r' => {
19+
let (content, suffix) = parse::parse_lit_str(&lit);
20+
Some((content.into_string().into_bytes(), suffix))
21+
}
22+
b'b' => match parse::byte(&lit, 1) {
23+
b'"' | b'r' => Some(parse::parse_lit_byte_str(&lit)),
24+
_ => None,
25+
},
26+
_ => None,
27+
}?;
28+
if !suffix.is_empty() {
29+
return None;
30+
}
31+
Some(content)
32+
}
33+
34+
fn expect_any_string(it: &mut token_stream::IntoIter) -> Vec<u8> {
35+
try_any_string(it).expect("Expected string or byte string")
36+
}
37+
38+
fn expect_end(it: &mut token_stream::IntoIter) {
39+
if it.next().is_some() {
40+
panic!("Expected end");
41+
}
42+
}
43+
44+
// If the literal is passed along from a macro, Rust will create a implicitly
45+
// delimited group. We need to extract the token inside.
46+
fn unwrap_implicit_delim(ts: TokenTree) -> TokenStream {
47+
match ts {
48+
TokenTree::Group(group) if group.delimiter() == Delimiter::None => group.stream(),
49+
_ => ts.into(),
50+
}
51+
}
52+
53+
#[proc_macro]
54+
pub fn append_nul(ts: TokenStream) -> TokenStream {
55+
let mut it = ts.into_iter();
56+
let mut content = {
57+
let mut it = unwrap_implicit_delim(it.next().expect("Unexpected end")).into_iter();
58+
let content = expect_any_string(&mut it);
59+
expect_end(&mut it);
60+
content
61+
};
62+
expect_end(&mut it);
63+
64+
if content.contains(&0) {
65+
panic!("String contains interior NUL bytes");
66+
}
67+
68+
content.push(0);
69+
TokenTree::Literal(Literal::byte_string(&content)).into()
70+
}

‎rust/kernel/chrdev.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ use core::mem::MaybeUninit;
1515
use core::pin::Pin;
1616

1717
use crate::bindings;
18-
use crate::c_types;
1918
use crate::error::{Error, KernelResult};
2019
use crate::file_operations;
2120
use crate::types::CStr;
@@ -31,7 +30,7 @@ struct RegistrationInner<const N: usize> {
3130
///
3231
/// May contain up to a fixed number (`N`) of devices. Must be pinned.
3332
pub struct Registration<const N: usize> {
34-
name: CStr<'static>,
33+
name: &'static CStr,
3534
minors_start: u16,
3635
this_module: &'static crate::ThisModule,
3736
inner: Option<RegistrationInner<N>>,
@@ -48,7 +47,7 @@ impl<const N: usize> Registration<{ N }> {
4847
/// are going to pin the registration right away, call
4948
/// [`Self::new_pinned()`] instead.
5049
pub fn new(
51-
name: CStr<'static>,
50+
name: &'static CStr,
5251
minors_start: u16,
5352
this_module: &'static crate::ThisModule,
5453
) -> Self {
@@ -64,7 +63,7 @@ impl<const N: usize> Registration<{ N }> {
6463
///
6564
/// This does *not* register the device: see [`Self::register()`].
6665
pub fn new_pinned(
67-
name: CStr<'static>,
66+
name: &'static CStr,
6867
minors_start: u16,
6968
this_module: &'static crate::ThisModule,
7069
) -> KernelResult<Pin<Box<Self>>> {
@@ -90,7 +89,7 @@ impl<const N: usize> Registration<{ N }> {
9089
&mut dev,
9190
this.minors_start.into(),
9291
N.try_into()?,
93-
this.name.as_ptr() as *const c_types::c_char,
92+
this.name.as_ptr(),
9493
)
9594
};
9695
if res != 0 {

‎rust/kernel/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
const_fn,
2020
const_mut_refs,
2121
const_panic,
22+
const_raw_ptr_deref,
2223
try_reserve
2324
)]
2425
#![deny(clippy::complexity)]
@@ -65,6 +66,9 @@ pub mod iov_iter;
6566
mod types;
6667
pub mod user_ptr;
6768

69+
#[doc(hidden)]
70+
pub use c_str_check;
71+
6872
pub use crate::error::{Error, KernelResult};
6973
pub use crate::types::{CStr, Mode};
7074

‎rust/kernel/miscdev.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
99
use crate::error::{Error, KernelResult};
1010
use crate::file_operations::{FileOpenAdapter, FileOpener, FileOperationsVtable};
11-
use crate::{bindings, c_types, CStr};
11+
use crate::{bindings, CStr};
1212
use alloc::boxed::Box;
1313
use core::marker::PhantomPinned;
1414
use core::pin::Pin;
@@ -41,7 +41,7 @@ impl<T: Sync> Registration<T> {
4141
///
4242
/// Returns a pinned heap-allocated representation of the registration.
4343
pub fn new_pinned<F: FileOpener<T>>(
44-
name: CStr<'static>,
44+
name: &'static CStr,
4545
minor: Option<i32>,
4646
context: T,
4747
) -> KernelResult<Pin<Box<Self>>> {
@@ -56,7 +56,7 @@ impl<T: Sync> Registration<T> {
5656
/// self-referential. If a minor is not given, the kernel allocates a new one if possible.
5757
pub fn register<F: FileOpener<T>>(
5858
self: Pin<&mut Self>,
59-
name: CStr<'static>,
59+
name: &'static CStr,
6060
minor: Option<i32>,
6161
) -> KernelResult {
6262
// SAFETY: We must ensure that we never move out of `this`.
@@ -68,7 +68,7 @@ impl<T: Sync> Registration<T> {
6868

6969
// SAFETY: The adapter is compatible with `misc_register`.
7070
this.mdev.fops = unsafe { FileOperationsVtable::<Self, F>::build() };
71-
this.mdev.name = name.as_ptr() as *const c_types::c_char;
71+
this.mdev.name = name.as_ptr();
7272
this.mdev.minor = minor.unwrap_or(bindings::MISC_DYNAMIC_MINOR as i32);
7373

7474
let ret = unsafe { bindings::misc_register(&mut this.mdev) };

‎rust/kernel/sync/condvar.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ impl CondVar {
130130
}
131131

132132
impl NeedsLockClass for CondVar {
133-
unsafe fn init(self: Pin<&Self>, name: CStr<'static>, key: *mut bindings::lock_class_key) {
134-
bindings::__init_waitqueue_head(self.wait_list.get(), name.as_ptr() as _, key);
133+
unsafe fn init(self: Pin<&Self>, name: &'static CStr, key: *mut bindings::lock_class_key) {
134+
bindings::__init_waitqueue_head(self.wait_list.get(), name.as_ptr(), key);
135135
}
136136
}

‎rust/kernel/sync/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ macro_rules! init_with_lockdep {
5151
// SAFETY: `CLASS` is never used by Rust code directly; the kernel may change it though.
5252
#[allow(unused_unsafe)]
5353
unsafe {
54-
$crate::sync::NeedsLockClass::init($obj, $crate::cstr!($name), CLASS.as_mut_ptr())
54+
$crate::sync::NeedsLockClass::init($obj, $crate::c_str!($name), CLASS.as_mut_ptr())
5555
};
5656
}};
5757
}
@@ -69,7 +69,7 @@ pub trait NeedsLockClass {
6969
/// # Safety
7070
///
7171
/// `key` must point to a valid memory location as it will be used by the kernel.
72-
unsafe fn init(self: Pin<&Self>, name: CStr<'static>, key: *mut bindings::lock_class_key);
72+
unsafe fn init(self: Pin<&Self>, name: &'static CStr, key: *mut bindings::lock_class_key);
7373
}
7474

7575
/// Determines if a signal is pending on the current process.

‎rust/kernel/sync/mutex.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ impl<T: ?Sized> Mutex<T> {
7171
}
7272

7373
impl<T: ?Sized> NeedsLockClass for Mutex<T> {
74-
unsafe fn init(self: Pin<&Self>, name: CStr<'static>, key: *mut bindings::lock_class_key) {
75-
bindings::__mutex_init(self.mutex.get(), name.as_ptr() as _, key);
74+
unsafe fn init(self: Pin<&Self>, name: &'static CStr, key: *mut bindings::lock_class_key) {
75+
bindings::__mutex_init(self.mutex.get(), name.as_ptr(), key);
7676
}
7777
}
7878

‎rust/kernel/sync/spinlock.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ impl<T: ?Sized> SpinLock<T> {
8585
}
8686

8787
impl<T: ?Sized> NeedsLockClass for SpinLock<T> {
88-
unsafe fn init(self: Pin<&Self>, name: CStr<'static>, key: *mut bindings::lock_class_key) {
89-
rust_helper_spin_lock_init(self.spin_lock.get(), name.as_ptr() as _, key);
88+
unsafe fn init(self: Pin<&Self>, name: &'static CStr, key: *mut bindings::lock_class_key) {
89+
rust_helper_spin_lock_init(self.spin_lock.get(), name.as_ptr(), key);
9090
}
9191
}
9292

‎rust/kernel/sysctl.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use crate::{
1616
bindings, c_types, error,
1717
io_buffer::IoBufferWriter,
1818
types,
19+
types::CStr,
1920
user_ptr::{UserSlicePtr, UserSlicePtrWriter},
2021
};
2122

@@ -130,19 +131,19 @@ unsafe extern "C" fn proc_handler<T: SysctlStorage>(
130131
impl<T: SysctlStorage> Sysctl<T> {
131132
/// Registers a single entry in `sysctl`.
132133
pub fn register(
133-
path: types::CStr<'static>,
134-
name: types::CStr<'static>,
134+
path: &'static CStr,
135+
name: &'static CStr,
135136
storage: T,
136137
mode: types::Mode,
137138
) -> error::KernelResult<Sysctl<T>> {
138-
if name.contains('/') {
139+
if name.as_bytes().contains(&b'/') {
139140
return Err(error::Error::EINVAL);
140141
}
141142

142143
let storage = Box::try_new(storage)?;
143144
let mut table = vec![
144145
bindings::ctl_table {
145-
procname: name.as_ptr() as *const i8,
146+
procname: name.as_ptr(),
146147
mode: mode.as_int(),
147148
data: &*storage as *const T as *mut c_types::c_void,
148149
proc_handler: Some(proc_handler::<T>),
@@ -157,8 +158,7 @@ impl<T: SysctlStorage> Sysctl<T> {
157158
]
158159
.into_boxed_slice();
159160

160-
let result =
161-
unsafe { bindings::register_sysctl(path.as_ptr() as *const i8, table.as_mut_ptr()) };
161+
let result = unsafe { bindings::register_sysctl(path.as_ptr(), table.as_mut_ptr()) };
162162
if result.is_null() {
163163
return Err(error::Error::ENOMEM);
164164
}

0 commit comments

Comments
 (0)