Skip to contentSkip to conversation

rust: dma: return EOVERFLOW instead of ENOMEM on size overflow

PATCH In review Aditya Rajan 4 participants 7 messages Apr 3 – Apr 5

Subsystems (3)
Driver core Rust DMA mapping & scatterlist API Rust
lore
Share & tools
lore

Aditya Rajan

author
#0 lore raw

PATCH 1/1 rust: dma: return EOVERFLOW instead of ENOMEM on size overflow

In alloc_slice_with_attrs(), the checked_mul() guards against arithmetic overflow when computing the total byte size (size_of::<T>() * len). If this overflows, the current code returns ENOMEM, which is misleading -- the system is not out of memory, the requested size simply cannot be represented in a usize.

Return EOVERFLOW instead, which accurately describes the failure. This also distinguishes it from the actual allocation failure two lines below, which correctly returns ENOMEM when dma_alloc_attrs() yields a null pointer.

Fixes d9aee73c56ee ("rust: dma: add generalized container for types other than slices")
Link https://lore.kernel.org/all/[email protected]/ open here
Signed-off-by Aditya Rajan <[email protected]>
---
 rust/kernel/dma.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
rust/kernel/dma.rs +1 -1
index 4995ee5dc689..179bc8832947 100644
@@ -832,7 +832,7 @@ fn alloc_slice_with_attrs(
             Err(EINVAL)?;
         }
 
-        let size = core::mem::size_of::<T>().checked_mul(len).ok_or(ENOMEM)?;
+        let size = core::mem::size_of::<T>().checked_mul(len).ok_or(EOVERFLOW)?;
         let mut dma_handle = 0;
         // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
         let addr = unsafe {
Signature · 1 line
2.53.0
reviewer
#1 lore raw
Quoted 24 lines · Aditya Rajan

In alloc_slice_with_attrs(), the checked_mul() guards against arithmetic overflow when computing the total byte size (size_of::<T>() * len). If this overflows, the current code returns ENOMEM, which is misleading -- the system is not out of memory, the requested size simply cannot be represented in a usize.

Return EOVERFLOW instead, which accurately describes the failure. This also distinguishes it from the actual allocation failure two lines below, which correctly returns ENOMEM when dma_alloc_attrs() yields a null pointer.

Fixes d9aee73c56ee ("rust: dma: add generalized container for types other than slices")
Link https://lore.kernel.org/all/[email protected]/ open here
Signed-off-by Aditya Rajan <[email protected]>
---
 rust/kernel/dma.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
rust/kernel/dma.rs +0 -0
index 4995ee5dc689..179bc8832947 100644

rust/kernel/dma.rs fn alloc_slice_with_attrs( · line 838

Quoted 8 lines
rust/kernel/dma.rs +1 -1
@@ -832,7 +832,7 @@ fn alloc_slice_with_attrs(
             Err(EINVAL)?;
         }
 
-        let size = core::mem::size_of::<T>().checked_mul(len).ok_or(ENOMEM)?;
+        let size = core::mem::size_of::<T>().checked_mul(len).ok_or(EOVERFLOW)?;
         let mut dma_handle = 0;
         // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
         let addr = unsafe {

Hi Aditya,

Thanks for the patch, but the behaviour here is intended.

Neither our KVec implementation nor upstream Rust distinguishes between allocation error caused by array size exceeding address space or running out of memory to allocate (AllocError is returned and it converts to ENOMEM).

kmalloc_array also just returns NULL when overflows, so arguably this behaviour also aligns us with C side.

Abstractly, the system is indeed running out memory because it cannot allocate something larger than its address space.

Best,
Gary

author
#2 lore raw
Quoted 11 lines · Gary Guo

Thanks for the patch, but the behaviour here is intended.

Neither our KVec implementation nor upstream Rust distinguishes between allocation error caused by array size exceeding address space or running out of memory to allocate (AllocError is returned and it converts to ENOMEM).

kmalloc_array also just returns NULL when overflows, so arguably this behaviour also aligns us with C side.

Abstractly, the system is indeed running out memory because it cannot allocate something larger than its address space.

Hi Gary,

Thanks for the reply, I saw at some similar places where EOVERFLOW is used, that is why i thought we should change this error code:

* In nouveau_drv.h, u_memcpya() does check_mul_overflow(nmemb, size, &bytes) and returns ERR_PTR(-EOVERFLOW), it is kind of same multiplication overflow on nmemb*size before an allocation. Similarly mm/mmap.c returns EOVERFLOW for arithmetic overflow in offset calculations, it also has a comment /* offset overflow? */.

* Also I saw existing Rust kernel code already follows similar convention, see rust/kernel/uaccess.rs it uses offset.checked_add(count).ok_or(EOVERFLOW)? for the same kind of arithmetic overflow check.

* For kmalloc_array i think it conflates overflow with OOM because its return type (pointer) can't express distinct errors, maybe it should be improved as well ?. When the API can distinguish (like here, or in nouveau), the kernel does use (or maybe should use?) EOVERFLOW.

IMO two failures have different semantics for callers, ENOMEM is transient (retry may succeed under less memory pressure), EOVERFLOW is deterministic (the input will never work). Using ENOMEM for overflow could mislead a caller into retrying a request that can never succeed.

Thanks, Aditya

maintainer
#3 lore raw
Quoted 23 lines · Aditya Rajan
Quoted 11 lines · Gary Guo

Thanks for the patch, but the behaviour here is intended.

Neither our KVec implementation nor upstream Rust distinguishes between allocation error caused by array size exceeding address space or running out of memory to allocate (AllocError is returned and it converts to ENOMEM).

kmalloc_array also just returns NULL when overflows, so arguably this behaviour also aligns us with C side.

Abstractly, the system is indeed running out memory because it cannot allocate something larger than its address space.

Thanks for the reply, I saw at some similar places where EOVERFLOW is used, that is why i thought we should change this error code:

* In nouveau_drv.h, u_memcpya() does check_mul_overflow(nmemb, size, &bytes) and returns ERR_PTR(-EOVERFLOW), it is kind of same multiplication overflow on nmemb*size before an allocation. Similarly mm/mmap.c returns EOVERFLOW for arithmetic overflow in offset calculations, it also has a comment /* offset overflow? */.

Quoted 8 lines

* Also I saw existing Rust kernel code already follows similar convention, see rust/kernel/uaccess.rs it uses offset.checked_add(count).ok_or(EOVERFLOW)? for the same kind of arithmetic overflow check.

* For kmalloc_array i think it conflates overflow with OOM because its return type (pointer) can't express distinct errors, maybe it should be improved as well ?. When the API can distinguish (like here, or in nouveau), the kernel does use (or maybe should use?) EOVERFLOW.

You mentioned u_memcpya() from nouveau, which follows memdup_array_user() and vmemdup_array_user(); and I think there are even more such examples that use -EOVERFLOW besides those and the also mentioned uaccess code.

That said, they all have on common that they are semantically different compared to a raw memory allocation, as they also access existing buffers the user wants those functions to copy from. Thus, a multiplication overflow also implies a potential out of bounds access of the given buffer. So, it makes sense to distinguish between -EOVERFLOW and -ENOMEM in those cases.

reviewer
#4 lore raw
In reply to Aditya Rajan
Quoted 16 lines · Aditya Rajan
Quoted 11 lines · Gary Guo

Thanks for the patch, but the behaviour here is intended.

Neither our KVec implementation nor upstream Rust distinguishes between allocation error caused by array size exceeding address space or running out of memory to allocate (AllocError is returned and it converts to ENOMEM).

kmalloc_array also just returns NULL when overflows, so arguably this behaviour also aligns us with C side.

Abstractly, the system is indeed running out memory because it cannot allocate something larger than its address space.

Hi Gary,

Quoted 8 lines

Thanks for the reply, I saw at some similar places where EOVERFLOW is used, that is why i thought we should change this error code:

* In nouveau_drv.h, u_memcpya() does check_mul_overflow(nmemb, size, &bytes) and returns ERR_PTR(-EOVERFLOW), it is kind of same multiplication overflow on nmemb*size before an allocation. Similarly mm/mmap.c returns EOVERFLOW for arithmetic overflow in offset calculations, it also has a comment /* offset overflow? */.

I think these cases are all related to values ultimately controlled by userspace, and the error conditions are propagated back to userspace, so it makes more sense to distinguish errors.

On the other hand, for KVec/Coherent cases, it is less useful to distinguish between the cases. If we want to add EOVERFLOW, then we'd need to extend AllocError to have two enums, which I wouldn't be against, but I think it's not worth having.

(I wanted Coherent to use AllocError as error too, but as it does not support zero-size allocation, so I didn't change the error type. I guess one option is to handle zero-size case by using a dangling pointer similar to KVec.)

Quoted 10 lines

* Also I saw existing Rust kernel code already follows similar convention, see rust/kernel/uaccess.rs it uses offset.checked_add(count).ok_or(EOVERFLOW)? for the same kind of arithmetic overflow check.

* For kmalloc_array i think it conflates overflow with OOM because its return type (pointer) can't express distinct errors, maybe it should be improved as well ?. When the API can distinguish (like here, or in nouveau), the kernel does use (or maybe should use?) EOVERFLOW.

Quoted 4 lines

IMO two failures have different semantics for callers, ENOMEM is transient (retry may succeed under less memory pressure), EOVERFLOW is deterministic (the input will never work). Using ENOMEM for overflow could mislead a caller into retrying a request that can never succeed.

If I allocate usize::MAX, it'll never work but the error would still be ENOMEM, and I see no reason to differentiate usize::MAX and usize::MAX + 1.

Best,
Gary

maintainer
#5 lore raw
Quoted 26 lines · Gary Guo
Quoted 16 lines · Aditya Rajan
Quoted 11 lines · Gary Guo

Thanks for the patch, but the behaviour here is intended.

Neither our KVec implementation nor upstream Rust distinguishes between allocation error caused by array size exceeding address space or running out of memory to allocate (AllocError is returned and it converts to ENOMEM).

kmalloc_array also just returns NULL when overflows, so arguably this behaviour also aligns us with C side.

Abstractly, the system is indeed running out memory because it cannot allocate something larger than its address space.

Hi Gary,

Quoted 8 lines

Thanks for the reply, I saw at some similar places where EOVERFLOW is used, that is why i thought we should change this error code:

* In nouveau_drv.h, u_memcpya() does check_mul_overflow(nmemb, size, &bytes) and returns ERR_PTR(-EOVERFLOW), it is kind of same multiplication overflow on nmemb*size before an allocation. Similarly mm/mmap.c returns EOVERFLOW for arithmetic overflow in offset calculations, it also has a comment /* offset overflow? */.

Quoted 3 lines

I think these cases are all related to values ultimately controlled by userspace, and the error conditions are propagated back to userspace, so it makes more sense to distinguish errors.

That's not the main difference, any of those primitives may be called on behalf of userspace with the error propagated back to userspace. Please also see my considerations in [1].

[1] https://lore.kernel.org/lkml/[email protected]/

Quoted 4 lines

On the other hand, for KVec/Coherent cases, it is less useful to distinguish between the cases. If we want to add EOVERFLOW, then we'd need to extend AllocError to have two enums, which I wouldn't be against, but I think it's not worth having.

I'd rather not, let's keep it consistent with other memory allocation primitives.

Quoted 3 lines

(I wanted Coherent to use AllocError as error too, but as it does not support zero-size allocation, so I didn't change the error type. I guess one option is to handle zero-size case by using a dangling pointer similar to KVec.)

I'm against this, a zero-sized dma::Coherent allocation does not make sense; I'd really want to see a real use-case for this.

Quoted 9 lines
Quoted 9 lines

* Also I saw existing Rust kernel code already follows similar convention, see rust/kernel/uaccess.rs it uses offset.checked_add(count).ok_or(EOVERFLOW)? for the same kind of arithmetic overflow check.

* For kmalloc_array i think it conflates overflow with OOM because its return type (pointer) can't express distinct errors, maybe it should be improved as well ?. When the API can distinguish (like here, or in nouveau), the kernel does use (or maybe should use?) EOVERFLOW.

Quoted 7 lines
Quoted 4 lines

IMO two failures have different semantics for callers, ENOMEM is transient (retry may succeed under less memory pressure), EOVERFLOW is deterministic (the input will never work). Using ENOMEM for overflow could mislead a caller into retrying a request that can never succeed.

If I allocate usize::MAX, it'll never work but the error would still be ENOMEM, and I see no reason to differentiate usize::MAX and usize::MAX + 1.

Agreed.

build bot
#6 lore raw
In reply to Aditya Rajan
kernel test robot noticed the following build errors:

Hi Aditya,

kernel test robot noticed the following build errors:

[auto build test ERROR on linux-next/master]
[cannot apply to linus/master v7.0-rc6]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Aditya-Rajan/rust-dma-return-EOVERFLOW-instead-of-ENOMEM-on-size-overflow/20260405-094049
base:   linux-next/master
patch link:    https://lore.kernel.org/r/20260403212822.294288-1-adi.dev.github%40gmail.com
patch subject: [PATCH] rust: dma: return EOVERFLOW instead of ENOMEM on size overflow
config: x86_64-rhel-9.4-rust (https://download.01.org/0day-ci/archive/20260405/[email protected]/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
rustc: rustc 1.88.0 (6b00bc388 2025-06-23)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260405/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

PATH=/opt/cross/clang-20/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
INFO PATH=/opt/cross/rustc-1.88.0-bindgen-0.72.1/cargo/bin:/opt/cross/clang-20/bin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
/usr/bin/timeout -k 100 12h /usr/bin/make KCFLAGS=\ -fno-crash-diagnostics\ -Wno-error=return-type\ -Wreturn-type\ -funsigned-char\ -Wundef\ -falign-functions=64 W=1 --keep-going LLVM=1 -j32 -C source O=/kbuild/obj/consumer/x86_64-rhel-9.4-rust ARCH=x86_64 SHELL=/bin/bash rustfmtcheck
make: Entering directory '/kbuild/src/consumer'
make[1]: Entering directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust'

>> Diff in rust/kernel/dma.rs:832:
                Err(EINVAL)?;
            }

- let size = core::mem::size_of::<T>().checked_mul(len).ok_or(EOVERFLOW)?;
+ let size = core::mem::size_of::<T>()
+ .checked_mul(len)

   +            .ok_or(EOVERFLOW)?;
            let mut dma_handle = 0;
            // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
            let addr = unsafe {
>> Diff in rust/kernel/dma.rs:832:
                Err(EINVAL)?;
            }

- let size = core::mem::size_of::<T>().checked_mul(len).ok_or(EOVERFLOW)?;
+ let size = core::mem::size_of::<T>()
+ .checked_mul(len)

   +            .ok_or(EOVERFLOW)?;
            let mut dma_handle = 0;
            // SAFETY: Device pointer is guaranteed as valid by the type invariant on `Device`.
            let addr = unsafe {

make[2]: *** [Makefile:1932: rustfmt] Error 123
make[2]: Target 'rustfmtcheck' not remade because of errors. make[1]: Leaving directory '/kbuild/obj/consumer/x86_64-rhel-9.4-rust' make[1]: *** [Makefile:248: __sub-make] Error 2
make: *** [Makefile:248: __sub-make] Error 2
make[1]: Target 'rustfmtcheck' not remade because of errors.
make: Target 'rustfmtcheck' not remade because of errors.
make: Leaving directory '/kbuild/src/consumer'

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki