public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Joel Fernandes" <joelagnelf@nvidia.com>
Cc: linux-kernel@vger.kernel.org, "Paul Walmsley" <pjw@kernel.org>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <lossin@kernel.org>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Alistair Popple" <apopple@nvidia.com>,
	"Zhi Wang" <zhiw@nvidia.com>, "Simona Vetter" <simona@ffwll.ch>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Dirk Behme" <dirk.behme@gmail.com>,
	nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
	rust-for-linux@vger.kernel.org, linux-riscv@lists.infradead.org
Subject: Re: [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing
Date: Wed, 28 Jan 2026 17:08:10 +0900	[thread overview]
Message-ID: <DG02HED94PLY.EZY2AUCW4LOL@nvidia.com> (raw)
In-Reply-To: <DG02AC8I2XUA.2UM92327TQKAL@nvidia.com>

On Wed Jan 28, 2026 at 4:58 PM JST, Alexandre Courbot wrote:
> On Tue Jan 27, 2026 at 5:23 AM JST, Joel Fernandes wrote:
>> Use checked_add() and checked_mul() when computing offsets from
>> firmware-provided values in new_fwsec().
>>
>> Without checked arithmetic, corrupt firmware could cause integer overflow. The
>> danger is not just wrapping to a huge value, but potentially wrapping to a
>> small plausible offset that passes validation yet accesses entirely wrong data,
>> causing silent corruption or security issues.
>>
>> Reviewed-by: Zhi Wang <zhiw@nvidia.com>
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/firmware/fwsec.rs | 60 ++++++++++++++-----------
>>  1 file changed, 35 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs b/drivers/gpu/nova-core/firmware/fwsec.rs
>> index a8ec08a500ac..71541b1f07d7 100644
>> --- a/drivers/gpu/nova-core/firmware/fwsec.rs
>> +++ b/drivers/gpu/nova-core/firmware/fwsec.rs
>> @@ -46,10 +46,7 @@
>>          Signed,
>>          Unsigned, //
>>      },
>> -    num::{
>> -        FromSafeCast,
>> -        IntoSafeCast, //
>> -    },
>> +    num::FromSafeCast,
>>      vbios::Vbios,
>>  };
>>  
>> @@ -267,7 +264,12 @@ fn new_fwsec(dev: &Device<device::Bound>, bios: &Vbios, cmd: FwsecCommand) -> Re
>>          let ucode = bios.fwsec_image().ucode(&desc)?;
>>          let mut dma_object = DmaObject::from_data(dev, ucode)?;
>>  
>> -        let hdr_offset = usize::from_safe_cast(desc.imem_load_size() + desc.interface_offset());
>> +        // Compute hdr_offset = imem_load_size + interface_offset.
>> +        let hdr_offset = desc
>> +            .imem_load_size()
>> +            .checked_add(desc.interface_offset())
>> +            .map(usize::from_safe_cast)
>> +            .ok_or(EINVAL)?;
>>          // SAFETY: we have exclusive access to `dma_object`.
>
> Missing empty line before the SAFETY comment (also in other places).
>
> I will fix when applying, no need to resend.

I also got this clippy warning when building:

		warning: unsafe block missing a safety comment
			--> ../drivers/gpu/nova-core/firmware/fwsec.rs:303:17
				|
		303 |                 unsafe { transmute_mut(&mut dma_object, dmem_mapper_offset) }?;
				|                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
				|
				= help: consider adding a safety comment on the preceding line
				= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks
				= note: requested on the command line with `-W clippy::undocumented-unsafe-blocks`

		warning: unsafe block missing a safety comment
			--> ../drivers/gpu/nova-core/firmware/fwsec.rs:319:17
				|
		319 |                 unsafe { transmute_mut(&mut dma_object, frts_cmd_offset) }?;
				|                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
				|
				= help: consider adding a safety comment on the preceding line
				= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks

		warning: 2 warnings emitted

Since the `unsafe` keyword has moved to a new line, its SAFETY comment needed
to be moved right above it, despite it still being part of the same statement.
I'll fix this as well.

  reply	other threads:[~2026-01-28  8:08 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-26 20:23 [PATCH v2 0/5] gpu: nova-core: use checked arithmetic for firmware parsing robustness Joel Fernandes
2026-01-26 20:23 ` [PATCH v2 1/5] gpu: nova-core: use checked arithmetic in FWSEC firmware parsing Joel Fernandes
2026-01-28  7:58   ` Alexandre Courbot
2026-01-28  8:08     ` Alexandre Courbot [this message]
2026-01-28 15:30       ` Joel Fernandes
2026-01-28 10:53   ` Danilo Krummrich
2026-01-28 15:14     ` Joel Fernandes
2026-01-29  0:20       ` Danilo Krummrich
2026-01-29  0:36         ` Alexandre Courbot
2026-01-29  0:42           ` Joel Fernandes
2026-01-29  0:58         ` John Hubbard
2026-02-03 22:24           ` Alexandre Courbot
2026-02-04 18:54             ` Miguel Ojeda
2026-02-04 21:08               ` Joel Fernandes
2026-01-26 20:23 ` [PATCH v2 2/5] gpu: nova-core: use checked arithmetic in Booter signature parsing Joel Fernandes
2026-01-26 20:23 ` [PATCH v2 3/5] gpu: nova-core: use checked arithmetic in frombytes_at helper Joel Fernandes
2026-01-26 20:23 ` [PATCH v2 4/5] gpu: nova-core: use checked arithmetic in BinFirmware::data Joel Fernandes
2026-01-26 20:23 ` [PATCH v2 5/5] gpu: nova-core: use checked arithmetic in RISC-V firmware parsing Joel Fernandes
2026-01-27 13:54 ` [PATCH v2 0/5] gpu: nova-core: use checked arithmetic for firmware parsing robustness Gary Guo
2026-01-28  7:59 ` Alexandre Courbot
2026-02-25  0:59   ` Alexandre Courbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DG02HED94PLY.EZY2AUCW4LOL@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=a.hindborg@kernel.org \
    --cc=alex.gaynor@gmail.com \
    --cc=alex@ghiti.fr \
    --cc=aliceryhl@google.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=apopple@nvidia.com \
    --cc=bhelgaas@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=dirk.behme@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=lossin@kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    --cc=ojeda@kernel.org \
    --cc=palmer@dabbelt.com \
    --cc=pjw@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tmgross@umich.edu \
    --cc=zhiw@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox