Skip to content

Commit 4f5408e

Browse files
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 755e5d6 commit 4f5408e

File tree

15 files changed

+181
-49
lines changed

15 files changed

+181
-49
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/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 macros;
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
}

rust/kernel/types.rs

Lines changed: 89 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,8 @@
44
//!
55
//! C header: [`include/linux/types.h`](../../../../include/linux/types.h)
66
7-
use core::ops::Deref;
8-
97
use crate::bindings;
8+
use crate::c_types;
109

1110
/// Permissions.
1211
///
@@ -27,31 +26,100 @@ impl Mode {
2726
}
2827
}
2928

29+
/// Possible errors when using conversion functions in [`CStr`] and [`CBoundedStr`].
30+
#[derive(Debug, Clone, Copy)]
31+
pub enum CStrConvertError {
32+
BoundExceeded,
33+
InteriorNul,
34+
NotNulTerminated,
35+
}
36+
3037
/// A string that is guaranteed to have exactly one `NUL` byte, which is at the
3138
/// end.
3239
///
3340
/// Used for interoperability with kernel APIs that take C strings.
3441
#[repr(transparent)]
35-
pub struct CStr<'a>(&'a str);
42+
pub struct CStr([u8]);
43+
44+
impl CStr {
45+
/// Returns the length of this string excluding NUL.
46+
pub const fn len(&self) -> usize {
47+
self.0.len() - 1
48+
}
49+
50+
/// Returns the length of this string with NUL.
51+
pub const fn len_with_nul(&self) -> usize {
52+
self.0.len()
53+
}
54+
55+
/// Returns `true` if the string only includes `NUL`.
56+
pub const fn is_empty(&self) -> bool {
57+
self.len() == 0
58+
}
59+
60+
/// Wraps a raw C string pointer.
61+
///
62+
/// # Safety
63+
///
64+
/// `ptr` must be a valid pointer to a NUL-terminated C string, and it must
65+
/// last at least `'a`. When `CStr` is alive, the memory pointed by `ptr`
66+
/// must not change.
67+
pub unsafe fn from_ptr<'a>(ptr: *const c_types::c_char) -> &'a Self {
68+
let len = bindings::strlen(ptr);
69+
Self::from_bytes_with_nul_unchecked(core::slice::from_raw_parts(ptr as _, len as _))
70+
}
71+
72+
/// Creates a [`CStr`] form a `[u8]`.
73+
///
74+
/// The provided slice must be nul-terminated, does not contain any
75+
/// interior nul bytes.
76+
pub fn from_bytes_with_nul(bytes: &[u8]) -> Result<&Self, CStrConvertError> {
77+
if bytes.is_empty() {
78+
return Err(CStrConvertError::NotNulTerminated);
79+
}
80+
if bytes[bytes.len() - 1] != 0 {
81+
return Err(CStrConvertError::NotNulTerminated);
82+
}
83+
if bytes[..bytes.len()].contains(&0) {
84+
return Err(CStrConvertError::InteriorNul);
85+
}
86+
// SAFETY: We just checked that all properties hold.
87+
Ok(unsafe { Self::from_bytes_with_nul_unchecked(bytes) })
88+
}
3689

37-
impl CStr<'_> {
38-
/// Creates a [`CStr`] from a [`str`] without performing any additional
90+
/// Creates a [`CStr`] from a `[u8]` without performing any additional
3991
/// checks.
4092
///
4193
/// # Safety
4294
///
43-
/// `data` *must* end with a `NUL` byte, and should only have only a single
95+
/// `bytes` *must* end with a `NUL` byte, and should only have a single
4496
/// `NUL` byte (or the string will be truncated).
45-
pub const unsafe fn new_unchecked(data: &str) -> CStr {
46-
CStr(data)
97+
#[inline]
98+
pub const unsafe fn from_bytes_with_nul_unchecked(bytes: &[u8]) -> &CStr {
99+
// Note: This can be done using pointer deref (which requires
100+
// const_raw_ptr_deref to be const) or transmute (which requires
101+
// const_transmute to be const) or ptr::from_raw_parts (which
102+
// requires ptr_metadata).
103+
// While none of them are current stable, it is very likely that
104+
// one of them will eventually be.
105+
&*(bytes as *const [u8] as *const Self)
47106
}
48-
}
49107

50-
impl Deref for CStr<'_> {
51-
type Target = str;
108+
/// Returns a C pointer to the string.
109+
pub const fn as_ptr(&self) -> *const c_types::c_char {
110+
self.0.as_ptr() as _
111+
}
52112

53-
fn deref(&self) -> &str {
54-
self.0
113+
/// Convert the string to a byte slice without the trailing 0 byte.
114+
#[inline]
115+
pub fn as_bytes(&self) -> &[u8] {
116+
&self.0[..self.0.len() - 1]
117+
}
118+
119+
/// Convert the string to a byte slice containing the trailing 0 byte.
120+
#[inline]
121+
pub const fn as_bytes_with_nul(&self) -> &[u8] {
122+
&self.0
55123
}
56124
}
57125

@@ -62,12 +130,15 @@ impl Deref for CStr<'_> {
62130
/// # Examples
63131
///
64132
/// ```rust,no_run
65-
/// const MY_CSTR: CStr<'static> = cstr!("My awesome CStr!");
133+
/// const MY_CSTR: &'static CStr = c_str!("My awesome CStr!");
66134
/// ```
67135
#[macro_export]
68-
macro_rules! cstr {
69-
($str:expr) => {{
70-
let s = concat!($str, "\x00");
71-
unsafe { $crate::CStr::new_unchecked(s) }
136+
macro_rules! c_str {
137+
($str:literal) => {{
138+
const C: &'static $crate::CStr = {
139+
let s = $crate::macros::append_nul!($str);
140+
unsafe { $crate::CStr::from_bytes_with_nul_unchecked(s) }
141+
};
142+
C
72143
}};
73144
}

rust/macros/c_str.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
3+
use crate::syn::{
4+
buffer::{Cursor, TokenBuffer},
5+
Lit,
6+
};
7+
use proc_macro::{Literal, TokenStream, TokenTree};
8+
9+
fn try_any_string(cursor: Cursor<'_>) -> Option<(Vec<u8>, Cursor<'_>)> {
10+
let (lit, next) = cursor.literal()?;
11+
let lit = Lit::new(lit);
12+
let (content, suffix) = match &lit {
13+
Lit::ByteStr(str) => Some((str.value(), str.suffix())),
14+
Lit::Str(str) => Some((str.value().into_bytes(), str.suffix())),
15+
_ => None,
16+
}?;
17+
if !suffix.is_empty() {
18+
return None;
19+
}
20+
Some((content, next))
21+
}
22+
23+
pub fn append_nul(ts: TokenStream) -> TokenStream {
24+
let buffer = TokenBuffer::new(ts);
25+
let cursor = buffer.begin();
26+
27+
let (mut content, cursor) = try_any_string(cursor).expect("Expected string or byte string");
28+
assert!(cursor.eof(), "Expected EOF");
29+
30+
if content.contains(&0) {
31+
panic!("String contains interior NUL bytes");
32+
}
33+
34+
content.push(0);
35+
TokenTree::Literal(Literal::byte_string(&content)).into()
36+
}

0 commit comments

Comments
 (0)