[PATCH] gpu: nova-core: fix bounds check in PmuLookupTableEntry::new

[PATCH] gpu: nova-core: fix bounds check in PmuLookupTableEntry::new

[Rhys Lloyd]

data is sliced from 2..6, but the bounds check data.len() < 5
does not satisfy those bounds.

Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")

Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
---
Changes in v2:
- Ensure commit description does not spill into commit message
- Fix author to match SoB
- Add "Fixes:" tag
- Add base commit

---
 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);
         }
 

base-commit: 215a3f91713383a3c0d2da82d223a608a3c17ac1
-- 
2.50.1

[PATCH] gpu: nova-core: define named constants for magic numbers

[Rhys Lloyd]

Introduce an associated constant `MIN_LEN` for each struct that checks
the length of the input data in its constructor against a magic number.

Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
---
Changes in v2:
- Add commit description
- Fix author to match SoB
- Add base commit

---
 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);
         }
 

base-commit: 215a3f91713383a3c0d2da82d223a608a3c17ac1
prerequisite-patch-id: d80f92d314a0693d4c89ffb7810d9ab6990336fa
-- 
2.50.1

Re: [PATCH] gpu: nova-core: fix bounds check in PmuLookupTableEntry::new

[Alexandre Courbot]

On Sun Jul 13, 2025 at 11:51 AM JST, Rhys Lloyd wrote:
> data is sliced from 2..6, but the bounds check data.len() < 5
> does not satisfy those bounds.
>
> Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
>
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
> ---
> Changes in v2:

Since this is a v2, the message subject should have read "[PATCH v2]".
You can achieve this by passing `-v 2` to git format-patch.

No need to resend, just pointing it out for next time. :)

For the fix in itself,

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH] gpu: nova-core: define named constants for magic numbers

[Alexandre Courbot]

On Sun Jul 13, 2025 at 11:51 AM JST, Rhys Lloyd wrote:
> Introduce an associated constant `MIN_LEN` for each struct that checks
> the length of the input data in its constructor against a magic number.
>
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>

As I mentioned in [1], I think this would be better addressed by working
in terms of `sizeof` upon the relevant structures, after making them
`#[repr(C)]`. It might require splitting them a bit since some contain
other data (or we can maybe turn them into DSTs).

[1] https://lore.kernel.org/rust-for-linux/DB97X8JAJFI4.3G1I8ZPC1MWLS@nvidia.com/

Re: [PATCH] gpu: nova-core: define named constants for magic numbers

[Rhys Lloyd]

On 7/13/25 8:11 PM, Alexandre Courbot wrote:
> On Sun Jul 13, 2025 at 11:51 AM JST, Rhys Lloyd wrote:
>> Introduce an associated constant `MIN_LEN` for each struct that checks
>> the length of the input data in its constructor against a magic number.
>>
>> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>
> As I mentioned in [1], I think this would be better addressed by working
> in terms of `sizeof` upon the relevant structures, after making them
> `#[repr(C)]`. It might require splitting them a bit since some contain
> other data (or we can maybe turn them into DSTs).
>
> [1] https://lore.kernel.org/rust-for-linux/DB97X8JAJFI4.3G1I8ZPC1MWLS@nvidia.com/

As far as I can tell, only one of the five structs with `MIN_LEN` have 
the same layout in-memory as they do in the `data` byte slice, that 
being `BitHeader`.  Perhaps `#[repr(packed)]` could be used for 
`PmuLookupTableEntry`, sacrificing alignment, but that is undesirable as 
it comes with its own footguns such as unaligned loads.  The other 
structs include optional values and vectors which do not have the same 
encoding when reading from the `data` byte slice as they do in memory.  
I have worked with DSTs before, but I don't recommend them for 
non-library code since they are not first-class citizens in Rust.  
Notably the fat pointer is not resized when taking a reference to the 
unsized struct field, and constructing such objects is cumbersome.  
Also, in the current version of Rust (1.88), DSTs cannot yet live 
comfortably on the stack.

This patch can be dropped if it's not valuable enough to warrant the 
change, I only made it because of your comment here: 
https://gitlab.freedesktop.org/drm/nova/-/merge_requests/4#note_2999761

Re: [PATCH] gpu: nova-core: fix bounds check in PmuLookupTableEntry::new

[Alexandre Courbot]

On Sun Jul 13, 2025 at 11:51 AM JST, Rhys Lloyd wrote:
> data is sliced from 2..6, but the bounds check data.len() < 5
> does not satisfy those bounds.
>
> Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
>
> Signed-off-by: Rhys Lloyd <krakow20@gmail.com>

Applied to nova-next, thanks!