All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
@ 2025-07-11  9:30 Quaternions
  2025-07-11  9:30 ` [PATCH] gpu: nova-core: define named constants for magic numbers Quaternions
  2025-07-11 18:03 ` [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds Danilo Krummrich
  0 siblings, 2 replies; 12+ messages in thread
From: Quaternions @ 2025-07-11  9:30 UTC (permalink / raw)
  To: dakr; +Cc: nouveau, Quaternions

Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
---
 drivers/gpu/nova-core/vbios.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 663fc50e8b66..5b5d9f38cbb3 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -901,7 +901,7 @@ struct PmuLookupTableEntry {
 
 impl PmuLookupTableEntry {
     fn new(data: &[u8]) -> Result<Self> {
-        if data.len() < 5 {
+        if data.len() < 6 {
             return Err(EINVAL);
         }
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] gpu: nova-core: define named constants for magic numbers
  2025-07-11  9:30 [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds Quaternions
@ 2025-07-11  9:30 ` Quaternions
  2025-07-11 18:03 ` [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds Danilo Krummrich
  1 sibling, 0 replies; 12+ messages in thread
From: Quaternions @ 2025-07-11  9:30 UTC (permalink / raw)
  To: dakr; +Cc: nouveau, Quaternions

Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
---
 drivers/gpu/nova-core/vbios.rs | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 5b5d9f38cbb3..d456c494374d 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -364,8 +364,9 @@ struct BitHeader {
 }
 
 impl BitHeader {
+    const MIN_LEN: usize = 12;
     fn new(data: &[u8]) -> Result<Self> {
-        if data.len() < 12 {
+        if data.len() < Self::MIN_LEN {
             return Err(EINVAL);
         }
 
@@ -467,8 +468,9 @@ struct PciRomHeader {
 }
 
 impl PciRomHeader {
+    const MIN_LEN: usize = 26;
     fn new(pdev: &pci::Device, data: &[u8]) -> Result<Self> {
-        if data.len() < 26 {
+        if data.len() < Self::MIN_LEN {
             // Need at least 26 bytes to read pciDataStrucPtr and sizeOfBlock.
             return Err(EINVAL);
         }
@@ -772,10 +774,11 @@ fn into_image(self) -> Result<BiosImage> {
         BiosImage::try_from(self)
     }
 
+    const MIN_LEN: usize = 26;
     /// Creates a new BiosImageBase from raw byte data.
     fn new(pdev: &pci::Device, data: &[u8]) -> Result<Self> {
         // Ensure we have enough data for the ROM header.
-        if data.len() < 26 {
+        if data.len() < Self::MIN_LEN {
             dev_err!(pdev.as_ref(), "Not enough data for ROM header\n");
             return Err(EINVAL);
         }
@@ -900,8 +903,9 @@ struct PmuLookupTableEntry {
 }
 
 impl PmuLookupTableEntry {
+    const MIN_LEN: usize = 6;
     fn new(data: &[u8]) -> Result<Self> {
-        if data.len() < 6 {
+        if data.len() < Self::MIN_LEN {
             return Err(EINVAL);
         }
 
@@ -928,8 +932,9 @@ struct PmuLookupTable {
 }
 
 impl PmuLookupTable {
+    const MIN_LEN: usize = 4;
     fn new(pdev: &pci::Device, data: &[u8]) -> Result<Self> {
-        if data.len() < 4 {
+        if data.len() < Self::MIN_LEN {
             return Err(EINVAL);
         }
 
-- 
2.50.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
@ 2025-07-11 11:33 Quaternions
  2025-07-11 11:54 ` Greg KH
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Quaternions @ 2025-07-11 11:33 UTC (permalink / raw)
  To: acourbot; +Cc: rust-for-linux, Quaternions

Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
---
 drivers/gpu/nova-core/vbios.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 663fc50e8b66..5b5d9f38cbb3 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -901,7 +901,7 @@ struct PmuLookupTableEntry {

 impl PmuLookupTableEntry {
     fn new(data: &[u8]) -> Result<Self> {
-        if data.len() < 5 {
+        if data.len() < 6 {
             return Err(EINVAL);
         }

--
2.50.1

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
  2025-07-11 11:33 Quaternions
@ 2025-07-11 11:54 ` Greg KH
  2025-07-11 12:04 ` Alexandre Courbot
  2025-07-11 12:12 ` Miguel Ojeda
  2 siblings, 0 replies; 12+ messages in thread
From: Greg KH @ 2025-07-11 11:54 UTC (permalink / raw)
  To: Quaternions; +Cc: acourbot, rust-for-linux

On Fri, Jul 11, 2025 at 04:33:26AM -0700, Quaternions wrote:
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

I know I can't take patches without a changelog, but perhaps other
maintainers are more lax?

Please don't do this...

And fix your "From:" line in your email client?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
  2025-07-11 11:33 Quaternions
  2025-07-11 11:54 ` Greg KH
@ 2025-07-11 12:04 ` Alexandre Courbot
  2025-07-11 12:20   ` Alexandre Courbot
  2025-07-11 12:12 ` Miguel Ojeda
  2 siblings, 1 reply; 12+ messages in thread
From: Alexandre Courbot @ 2025-07-11 12:04 UTC (permalink / raw)
  To: Quaternions; +Cc: rust-for-linux

Thanks for the fix!

On Fri Jul 11, 2025 at 8:33 PM JST, Quaternions wrote:
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>

You will want to split the subject to something more minimal (e.g. "gpu:
nova-core: fix bounds check In PmuLookupTableEntry::new"), and use the
rest of the sentence as the patch description (as a rule of thumb, 70
chars for the subject, and kernel patches must all have a description).

Also your "From:" field must also match your "Signed-off-by".

./scripts/checkpatch.pl will complain about these points (and check
other things as well), so please make sure to run it before submission:

    WARNING: Missing commit description - Add an appropriate one
    WARNING: From:/Signed-off-by: email name mismatch: 'From: Quaternions <krakow20@gmail.com>' != 'Signed-off-by: Rhys Lloyd <krakow20@gmail.com>'

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
  2025-07-11 11:33 Quaternions
  2025-07-11 11:54 ` Greg KH
  2025-07-11 12:04 ` Alexandre Courbot
@ 2025-07-11 12:12 ` Miguel Ojeda
  2 siblings, 0 replies; 12+ messages in thread
From: Miguel Ojeda @ 2025-07-11 12:12 UTC (permalink / raw)
  To: Quaternions; +Cc: acourbot, rust-for-linux

On Fri, Jul 11, 2025 at 1:34 PM Quaternions <krakow20@gmail.com> wrote:
>
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>

Few procedural nits:

  - The title needs to be shorter.

  - The kernel generally does not allow empty commit messages.

  - I would have expected a "From:" line, since the email header do
not match the SoB. Git should have generated this for you, normally.

  - This should have a "Fixes:" tag.

  - Ideally you would provide a base commit via `--base`.

Regarding the patch itself: is there a way this could be done better?
i.e. avoiding a literal?

I see in your other patch you create a `const`, but could we infer
that value somehow from elsewhere?

For instance, we have a range `2..6`, so can we save somewhere that
range and query its end? (it is a field)

Or, even better, can we do fallible slicing with `get()` and avoid
having a "separate" check?

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
  2025-07-11 12:04 ` Alexandre Courbot
@ 2025-07-11 12:20   ` Alexandre Courbot
  2025-07-11 14:32     ` Rhys Lloyd
  0 siblings, 1 reply; 12+ messages in thread
From: Alexandre Courbot @ 2025-07-11 12:20 UTC (permalink / raw)
  To: Alexandre Courbot, Quaternions; +Cc: rust-for-linux

On Fri Jul 11, 2025 at 9:04 PM JST, Alexandre Courbot wrote:
> Thanks for the fix!
>
> On Fri Jul 11, 2025 at 8:33 PM JST, Quaternions wrote:
>> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
>
> You will want to split the subject to something more minimal (e.g. "gpu:
> nova-core: fix bounds check In PmuLookupTableEntry::new"), and use the
> rest of the sentence as the patch description (as a rule of thumb, 70
> chars for the subject, and kernel patches must all have a description).
>
> Also your "From:" field must also match your "Signed-off-by".
>
> ./scripts/checkpatch.pl will complain about these points (and check
> other things as well), so please make sure to run it before submission:
>
>     WARNING: Missing commit description - Add an appropriate one
>     WARNING: From:/Signed-off-by: email name mismatch: 'From: Quaternions <krakow20@gmail.com>' != 'Signed-off-by: Rhys Lloyd <krakow20@gmail.com>'

Forgot to mention: please also run "./scripts/get_maintainer.pl" on
your patches to get the list of recipients you should send them to.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
  2025-07-11 12:20   ` Alexandre Courbot
@ 2025-07-11 14:32     ` Rhys Lloyd
  2025-07-11 14:55       ` Miguel Ojeda
  0 siblings, 1 reply; 12+ messages in thread
From: Rhys Lloyd @ 2025-07-11 14:32 UTC (permalink / raw)
  To: Alexandre Courbot; +Cc: rust-for-linux

Hi, this is my first contribution so thanks for pointing out my
mistakes.  It looks like there was only one newline between the commit
message and description, so the description spilled into the message.
If `git send-email` was supposed to add a From: line, I couldn't tell
you why it didn't, but I'll change the commit author to my full name
instead of my handle.

> Regarding the patch itself: is there a way this could be done better?

As Alexandre Courbot mentioned in the other patch, the solution that
fully addresses the issue will be to use `FromBytes` once it lands for
structs with no padding holes.

I don't know if I can submit v2 in a reply, so I'll attempt another
`send-email` shortly.


On Fri, Jul 11, 2025 at 5:20 AM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> On Fri Jul 11, 2025 at 9:04 PM JST, Alexandre Courbot wrote:
> > Thanks for the fix!
> >
> > On Fri Jul 11, 2025 at 8:33 PM JST, Quaternions wrote:
> >> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
> >
> > You will want to split the subject to something more minimal (e.g. "gpu:
> > nova-core: fix bounds check In PmuLookupTableEntry::new"), and use the
> > rest of the sentence as the patch description (as a rule of thumb, 70
> > chars for the subject, and kernel patches must all have a description).
> >
> > Also your "From:" field must also match your "Signed-off-by".
> >
> > ./scripts/checkpatch.pl will complain about these points (and check
> > other things as well), so please make sure to run it before submission:
> >
> >     WARNING: Missing commit description - Add an appropriate one
> >     WARNING: From:/Signed-off-by: email name mismatch: 'From: Quaternions <krakow20@gmail.com>' != 'Signed-off-by: Rhys Lloyd <krakow20@gmail.com>'
>
> Forgot to mention: please also run "./scripts/get_maintainer.pl" on
> your patches to get the list of recipients you should send them to.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
  2025-07-11 14:32     ` Rhys Lloyd
@ 2025-07-11 14:55       ` Miguel Ojeda
  0 siblings, 0 replies; 12+ messages in thread
From: Miguel Ojeda @ 2025-07-11 14:55 UTC (permalink / raw)
  To: Rhys Lloyd; +Cc: Alexandre Courbot, rust-for-linux

On Fri, Jul 11, 2025 at 4:34 PM Rhys Lloyd <krakow20@gmail.com> wrote:
>
> Hi, this is my first contribution so thanks for pointing out my
> mistakes.  It looks like there was only one newline between the commit
> message and description, so the description spilled into the message.

You're welcome!

> If `git send-email` was supposed to add a From: line, I couldn't tell
> you why it didn't, but I'll change the commit author to my full name
> instead of my handle.

Yeah, if you have the correct Git author in the commit, then it should
all work smoothly.

> I don't know if I can submit v2 in a reply, so I'll attempt another
> `send-email` shortly.

Personally I suggest to avoid nesting threads, especially when there
is a long discussion or they are a long series, and instead sending
versions as independent email threads -- it reads better in some
clients (including in lore.kernel.org, in my opinion).

As for `send-email`, yeah, I would suggest always using the same way
to submit your patches -- doing things differently is prone to
mistakes. You may also want to check `b4` for that, by the way.

I hope that helps.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
  2025-07-11  9:30 [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds Quaternions
  2025-07-11  9:30 ` [PATCH] gpu: nova-core: define named constants for magic numbers Quaternions
@ 2025-07-11 18:03 ` Danilo Krummrich
  2025-07-12  1:30   ` Rhys Lloyd
  1 sibling, 1 reply; 12+ messages in thread
From: Danilo Krummrich @ 2025-07-11 18:03 UTC (permalink / raw)
  To: Quaternions; +Cc: nouveau, acourbot

Hi Rhys,

On Fri Jul 11, 2025 at 11:30 AM CEST, Quaternions wrote:
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>

Thanks for your contribution.

When sending patches, please make sure to follow the kernel's patch submission
guidelines [1].

In particular, please stick to short and meaningful commit subject, followed by
a commit description, even if the patch is simple and obvious.

Also make sure to run ./scripts/checkpatch.pl and ./scripts/get_maintainer.pl to
get your patches checked and find the correct list of people to send them to.

Finally, please also consider the Rust submit checklist [2].

Thanks,
Danilo

[1] https://docs.kernel.org/process/submitting-patches.html
[2] https://rust-for-linux.com/contributing#submit-checklist-addendum

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new,  data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
  2025-07-11 18:03 ` [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds Danilo Krummrich
@ 2025-07-12  1:30   ` Rhys Lloyd
  2025-07-12 13:11     ` Danilo Krummrich
  0 siblings, 1 reply; 12+ messages in thread
From: Rhys Lloyd @ 2025-07-12  1:30 UTC (permalink / raw)
  To: Danilo Krummrich; +Cc: nouveau, acourbot

Hi Danilo,
I didn't understand the maintainers list yesterday, so all the patches
I sent are missing mailing lists and recipients.  I sent a second copy
as requested in
https://gitlab.freedesktop.org/drm/nova/-/merge_requests/4#note_3003142,
and my mailing list mistakes were pointed out in detail.  I also sent
a fixed up v2 to the rust-for-linux mailing list only.  If I need to
send out another copy let me know, thanks.

On Fri, Jul 11, 2025 at 11:03 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Hi Rhys,
>
> On Fri Jul 11, 2025 at 11:30 AM CEST, Quaternions wrote:
> > Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
>
> Thanks for your contribution.
>
> When sending patches, please make sure to follow the kernel's patch submission
> guidelines [1].
>
> In particular, please stick to short and meaningful commit subject, followed by
> a commit description, even if the patch is simple and obvious.
>
> Also make sure to run ./scripts/checkpatch.pl and ./scripts/get_maintainer.pl to
> get your patches checked and find the correct list of people to send them to.
>
> Finally, please also consider the Rust submit checklist [2].
>
> Thanks,
> Danilo
>
> [1] https://docs.kernel.org/process/submitting-patches.html
> [2] https://rust-for-linux.com/contributing#submit-checklist-addendum

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds.
  2025-07-12  1:30   ` Rhys Lloyd
@ 2025-07-12 13:11     ` Danilo Krummrich
  0 siblings, 0 replies; 12+ messages in thread
From: Danilo Krummrich @ 2025-07-12 13:11 UTC (permalink / raw)
  To: Rhys Lloyd; +Cc: nouveau, acourbot

On Sat Jul 12, 2025 at 3:30 AM CEST, Rhys Lloyd wrote:
> Hi Danilo,
> I didn't understand the maintainers list yesterday, so all the patches
> I sent are missing mailing lists and recipients.  I sent a second copy
> as requested in
> https://gitlab.freedesktop.org/drm/nova/-/merge_requests/4#note_3003142,
> and my mailing list mistakes were pointed out in detail.  I also sent
> a fixed up v2 to the rust-for-linux mailing list only.  If I need to
> send out another copy let me know, thanks.

Can you please send a v2 following the instructions I linked in my previous
reply?

For your patch, scripts/get_maintainer.pl should give you

	Danilo Krummrich <dakr@kernel.org> (maintainer:CORE DRIVER FOR NVIDIA GPUS [RUST])
	Alexandre Courbot <acourbot@nvidia.com> (maintainer:CORE DRIVER FOR NVIDIA GPUS [RUST])
	David Airlie <airlied@gmail.com> (maintainer:DRM DRIVERS)
	Simona Vetter <simona@ffwll.ch> (maintainer:DRM DRIVERS)
	nouveau@lists.freedesktop.org (open list:CORE DRIVER FOR NVIDIA GPUS [RUST])
	dri-devel@lists.freedesktop.org (open list:DRM DRIVERS)
	linux-kernel@vger.kernel.org (open list)

Additionally, it makes sense to also add rust-for-linux@vger.kernel.org.

- Danilo

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-12-13 12:47 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-11  9:30 [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds Quaternions
2025-07-11  9:30 ` [PATCH] gpu: nova-core: define named constants for magic numbers Quaternions
2025-07-11 18:03 ` [PATCH] gpu: nova-core: fix bounds check In PmuLookupTableEntry::new, data is sliced from 2..6, but the bounds check data.len() < 5 does not satisfy those bounds Danilo Krummrich
2025-07-12  1:30   ` Rhys Lloyd
2025-07-12 13:11     ` Danilo Krummrich
  -- strict thread matches above, loose matches on Subject: below --
2025-07-11 11:33 Quaternions
2025-07-11 11:54 ` Greg KH
2025-07-11 12:04 ` Alexandre Courbot
2025-07-11 12:20   ` Alexandre Courbot
2025-07-11 14:32     ` Rhys Lloyd
2025-07-11 14:55       ` Miguel Ojeda
2025-07-11 12:12 ` Miguel Ojeda

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.