diff --git a/drivers/cube/cube_syscall.c b/drivers/cube/cube_syscall.c index c1a5b1038..a8dbcbc5f 100644 --- a/drivers/cube/cube_syscall.c +++ b/drivers/cube/cube_syscall.c @@ -95,7 +95,6 @@ static char store_device_path[256] = "/dev/vda"; static int __init cube_store_setup(char *str) { strscpy(store_device_path, str, sizeof(store_device_path)); - pr_info("cubelinux: cube_store= resolved to %s\n", store_device_path); return 1; } __setup("cube_store=", cube_store_setup); @@ -265,8 +264,24 @@ static long cube_enum_op(unsigned int op, void __user *uargs) __u8 found[32]; ret = cubelinux_kernel_spaces(e.cursor, found); + /* + * The driver answers with one space written and 0, or with a negative errno. A + * positive value is neither, and it must not be read as either: the buffer was not + * written, so a caller handed this answer copies a space nobody found. + * + * This arm is the only one that advances the cursor by itself. Every other walk + * leaves the cursor where the caller put it, so a bogus return there ends the walk + * on the client's no-progress rule. Here it would hand the walk a cursor that keeps + * moving over a space that is not there — which is not a hypothetical: a kernel error + * code that had lost its sign did exactly that, and the walk served ~200,000 invented + * records a minute until the machine stopped making progress. The cause is fixed + * where it was (the driver's errno conversions, cubelinux_store.rs); this is the + * bound that keeps a defect of that shape from becoming a loop again. + */ if (ret < 0) return ret; + if (ret != 0) + return -EINVAL; memcpy(e.space, found, sizeof(found)); /* An index here, not a count of records: hand back the one after this space. */ e.cursor = e.cursor + 1; diff --git a/drivers/cube/cubelinux_store.rs b/drivers/cube/cubelinux_store.rs index 5c2ebf020..d991ec734 100644 --- a/drivers/cube/cubelinux_store.rs +++ b/drivers/cube/cubelinux_store.rs @@ -466,7 +466,15 @@ fn store_file() -> Result<*mut bindings::file> { // O_RDWR is 2. SAFETY: store_device() is a NUL-terminated C string the module parameter filled // at boot; filp_open returns a valid `struct file *` or an error pointer, checked below. let filp = unsafe { bindings::filp_open(store_device(), 2, 0) }; - let filp = kernel::error::from_err_ptr(filp)?; + // A store that is not there is not an empty store, and this interface has to be able to say so. + // Every walk here ends on -ENOENT — "no such space; the walk is finished" — so a store that + // cannot even be opened would answer its caller with an empty listing and a success, and a + // caller has no way to tell the two apart. That is not hypothetical either: the benchmark boot + // that started this work had its store on a disk whose driver had not loaded, and the walk had + // no way to say what was wrong. "No such device" is what the store being absent is called here; + // the interface's own words for it are `cube_store=` and `/dev/cubelinux`. + let filp = + kernel::error::from_err_ptr(filp).map_err(|e| if e == ENOENT { ENODEV } else { e })?; if filp.is_null() { return Err(EINVAL); } @@ -2088,7 +2096,7 @@ pub unsafe extern "C" fn cubelinux_kernel_put( }; match append_mutation(&mutation, 1) { Ok(_) => 0, - Err(e) => -(e.to_errno() as i32), + Err(e) => e.to_errno() as i32, } } @@ -3301,16 +3309,16 @@ unsafe fn v3_get( let (first, records) = match image.space_entry(&sp) { Ok(Some(entry)) => entry, Ok(None) => return -2, - Err(e) => return -(e.to_errno() as isize), + Err(e) => return e.to_errno() as isize, }; let (value_off, value_len, flags) = match image.find(&key, first, records) { Ok(Some(entry)) => entry, Ok(None) => return -2, - Err(e) => return -(e.to_errno() as isize), + Err(e) => return e.to_errno() as isize, }; let value = match image.value_at(value_off, value_len) { Ok(v) => v, - Err(e) => return -(e.to_errno() as isize), + Err(e) => return e.to_errno() as isize, }; // The record's own class, from the index entry the address came from. A v3 entry has no mask // and answers zero, which is what "no class" means everywhere here. @@ -3405,7 +3413,7 @@ unsafe fn v3_get_chain( let image = match Addressed::open() { Ok(Some(image)) => image, Ok(None) => return -95, // -EOPNOTSUPP - Err(e) => return -(e.to_errno() as isize), + Err(e) => return e.to_errno() as isize, }; // The next slice is arithmetic, not a pointer: the same space at the same x and y, one `z` // further along. Nothing is stored to find it, so nothing can point wrongly. @@ -3448,7 +3456,7 @@ unsafe fn v3_get_chain( let image = match Addressed::open() { Ok(Some(image)) => image, Ok(None) => return -95, // -EOPNOTSUPP - Err(e) => return -(e.to_errno() as isize), + Err(e) => return e.to_errno() as isize, }; let (sp, key) = unsafe { coord_key(space, x, y, zz) }; let mut slice_flags = 0u16; @@ -3566,7 +3574,7 @@ unsafe fn v3_enum( let (first, records) = match image.space_entry(&wanted) { Ok(Some(entry)) => entry, Ok(None) => (0, 0), - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; // Own the log: the edits borrow it, and walking the image needs `&mut image`. let mut log = KVVec::::new(); @@ -3621,11 +3629,11 @@ unsafe fn v3_enum( while at < first + records { let (value_off, value_len, flags) = match image.index_entry(at, &mut entry) { Ok(triple) => triple, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; let value = match image.value_at(value_off, value_len) { Ok(v) => v, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; let key: &[u8; RAW_KEY_LEN] = match entry.as_slice()[..RAW_KEY_LEN].try_into() { Ok(k) => k, @@ -3660,7 +3668,7 @@ unsafe fn v3_enum( let image_entry = if at < first + records { match image.index_entry(at, &mut entry) { Ok(pair) => Some(pair), - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, } } else { None @@ -3686,7 +3694,7 @@ unsafe fn v3_enum( (_, Some((value_off, value_len, flags)), Some(key)) => { let value = match image.value_at(value_off, value_len) { Ok(v) => v, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; at += 1; if !batch.offer(&wanted, key, flags, value.as_slice()) { @@ -3749,7 +3757,7 @@ unsafe fn v3_range( let (first, records) = match image.space_entry(&wanted) { Ok(Some(entry)) => entry, Ok(None) => (0, 0), - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; // Own the log: the edits borrow it, and walking the image needs `&mut image`. let mut log = KVVec::::new(); @@ -3772,7 +3780,7 @@ unsafe fn v3_range( // The index's half. let mut at = match image.lower_bound(®ion.foot, first, records) { Ok(p) => p, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; // SAFETY: the shim guarantees `cap` writable bytes at `buf`. @@ -3805,7 +3813,7 @@ unsafe fn v3_range( let image_entry = if at < first + records { match image.index_entry(at, &mut entry) { Ok(pair) => Some(pair), - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, } } else { None @@ -3860,7 +3868,7 @@ unsafe fn v3_range( (_, Some((value_off, value_len, flags)), Some(key)) => { let value = match image.value_at(value_off, value_len) { Ok(v) => v, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; at += 1; if region.holds(key) && !batch.offer(&wanted, key, flags, value.as_slice()) { @@ -4118,7 +4126,7 @@ unsafe fn v3_flag_scan( let (first, records) = match image.space_entry(&wanted) { Ok(Some(entry)) => entry, Ok(None) => (0, 0), - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; // Own the log: the edits borrow it, and walking the image needs `&mut image`. let mut log = KVVec::::new(); @@ -4140,7 +4148,7 @@ unsafe fn v3_flag_scan( mode, &mut batch, ) { - return -(e.to_errno() as i32); + return e.to_errno() as i32; } // SAFETY: both out-pointers are writable under this function's contract. @@ -4192,7 +4200,7 @@ unsafe fn v3_flag_scan_every( let (space, first, records) = match image.space_row(row) { Ok(Some(triple)) => triple, Ok(None) => break, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; // A space only the log writes sorts before this one: visit it here, in its place. while log_at < from_log.len() @@ -4203,7 +4211,7 @@ unsafe fn v3_flag_scan_every( log_at += 1; match flag_scan_space(&mut image, log.as_slice(), &candidate, 0, 0, mask, mode, &mut batch) { Ok(keep_going) => full = !keep_going, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, } } // A space the log also writes into is the same space, not a second one. @@ -4213,7 +4221,7 @@ unsafe fn v3_flag_scan_every( if !full { match flag_scan_space(&mut image, log.as_slice(), &space, first, records, mask, mode, &mut batch) { Ok(keep_going) => full = !keep_going, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, } } row += 1; @@ -4224,7 +4232,7 @@ unsafe fn v3_flag_scan_every( log_at += 1; match flag_scan_space(&mut image, log.as_slice(), &candidate, 0, 0, mask, mode, &mut batch) { Ok(keep_going) => full = !keep_going, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, } } @@ -4289,11 +4297,11 @@ pub unsafe extern "C" fn cubelinux_kernel_get( return got; } Ok(None) => {} - Err(e) => return -(e.to_errno() as isize), + Err(e) => return e.to_errno() as isize, } let view = match read_view() { Ok(v) => v, - Err(e) => return -(e.to_errno() as isize), + Err(e) => return e.to_errno() as isize, }; match log_effect(view.log(), &sp, &key) { @@ -4409,8 +4417,7 @@ pub unsafe extern "C" fn cubelinux_kernel_enum( out_cursor: *mut u64, ) -> i32 { if let Err(e) = store_readable() { - pr_warn!("cubelinux: the store is not readable; refusing to answer\n"); - return -(e.to_errno() as i32); + return e.to_errno() as i32; } let mut wanted = [0u8; SPACE_ID_LEN]; // SAFETY: the caller guarantees 32 readable bytes at `space`. @@ -4421,12 +4428,12 @@ pub unsafe extern "C" fn cubelinux_kernel_enum( return unsafe { v3_enum(image, wanted, cursor, buf, cap, out_len, out_cursor) } } Ok(None) => {} - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, } let view = match read_view() { Ok(v) => v, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; // The live records of this space: the image's, with the log's edits applied as an overlay. @@ -4526,12 +4533,12 @@ pub unsafe extern "C" fn cubelinux_kernel_range( return unsafe { v3_range(image, wanted, region, cursor, buf, cap, out_len, out_cursor) } } Ok(None) => {} - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, } let view = match read_view() { Ok(v) => v, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; let mut walker = match SpaceWalker::new(&view, &wanted) { Ok(w) => w, @@ -4637,12 +4644,12 @@ pub unsafe extern "C" fn cubelinux_kernel_flag_scan( } } Ok(None) => {} - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, } let view = match read_view() { Ok(v) => v, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; // A v1/v2 image holds packed records, which have no mask field at all — they read as "no class" @@ -4675,25 +4682,17 @@ pub unsafe extern "C" fn cubelinux_kernel_flag_scan( match flag_scan_log_space(log.as_slice(), space, mask, mode, &mut batch) { Ok(true) => {} Ok(false) => break, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, } } } else if let Err(e) = flag_scan_log_space(log.as_slice(), &wanted, mask, mode, &mut batch) { - return -(e.to_errno() as i32); + return e.to_errno() as i32; } // SAFETY: both out-pointers are writable under this function's contract. unsafe { finish_batch(&batch, cursor, out_len, out_cursor) } } -/// `CUBE_OP_SPACES`: the `cursor`-th distinct space that holds a record, or -ENOENT at the end. -/// -/// An index rather than a count of records: a caller that wants a space's records walks it with -/// `CUBE_OP_ENUM` once it has learned the name. Entries are already in (space, key) order, so the -/// distinct spaces come out sorted. -/// -/// # Safety -/// `space_out` must point to 32 writable bytes. /// Whether the store can be read at all, asked before an operation answers anything. /// /// A walk pointed at something that is not a store served invented records — a garbage space, a @@ -4704,6 +4703,14 @@ pub unsafe extern "C" fn cubelinux_kernel_flag_scan( /// downstream answered over a device that cannot be read. /// /// So the question is asked here, where the answer leaves no room: four bytes at offset zero. +/// +/// **A refusal is only a refusal if it is negative.** This guard fired on every call and the walk +/// still received "here is a space": the callers below converted the error with `-(e.to_errno())`, +/// and [`Error::to_errno`] already returns the kernel's error code, which is negative. A doubled +/// minus sign made every refusal in this file a *positive* number, and a positive return is what +/// every layer above reads as success — the syscall's `ret < 0`, and the client's own wrapper. The +/// conversion is now `e.to_errno()`, as `kernel::from_result` does it, and the walk arm that advances +/// its own cursor refuses a positive answer as well, so this cannot be read as an answer again. fn store_readable() -> Result<()> { let file = store_file()?; let mut head = KVVec::::with_capacity(8, GFP_KERNEL)?; @@ -4716,20 +4723,27 @@ fn store_readable() -> Result<()> { Ok(()) } +/// `CUBE_OP_SPACES`: the `cursor`-th distinct space that holds a record, or -ENOENT at the end. +/// +/// An index rather than a count of records: a caller that wants a space's records walks it with +/// `CUBE_OP_ENUM` once it has learned the name. Entries are already in (space, key) order, so the +/// distinct spaces come out sorted. +/// +/// # Safety +/// `space_out` must point to 32 writable bytes. #[unsafe(no_mangle)] pub unsafe extern "C" fn cubelinux_kernel_spaces(cursor: u64, space_out: *mut u8) -> i32 { if let Err(e) = store_readable() { - pr_warn!("cubelinux: the store is not readable; refusing to answer\n"); - return -(e.to_errno() as i32); + return e.to_errno() as i32; } match Addressed::open() { Ok(Some(image)) => return unsafe { v3_spaces(image, cursor, space_out) }, Ok(None) => {} - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, } let view = match read_view() { Ok(v) => v, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; // Candidates, in ascending order: the spaces the image's records run through (a checkpoint @@ -4816,7 +4830,7 @@ pub unsafe extern "C" fn cubelinux_kernel_del(space: *const u8, x: u64, y: u64, // bounded read: a delete does not touch the records either. match append_mutation(&mutation, 2) { Ok(_) => 0, - Err(e) => -(e.to_errno() as i32), + Err(e) => e.to_errno() as i32, } } @@ -4827,11 +4841,11 @@ pub extern "C" fn cubelinux_kernel_sync() -> i32 { ensure_boot_record(); let (device, layout) = match device_and_layout() { Ok(pair) => pair, - Err(e) => return -(e.to_errno() as i32), + Err(e) => return e.to_errno() as i32, }; match fold_now(&device, &layout) { Ok(()) => 0, - Err(e) => -(e.to_errno() as i32), + Err(e) => e.to_errno() as i32, } }