Linux kernel mirror (for testing)
git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel
os
linux
gpu: nova-core: gsp: fix undefined behavior in command queue code
`driver_read_area` and `driver_write_area` are internal methods that
return slices containing the area of the command queue buffer that the
driver has exclusive read or write access, respectively.
While their returned value is correct and safe to use, internally they
temporarily create a reference to the whole command-buffer slice,
including GSP-owned regions. These regions can change without notice,
and thus creating a slice to them, even if never accessed, is undefined
behavior.
Fix this by making these methods create slices to valid regions only.
Fixes: 75f6b1de8133 ("gpu: nova-core: gsp: Add GSP command queue bindings and handling")
Reported-by: Danilo Krummrich <dakr@kernel.org>
Closes: https://lore.kernel.org/all/DH47AVPEKN06.3BERUSJIB4M1R@kernel.org/
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Link: https://patch.msgid.link/20260404-cmdq-ub-fix-v5-1-53d21f4752f5@nvidia.com
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
authored by
Alexandre Courbot
and committed by
Danilo Krummrich
8e6c3478
7c50d748
+69
-47
+69
-47
drivers/gpu/nova-core/gsp/cmdq.rs
+69
-47
drivers/gpu/nova-core/gsp/cmdq.rs
···
17
17
},
18
18
new_mutex,
19
19
prelude::*,
20
+
ptr,
20
21
sync::{
21
22
aref::ARef,
22
23
Mutex, //
···
256
255
/// As the message queue is a circular buffer, the region may be discontiguous in memory. In
257
256
/// that case the second slice will have a non-zero length.
258
257
fn driver_write_area(&mut self) -> (&mut [[u8; GSP_PAGE_SIZE]], &mut [[u8; GSP_PAGE_SIZE]]) {
259
-
let tx = self.cpu_write_ptr() as usize;
260
-
let rx = self.gsp_read_ptr() as usize;
258
+
let tx = self.cpu_write_ptr();
259
+
let rx = self.gsp_read_ptr();
260
+
261
+
// Pointer to the first entry of the CPU message queue.
262
+
let data = ptr::project!(mut self.0.as_mut_ptr(), .cpuq.msgq.data[0]);
263
+
264
+
let (tail_end, wrap_end) = if rx == 0 {
265
+
// The write area is non-wrapping, and stops at the second-to-last entry of the command
266
+
// queue (to leave the last one empty).
267
+
(MSGQ_NUM_PAGES - 1, 0)
268
+
} else if rx <= tx {
269
+
// The write area wraps and continues until `rx - 1`.
270
+
(MSGQ_NUM_PAGES, rx - 1)
271
+
} else {
272
+
// The write area doesn't wrap and stops at `rx - 1`.
273
+
(rx - 1, 0)
274
+
};
261
275
262
276
// SAFETY:
263
-
// - We will only access the driver-owned part of the shared memory.
264
-
// - Per the safety statement of the function, no concurrent access will be performed.
265
-
let gsp_mem = unsafe { &mut *self.0.as_mut() };
266
-
// PANIC: per the invariant of `cpu_write_ptr`, `tx` is `< MSGQ_NUM_PAGES`.
267
-
let (before_tx, after_tx) = gsp_mem.cpuq.msgq.data.split_at_mut(tx);
268
-
269
-
// The area starting at `tx` and ending at `rx - 2` modulo MSGQ_NUM_PAGES, inclusive,
270
-
// belongs to the driver for writing.
271
-
272
-
if rx == 0 {
273
-
// Since `rx` is zero, leave an empty slot at end of the buffer.
274
-
let last = after_tx.len() - 1;
275
-
(&mut after_tx[..last], &mut [])
276
-
} else if rx <= tx {
277
-
// The area is discontiguous and we leave an empty slot before `rx`.
278
-
// PANIC:
279
-
// - The index `rx - 1` is non-negative because `rx != 0` in this branch.
280
-
// - The index does not exceed `before_tx.len()` (which equals `tx`) because
281
-
// `rx <= tx` in this branch.
282
-
(after_tx, &mut before_tx[..(rx - 1)])
283
-
} else {
284
-
// The area is contiguous and we leave an empty slot before `rx`.
285
-
// PANIC:
286
-
// - The index `rx - tx - 1` is non-negative because `rx > tx` in this branch.
287
-
// - The index does not exceed `after_tx.len()` (which is `MSGQ_NUM_PAGES - tx`)
288
-
// because `rx < MSGQ_NUM_PAGES` by the `gsp_read_ptr` invariant.
289
-
(&mut after_tx[..(rx - tx - 1)], &mut [])
277
+
// - `data` was created from a valid pointer, and `rx` and `tx` are in the
278
+
// `0..MSGQ_NUM_PAGES` range per the invariants of `cpu_write_ptr` and `gsp_read_ptr`,
279
+
// thus the created slices are valid.
280
+
// - The area starting at `tx` and ending at `rx - 2` modulo `MSGQ_NUM_PAGES`,
281
+
// inclusive, belongs to the driver for writing and is not accessed concurrently by
282
+
// the GSP.
283
+
// - The caller holds a reference to `self` for as long as the returned slices are live,
284
+
// meaning the CPU write pointer cannot be advanced and thus that the returned area
285
+
// remains exclusive to the CPU for the duration of the slices.
286
+
// - The created slices point to non-overlapping sub-ranges of `data` in all
287
+
// branches (in the `rx <= tx` case, the second slice ends at `rx - 1` which is strictly
288
+
// less than `tx` where the first slice starts; in the other cases the second slice is
289
+
// empty), so creating two `&mut` references from them does not violate aliasing rules.
290
+
unsafe {
291
+
(
292
+
core::slice::from_raw_parts_mut(
293
+
data.add(num::u32_as_usize(tx)),
294
+
num::u32_as_usize(tail_end - tx),
295
+
),
296
+
core::slice::from_raw_parts_mut(data, num::u32_as_usize(wrap_end)),
297
+
)
290
298
}
291
299
}
292
300
···
318
308
/// As the message queue is a circular buffer, the region may be discontiguous in memory. In
319
309
/// that case the second slice will have a non-zero length.
320
310
fn driver_read_area(&self) -> (&[[u8; GSP_PAGE_SIZE]], &[[u8; GSP_PAGE_SIZE]]) {
321
-
let tx = self.gsp_write_ptr() as usize;
322
-
let rx = self.cpu_read_ptr() as usize;
311
+
let tx = self.gsp_write_ptr();
312
+
let rx = self.cpu_read_ptr();
313
+
314
+
// Pointer to the first entry of the GSP message queue.
315
+
let data = ptr::project!(self.0.as_ptr(), .gspq.msgq.data[0]);
316
+
317
+
let (tail_end, wrap_end) = if rx <= tx {
318
+
// Read area is non-wrapping and stops right before `tx`.
319
+
(tx, 0)
320
+
} else {
321
+
// Read area is wrapping and stops right before `tx`.
322
+
(MSGQ_NUM_PAGES, tx)
323
+
};
323
324
324
325
// SAFETY:
325
-
// - We will only access the driver-owned part of the shared memory.
326
-
// - Per the safety statement of the function, no concurrent access will be performed.
327
-
let gsp_mem = unsafe { &*self.0.as_ptr() };
328
-
let data = &gsp_mem.gspq.msgq.data;
329
-
330
-
// The area starting at `rx` and ending at `tx - 1` modulo MSGQ_NUM_PAGES, inclusive,
331
-
// belongs to the driver for reading.
332
-
// PANIC:
333
-
// - per the invariant of `cpu_read_ptr`, `rx < MSGQ_NUM_PAGES`
334
-
// - per the invariant of `gsp_write_ptr`, `tx < MSGQ_NUM_PAGES`
335
-
if rx <= tx {
336
-
// The area is contiguous.
337
-
(&data[rx..tx], &[])
338
-
} else {
339
-
// The area is discontiguous.
340
-
(&data[rx..], &data[..tx])
326
+
// - `data` was created from a valid pointer, and `rx` and `tx` are in the
327
+
// `0..MSGQ_NUM_PAGES` range per the invariants of `gsp_write_ptr` and `cpu_read_ptr`,
328
+
// thus the created slices are valid.
329
+
// - The area starting at `rx` and ending at `tx - 1` modulo `MSGQ_NUM_PAGES`,
330
+
// inclusive, belongs to the driver for reading and is not accessed concurrently by
331
+
// the GSP.
332
+
// - The caller holds a reference to `self` for as long as the returned slices are live,
333
+
// meaning the CPU read pointer cannot be advanced and thus that the returned area
334
+
// remains exclusive to the CPU for the duration of the slices.
335
+
unsafe {
336
+
(
337
+
core::slice::from_raw_parts(
338
+
data.add(num::u32_as_usize(rx)),
339
+
num::u32_as_usize(tail_end - rx),
340
+
),
341
+
core::slice::from_raw_parts(data, num::u32_as_usize(wrap_end)),
342
+
)
341
343
}
342
344
}
343
345