linux-kernel - Re: [PATCH v4 19/20] gpu: nova-core: extract FWSEC from BIOS and patch it to run FWSEC-FRTS

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DAKDFE2ZH4PL.3BGYOZUID5CNF@nvidia.com>
Date: Thu, 12 Jun 2025 16:20:14 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Danilo Krummrich" <dakr@...nel.org>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor"
 <alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
 <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <benno.lossin@...ton.me>,
 "Andreas Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl"
 <aliceryhl@...gle.com>, "Trevor Gross" <tmgross@...ch.edu>, "David Airlie"
 <airlied@...il.com>, "Simona Vetter" <simona@...ll.ch>, "Maarten Lankhorst"
 <maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
 "Thomas Zimmermann" <tzimmermann@...e.de>, "John Hubbard"
 <jhubbard@...dia.com>, "Ben Skeggs" <bskeggs@...dia.com>, "Joel Fernandes"
 <joelagnelf@...dia.com>, "Timur Tabi" <ttabi@...dia.com>, "Alistair Popple"
 <apopple@...dia.com>, <linux-kernel@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>, <nouveau@...ts.freedesktop.org>,
 <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v4 19/20] gpu: nova-core: extract FWSEC from BIOS and
 patch it to run FWSEC-FRTS

On Wed Jun 4, 2025 at 7:42 PM JST, Danilo Krummrich wrote:
> On Wed, May 21, 2025 at 03:45:14PM +0900, Alexandre Courbot wrote:
>> +impl FirmwareDmaObject<FwsecFirmware> {
>> +    /// Patch the Fwsec firmware image in `fw` to run the command `cmd`.
>> +    fn patch_command(&mut self, v3_desc: &FalconUCodeDescV3, cmd: FwsecCommand) -> Result<()> {
>
> Same comment as on the previous patch regarding patch_signature().

This one can be easily fixed, contrary to the previous one. The
constructor now takes the command to patch and does it here (as it makes
no sense to run FWSEC without the command patched in).

>
> <snip>
>
>> +    fn dmem_load_params(&self) -> FalconLoadTarget {
>> +        FalconLoadTarget {
>> +            src_start: self.desc.imem_load_size,
>> +            dst_start: self.desc.dmem_phys_base,
>> +            len: Layout::from_size_align(self.desc.dmem_load_size as usize, 256)
>> +                // Cannot panic, as 256 is non-zero and a power of 2.
>> +                .unwrap()
>
> There is also Layout::from_size_align_unchecked(), which I prefer over unwrap().
> I think we should never use unwrap() and rather the unsafe variant, which at least
> forces us to document things properly, if there's no other option.
>
> In this case, however, I don't see why we can't just propage the error? This
> method is used from Falcon::dma_load(), which returns a Result anyways, so let's
> just propagate it.
>
> In general, we should *never* potentially panic the whole kernel just because
> of a wrong size calculation in a driver.

Good point. In this case we can do something even simpler, which is
use the `align_up` method introduced in the `num` module.