[PATCH] rust: dma: return zero for Coherent reads past EOF

Younes Akhouayri via B4 Relay posted 1 patch 4 days, 6 hours ago
rust/kernel/dma.rs | 4 ++++
1 file changed, 4 insertions(+)
[PATCH] rust: dma: return zero for Coherent reads past EOF
Posted by Younes Akhouayri via B4 Relay 4 days, 6 hours ago
From: Younes Akhouayri <git@younes.io>

Coherent<T>::write_to_slice() calculates a zero-byte copy when the file
offset is beyond the allocation, but still calls
UserSliceWriter::write_dma(). The latter rejects offsets beyond the
allocation even when the copy length is zero, so a debugfs read past EOF
returns -ERANGE.

Return before calling write_dma() when the offset is at or beyond the
allocation, matching simple_read_from_buffer() EOF semantics.

Fixes: 016818513936 ("rust: dma: implement BinaryWriter for Coherent<[u8]>")
Signed-off-by: Younes Akhouayri <git@younes.io>
---
 rust/kernel/dma.rs | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 200def84fb69..36b72fb9ab46 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -1005,6 +1005,10 @@ fn write_to_slice(
             return Ok(0);
         };
 
+        if offset_val >= self.size() {
+            return Ok(0);
+        }
+
         let count = self.size().saturating_sub(offset_val).min(writer.len());
 
         writer.write_dma(self, offset_val, count)?;

---
base-commit: 880c43b185ca52239e75bc546cc4f4d9154d0fed
change-id: 20260720-fix-dma-coherent-eof-d0d08c91e013

Best regards,
--  
Younes Akhouayri <git@younes.io>
Re: [PATCH] rust: dma: return zero for Coherent reads past EOF
Posted by Gary Guo 1 day, 1 hour ago
On Mon Jul 20, 2026 at 8:21 PM BST, Younes Akhouayri via B4 Relay wrote:
> From: Younes Akhouayri <git@younes.io>
>
> Coherent<T>::write_to_slice() calculates a zero-byte copy when the file
> offset is beyond the allocation, but still calls
> UserSliceWriter::write_dma(). The latter rejects offsets beyond the
> allocation even when the copy length is zero, so a debugfs read past EOF
> returns -ERANGE.
>
> Return before calling write_dma() when the offset is at or beyond the
> allocation, matching simple_read_from_buffer() EOF semantics.
>
> Fixes: 016818513936 ("rust: dma: implement BinaryWriter for Coherent<[u8]>")
> Signed-off-by: Younes Akhouayri <git@younes.io>
> ---
>  rust/kernel/dma.rs | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
> index 200def84fb69..36b72fb9ab46 100644
> --- a/rust/kernel/dma.rs
> +++ b/rust/kernel/dma.rs
> @@ -1005,6 +1005,10 @@ fn write_to_slice(
>              return Ok(0);
>          };
>  
> +        if offset_val >= self.size() {
> +            return Ok(0);
> +        }
> +
>          let count = self.size().saturating_sub(offset_val).min(writer.len());

This saturating_sub is redundant now and should just use -.

Best,
Gary

>  
>          writer.write_dma(self, offset_val, count)?;
>
> ---
> base-commit: 880c43b185ca52239e75bc546cc4f4d9154d0fed
> change-id: 20260720-fix-dma-coherent-eof-d0d08c91e013
>
> Best regards,
> --  
> Younes Akhouayri <git@younes.io>
Re: [PATCH] rust: dma: return zero for Coherent reads past EOF
Posted by Alexandre Courbot 1 day, 1 hour ago
On Fri Jul 24, 2026 at 8:32 AM JST, Gary Guo wrote:
> On Mon Jul 20, 2026 at 8:21 PM BST, Younes Akhouayri via B4 Relay wrote:
>> From: Younes Akhouayri <git@younes.io>
>>
>> Coherent<T>::write_to_slice() calculates a zero-byte copy when the file
>> offset is beyond the allocation, but still calls
>> UserSliceWriter::write_dma(). The latter rejects offsets beyond the
>> allocation even when the copy length is zero, so a debugfs read past EOF
>> returns -ERANGE.
>>
>> Return before calling write_dma() when the offset is at or beyond the
>> allocation, matching simple_read_from_buffer() EOF semantics.
>>
>> Fixes: 016818513936 ("rust: dma: implement BinaryWriter for Coherent<[u8]>")
>> Signed-off-by: Younes Akhouayri <git@younes.io>
>> ---
>>  rust/kernel/dma.rs | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
>> index 200def84fb69..36b72fb9ab46 100644
>> --- a/rust/kernel/dma.rs
>> +++ b/rust/kernel/dma.rs
>> @@ -1005,6 +1005,10 @@ fn write_to_slice(
>>              return Ok(0);
>>          };
>>  
>> +        if offset_val >= self.size() {
>> +            return Ok(0);
>> +        }
>> +
>>          let count = self.size().saturating_sub(offset_val).min(writer.len());
>
> This saturating_sub is redundant now and should just use -.

Or even better, you could use `checked_sub` to perform the test and the
subtraction in one go, while avoiding potentially panicking operands.
Re: [PATCH] rust: dma: return zero for Coherent reads past EOF
Posted by Gary Guo 1 day ago
On Fri Jul 24, 2026 at 1:18 AM BST, Alexandre Courbot wrote:
> On Fri Jul 24, 2026 at 8:32 AM JST, Gary Guo wrote:
>> On Mon Jul 20, 2026 at 8:21 PM BST, Younes Akhouayri via B4 Relay wrote:
>>> From: Younes Akhouayri <git@younes.io>
>>>
>>> Coherent<T>::write_to_slice() calculates a zero-byte copy when the file
>>> offset is beyond the allocation, but still calls
>>> UserSliceWriter::write_dma(). The latter rejects offsets beyond the
>>> allocation even when the copy length is zero, so a debugfs read past EOF
>>> returns -ERANGE.
>>>
>>> Return before calling write_dma() when the offset is at or beyond the
>>> allocation, matching simple_read_from_buffer() EOF semantics.
>>>
>>> Fixes: 016818513936 ("rust: dma: implement BinaryWriter for Coherent<[u8]>")
>>> Signed-off-by: Younes Akhouayri <git@younes.io>
>>> ---
>>>  rust/kernel/dma.rs | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
>>> index 200def84fb69..36b72fb9ab46 100644
>>> --- a/rust/kernel/dma.rs
>>> +++ b/rust/kernel/dma.rs
>>> @@ -1005,6 +1005,10 @@ fn write_to_slice(
>>>              return Ok(0);
>>>          };
>>>  
>>> +        if offset_val >= self.size() {
>>> +            return Ok(0);
>>> +        }
>>> +
>>>          let count = self.size().saturating_sub(offset_val).min(writer.len());
>>
>> This saturating_sub is redundant now and should just use -.
>
> Or even better, you could use `checked_sub` to perform the test and the
> subtraction in one go, while avoiding potentially panicking operands.

I think the code is clearer with separate EOF check length calc. This also needs
to return `Ok` so it's not like that you can use `?`.

Best,
Gary
Re: [PATCH] rust: dma: return zero for Coherent reads past EOF
Posted by Danilo Krummrich 1 day ago
On Fri Jul 24, 2026 at 2:46 AM CEST, Gary Guo wrote:
> On Fri Jul 24, 2026 at 1:18 AM BST, Alexandre Courbot wrote:
>> On Fri Jul 24, 2026 at 8:32 AM JST, Gary Guo wrote:
>>> On Mon Jul 20, 2026 at 8:21 PM BST, Younes Akhouayri via B4 Relay wrote:
>>>> From: Younes Akhouayri <git@younes.io>
>>>>
>>>> Coherent<T>::write_to_slice() calculates a zero-byte copy when the file
>>>> offset is beyond the allocation, but still calls
>>>> UserSliceWriter::write_dma(). The latter rejects offsets beyond the
>>>> allocation even when the copy length is zero, so a debugfs read past EOF
>>>> returns -ERANGE.
>>>>
>>>> Return before calling write_dma() when the offset is at or beyond the
>>>> allocation, matching simple_read_from_buffer() EOF semantics.
>>>>
>>>> Fixes: 016818513936 ("rust: dma: implement BinaryWriter for Coherent<[u8]>")
>>>> Signed-off-by: Younes Akhouayri <git@younes.io>
>>>> ---
>>>>  rust/kernel/dma.rs | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
>>>> index 200def84fb69..36b72fb9ab46 100644
>>>> --- a/rust/kernel/dma.rs
>>>> +++ b/rust/kernel/dma.rs
>>>> @@ -1005,6 +1005,10 @@ fn write_to_slice(
>>>>              return Ok(0);
>>>>          };
>>>>  
>>>> +        if offset_val >= self.size() {
>>>> +            return Ok(0);
>>>> +        }
>>>> +
>>>>          let count = self.size().saturating_sub(offset_val).min(writer.len());
>>>
>>> This saturating_sub is redundant now and should just use -.
>>
>> Or even better, you could use `checked_sub` to perform the test and the
>> subtraction in one go, while avoiding potentially panicking operands.
>
> I think the code is clearer with separate EOF check length calc. This also needs
> to return `Ok` so it's not like that you can use `?`.

At a first glance I also thought of checked_sub(), and I think

	let Some(count) = self.size().checked_sub(offset_val) else {
	    return Ok(0);
	};

	writer.write_dma(self, offset_val, count.min(writer.len()))?;

would work perfectly fine, but compared to Gary's proposal it still calls
write_dma() for offset_val == self.size(), which is harmless but unnecessary.

Another option to avoid the panicking operand would be to keep saturating_sub()
and add the check below the existing saturating_sub() call.

	if count == 0 {
	    return Ok(0);
	}

But this is also less obvious, as it hides a bit the fact that the problem is
about offset_val when calling write_dma().

So, I think using the potentially panicking operand is probably the best option,
but I'd also be fine taking the patch as it is.
Re: [PATCH] rust: dma: return zero for Coherent reads past EOF
Posted by Alexandre Courbot 20 hours ago
On Fri Jul 24, 2026 at 10:22 AM JST, Danilo Krummrich wrote:
> On Fri Jul 24, 2026 at 2:46 AM CEST, Gary Guo wrote:
>> On Fri Jul 24, 2026 at 1:18 AM BST, Alexandre Courbot wrote:
>>> On Fri Jul 24, 2026 at 8:32 AM JST, Gary Guo wrote:
>>>> On Mon Jul 20, 2026 at 8:21 PM BST, Younes Akhouayri via B4 Relay wrote:
>>>>> From: Younes Akhouayri <git@younes.io>
>>>>>
>>>>> Coherent<T>::write_to_slice() calculates a zero-byte copy when the file
>>>>> offset is beyond the allocation, but still calls
>>>>> UserSliceWriter::write_dma(). The latter rejects offsets beyond the
>>>>> allocation even when the copy length is zero, so a debugfs read past EOF
>>>>> returns -ERANGE.
>>>>>
>>>>> Return before calling write_dma() when the offset is at or beyond the
>>>>> allocation, matching simple_read_from_buffer() EOF semantics.
>>>>>
>>>>> Fixes: 016818513936 ("rust: dma: implement BinaryWriter for Coherent<[u8]>")
>>>>> Signed-off-by: Younes Akhouayri <git@younes.io>
>>>>> ---
>>>>>  rust/kernel/dma.rs | 4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
>>>>> index 200def84fb69..36b72fb9ab46 100644
>>>>> --- a/rust/kernel/dma.rs
>>>>> +++ b/rust/kernel/dma.rs
>>>>> @@ -1005,6 +1005,10 @@ fn write_to_slice(
>>>>>              return Ok(0);
>>>>>          };
>>>>>  
>>>>> +        if offset_val >= self.size() {
>>>>> +            return Ok(0);
>>>>> +        }
>>>>> +
>>>>>          let count = self.size().saturating_sub(offset_val).min(writer.len());
>>>>
>>>> This saturating_sub is redundant now and should just use -.
>>>
>>> Or even better, you could use `checked_sub` to perform the test and the
>>> subtraction in one go, while avoiding potentially panicking operands.
>>
>> I think the code is clearer with separate EOF check length calc. This also needs
>> to return `Ok` so it's not like that you can use `?`.
>
> At a first glance I also thought of checked_sub(), and I think
>
> 	let Some(count) = self.size().checked_sub(offset_val) else {
> 	    return Ok(0);
> 	};
>
> 	writer.write_dma(self, offset_val, count.min(writer.len()))?;
>
> would work perfectly fine, but compared to Gary's proposal it still calls
> write_dma() for offset_val == self.size(), which is harmless but unnecessary.
>
> Another option to avoid the panicking operand would be to keep saturating_sub()
> and add the check below the existing saturating_sub() call.
>
> 	if count == 0 {
> 	    return Ok(0);
> 	}
>
> But this is also less obvious, as it hides a bit the fact that the problem is
> about offset_val when calling write_dma().

So far this is the version I like best; a comment to clarify the
intent of the code should also remove all ambiguity.

What I had in mind was something like

    let count = match self.size().checked_sub(offset_val) {
          Some(0) | None => return Ok(0),
          Some(count) => count.min(writer.len()),
    };

... but I understand the concern that this is less legible.

The most important imho is that this check gives us an opportunity to
remove a panicking operator; granted, the check is enough to guarantee
the panic case will never be met, but it's still cleaner to not use
these when it is possible and easy to do so, which is the case here.
Re: [PATCH] rust: dma: return zero for Coherent reads past EOF
Posted by Alexandre Courbot 1 day ago
On Fri Jul 24, 2026 at 9:46 AM JST, Gary Guo wrote:
> On Fri Jul 24, 2026 at 1:18 AM BST, Alexandre Courbot wrote:
>> On Fri Jul 24, 2026 at 8:32 AM JST, Gary Guo wrote:
>>> On Mon Jul 20, 2026 at 8:21 PM BST, Younes Akhouayri via B4 Relay wrote:
>>>> From: Younes Akhouayri <git@younes.io>
>>>>
>>>> Coherent<T>::write_to_slice() calculates a zero-byte copy when the file
>>>> offset is beyond the allocation, but still calls
>>>> UserSliceWriter::write_dma(). The latter rejects offsets beyond the
>>>> allocation even when the copy length is zero, so a debugfs read past EOF
>>>> returns -ERANGE.
>>>>
>>>> Return before calling write_dma() when the offset is at or beyond the
>>>> allocation, matching simple_read_from_buffer() EOF semantics.
>>>>
>>>> Fixes: 016818513936 ("rust: dma: implement BinaryWriter for Coherent<[u8]>")
>>>> Signed-off-by: Younes Akhouayri <git@younes.io>
>>>> ---
>>>>  rust/kernel/dma.rs | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
>>>> index 200def84fb69..36b72fb9ab46 100644
>>>> --- a/rust/kernel/dma.rs
>>>> +++ b/rust/kernel/dma.rs
>>>> @@ -1005,6 +1005,10 @@ fn write_to_slice(
>>>>              return Ok(0);
>>>>          };
>>>>  
>>>> +        if offset_val >= self.size() {
>>>> +            return Ok(0);
>>>> +        }
>>>> +
>>>>          let count = self.size().saturating_sub(offset_val).min(writer.len());
>>>
>>> This saturating_sub is redundant now and should just use -.
>>
>> Or even better, you could use `checked_sub` to perform the test and the
>> subtraction in one go, while avoiding potentially panicking operands.
>
> I think the code is clearer with separate EOF check length calc. This also needs
> to return `Ok` so it's not like that you can use `?`.

Is it clearer? The `checked_sub` is efficient and makes it clear the
code won't panic. And `ok_or` takes care of converting to the right
error nicely.
Re: [PATCH] rust: dma: return zero for Coherent reads past EOF
Posted by Gary Guo 23 hours ago
On Fri Jul 24, 2026 at 2:19 AM BST, Alexandre Courbot wrote:
> On Fri Jul 24, 2026 at 9:46 AM JST, Gary Guo wrote:
>> On Fri Jul 24, 2026 at 1:18 AM BST, Alexandre Courbot wrote:
>>> On Fri Jul 24, 2026 at 8:32 AM JST, Gary Guo wrote:
>>>> On Mon Jul 20, 2026 at 8:21 PM BST, Younes Akhouayri via B4 Relay wrote:
>>>>> From: Younes Akhouayri <git@younes.io>
>>>>>
>>>>> Coherent<T>::write_to_slice() calculates a zero-byte copy when the file
>>>>> offset is beyond the allocation, but still calls
>>>>> UserSliceWriter::write_dma(). The latter rejects offsets beyond the
>>>>> allocation even when the copy length is zero, so a debugfs read past EOF
>>>>> returns -ERANGE.
>>>>>
>>>>> Return before calling write_dma() when the offset is at or beyond the
>>>>> allocation, matching simple_read_from_buffer() EOF semantics.
>>>>>
>>>>> Fixes: 016818513936 ("rust: dma: implement BinaryWriter for Coherent<[u8]>")
>>>>> Signed-off-by: Younes Akhouayri <git@younes.io>
>>>>> ---
>>>>>  rust/kernel/dma.rs | 4 ++++
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
>>>>> index 200def84fb69..36b72fb9ab46 100644
>>>>> --- a/rust/kernel/dma.rs
>>>>> +++ b/rust/kernel/dma.rs
>>>>> @@ -1005,6 +1005,10 @@ fn write_to_slice(
>>>>>              return Ok(0);
>>>>>          };
>>>>>  
>>>>> +        if offset_val >= self.size() {
>>>>> +            return Ok(0);
>>>>> +        }
>>>>> +
>>>>>          let count = self.size().saturating_sub(offset_val).min(writer.len());
>>>>
>>>> This saturating_sub is redundant now and should just use -.
>>>
>>> Or even better, you could use `checked_sub` to perform the test and the
>>> subtraction in one go, while avoiding potentially panicking operands.
>>
>> I think the code is clearer with separate EOF check length calc. This also needs
>> to return `Ok` so it's not like that you can use `?`.
>
> Is it clearer? The `checked_sub` is efficient and makes it clear the
> code won't panic. And `ok_or` takes care of converting to the right
> error nicely.

There's no error here.

Best,
Gary
Re: [PATCH] rust: dma: return zero for Coherent reads past EOF
Posted by Miguel Ojeda 1 day, 9 hours ago
On Mon, Jul 20, 2026 at 9:22 PM Younes Akhouayri via B4 Relay
<devnull+git.younes.io@kernel.org> wrote:
>
> Fixes: 016818513936 ("rust: dma: implement BinaryWriter for Coherent<[u8]>")

This hash is in v7.1 so it should likely have:

Cc: stable@vger.kernel.org

In addition:

Link: https://rust-for-linux.zulipchat.com/#narrow/channel/291566-Library/topic/.E2.9C.94.20Possible.20past-EOF.20bug.20in.20Coherent.3CT.3E.3A.3Awrite_to_slice/near/611677095

since it was discussed there :)

No need to resend, though!

> base-commit: 880c43b185ca52239e75bc546cc4f4d9154d0fed

This points to `rust-fixes`, but it was likely better to point to the
`driver-core` branch instead.

Thanks for the patch!

Cheers,
Miguel