* [PATCH] rust: dma: return zero for Coherent reads past EOF
@ 2026-07-20 19:21 ` Younes Akhouayri
0 siblings, 0 replies; 10+ messages in thread
From: Younes Akhouayri via B4 Relay @ 2026-07-20 19:21 UTC (permalink / raw)
To: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy,
Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross,
Tamir Duberstein, Alexandre Courbot, Onur Özkan, Timur Tabi
Cc: driver-core, rust-for-linux, linux-kernel, Younes Akhouayri
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>
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH] rust: dma: return zero for Coherent reads past EOF @ 2026-07-20 19:21 ` Younes Akhouayri 0 siblings, 0 replies; 10+ messages in thread From: Younes Akhouayri @ 2026-07-20 19:21 UTC (permalink / raw) To: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross, Tamir Duberstein, Alexandre Courbot, Onur Özkan, Timur Tabi Cc: driver-core, rust-for-linux, linux-kernel, Younes Akhouayri 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> ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: dma: return zero for Coherent reads past EOF 2026-07-20 19:21 ` Younes Akhouayri (?) @ 2026-07-23 16:15 ` Miguel Ojeda -1 siblings, 0 replies; 10+ messages in thread From: Miguel Ojeda @ 2026-07-23 16:15 UTC (permalink / raw) To: git Cc: Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross, Tamir Duberstein, Alexandre Courbot, Onur Özkan, Timur Tabi, driver-core, rust-for-linux, linux-kernel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: dma: return zero for Coherent reads past EOF 2026-07-20 19:21 ` Younes Akhouayri (?) (?) @ 2026-07-23 23:32 ` Gary Guo 2026-07-24 0:18 ` Alexandre Courbot -1 siblings, 1 reply; 10+ messages in thread From: Gary Guo @ 2026-07-23 23:32 UTC (permalink / raw) To: git, Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross, Tamir Duberstein, Alexandre Courbot, Onur Özkan, Timur Tabi Cc: driver-core, rust-for-linux, linux-kernel 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> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: dma: return zero for Coherent reads past EOF 2026-07-23 23:32 ` Gary Guo @ 2026-07-24 0:18 ` Alexandre Courbot 2026-07-24 0:46 ` Gary Guo 0 siblings, 1 reply; 10+ messages in thread From: Alexandre Courbot @ 2026-07-24 0:18 UTC (permalink / raw) To: Gary Guo Cc: git, Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross, Tamir Duberstein, Onur Özkan, Timur Tabi, driver-core, rust-for-linux, linux-kernel 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: dma: return zero for Coherent reads past EOF 2026-07-24 0:18 ` Alexandre Courbot @ 2026-07-24 0:46 ` Gary Guo 2026-07-24 1:19 ` Alexandre Courbot 2026-07-24 1:22 ` Danilo Krummrich 0 siblings, 2 replies; 10+ messages in thread From: Gary Guo @ 2026-07-24 0:46 UTC (permalink / raw) To: Alexandre Courbot, Gary Guo Cc: git, Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross, Tamir Duberstein, Onur Özkan, Timur Tabi, driver-core, rust-for-linux, linux-kernel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: dma: return zero for Coherent reads past EOF 2026-07-24 0:46 ` Gary Guo @ 2026-07-24 1:19 ` Alexandre Courbot 2026-07-24 2:12 ` Gary Guo 2026-07-24 1:22 ` Danilo Krummrich 1 sibling, 1 reply; 10+ messages in thread From: Alexandre Courbot @ 2026-07-24 1:19 UTC (permalink / raw) To: Gary Guo Cc: git, Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross, Tamir Duberstein, Onur Özkan, Timur Tabi, driver-core, rust-for-linux, linux-kernel 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: dma: return zero for Coherent reads past EOF 2026-07-24 1:19 ` Alexandre Courbot @ 2026-07-24 2:12 ` Gary Guo 0 siblings, 0 replies; 10+ messages in thread From: Gary Guo @ 2026-07-24 2:12 UTC (permalink / raw) To: Alexandre Courbot, Gary Guo Cc: git, Danilo Krummrich, Abdiel Janulgue, Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross, Tamir Duberstein, Onur Özkan, Timur Tabi, driver-core, rust-for-linux, linux-kernel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: dma: return zero for Coherent reads past EOF 2026-07-24 0:46 ` Gary Guo 2026-07-24 1:19 ` Alexandre Courbot @ 2026-07-24 1:22 ` Danilo Krummrich 2026-07-24 5:14 ` Alexandre Courbot 1 sibling, 1 reply; 10+ messages in thread From: Danilo Krummrich @ 2026-07-24 1:22 UTC (permalink / raw) To: Gary Guo Cc: Alexandre Courbot, git, Abdiel Janulgue, Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross, Tamir Duberstein, Onur Özkan, Timur Tabi, driver-core, rust-for-linux, linux-kernel 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: dma: return zero for Coherent reads past EOF 2026-07-24 1:22 ` Danilo Krummrich @ 2026-07-24 5:14 ` Alexandre Courbot 0 siblings, 0 replies; 10+ messages in thread From: Alexandre Courbot @ 2026-07-24 5:14 UTC (permalink / raw) To: Danilo Krummrich Cc: Gary Guo, git, Abdiel Janulgue, Daniel Almeida, Robin Murphy, Andreas Hindborg, Miguel Ojeda, Boqun Feng, Björn Roy Baron, Benno Lossin, Alice Ryhl, Trevor Gross, Tamir Duberstein, Onur Özkan, Timur Tabi, driver-core, rust-for-linux, linux-kernel 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-07-24 5:15 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-20 19:21 [PATCH] rust: dma: return zero for Coherent reads past EOF Younes Akhouayri via B4 Relay 2026-07-20 19:21 ` Younes Akhouayri 2026-07-23 16:15 ` Miguel Ojeda 2026-07-23 23:32 ` Gary Guo 2026-07-24 0:18 ` Alexandre Courbot 2026-07-24 0:46 ` Gary Guo 2026-07-24 1:19 ` Alexandre Courbot 2026-07-24 2:12 ` Gary Guo 2026-07-24 1:22 ` Danilo Krummrich 2026-07-24 5:14 ` Alexandre Courbot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.