From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Eliot Courtney" <ecourtney@nvidia.com>
Cc: "Danilo Krummrich" <dakr@kernel.org>,
"Alice Ryhl" <aliceryhl@google.com>,
"David Airlie" <airlied@gmail.com>,
"Simona Vetter" <simona@ffwll.ch>,
"Joel Fernandes" <joelagnelf@nvidia.com>,
"John Hubbard" <jhubbard@nvidia.com>,
"Alistair Popple" <apopple@nvidia.com>,
"Timur Tabi" <ttabi@nvidia.com>, <rust-for-linux@vger.kernel.org>,
<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 01/11] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`
Date: Wed, 29 Apr 2026 22:24:36 +0900 [thread overview]
Message-ID: <DI5O794IQL4T.1TUZKR7LI9MOS@nvidia.com> (raw)
In-Reply-To: <20260421-fix-vbios-v3-1-8f648aef7a85@nvidia.com>
On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
> Fix various cases that allow reading past `BIOS_MAX_SCAN_LEN` when
> scanning the VBIOS.
>
> Fix bug where `read_more_at_offset` would unnecessarily read more data.
> This happens when the window to read has some part cached and some part
> not. It would read `len` bytes instead of just the uncached portion,
> which could read past `BIOS_MAX_SCAN_LEN`.
>
> Also add more checked arithmetic to catch potential overflows.
> `read_bios_image_at_offset` is called with a length from the VBIOS
> header, so we should be more defensive here.
This reads like this patch is doing 3 different things, or at least two,
since the second chunk (`read_bios_image_at_offset`) does not seem
related to `BIOS_MAX_SCAN_LEN`.
The general rule is that one patch should do one thing - the trick here
will be to either update the message to describe a larger thing (and not
3 small ones), or to split the patch. Both are acceptable IMHO.
>
> Fixes: 6fda04e7f0cd ("gpu: nova-core: vbios: Add base support for VBIOS construction and iteration")
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> drivers/gpu/nova-core/vbios.rs | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index ebda28e596c5..6de7e58e0da0 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -132,17 +132,14 @@ fn read_more(&mut self, len: usize) -> Result {
>
> /// Read bytes at a specific offset, filling any gap.
> fn read_more_at_offset(&mut self, offset: usize, len: usize) -> Result {
> - if offset > BIOS_MAX_SCAN_LEN {
> + let end = offset.checked_add(len).ok_or(EINVAL)?;
> +
> + if end > BIOS_MAX_SCAN_LEN {
> dev_err!(self.dev, "Error: exceeded BIOS scan limit.\n");
> return Err(EINVAL);
> }
>
> - // If `offset` is beyond current data size, fill the gap first.
> - let current_len = self.data.len();
> - let gap_bytes = offset.saturating_sub(current_len);
> -
> - // Now read the requested bytes at the offset.
> - self.read_more(gap_bytes + len)
> + self.read_more(end.saturating_sub(self.data.len()))
> }
>
> /// Read a BIOS image at a specific offset and create a [`BiosImage`] from it.
> @@ -155,8 +152,9 @@ fn read_bios_image_at_offset(
> len: usize,
> context: &str,
> ) -> Result<BiosImage> {
> + let end = offset.checked_add(len).ok_or(EINVAL)?;
> let data_len = self.data.len();
> - if offset + len > data_len {
> + if end > data_len {
nit: `data_len` is only used on this line, so it can if `if end >
self.data.len() {`.
Otherwise these fixes look quite needed inded.
next prev parent reply other threads:[~2026-04-29 13:24 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-21 8:20 [PATCH v3 00/11] gpu: nova-core: vbios: harden various array accesses and refactor Eliot Courtney
2026-04-21 8:20 ` [PATCH v3 01/11] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN` Eliot Courtney
2026-04-29 13:24 ` Alexandre Courbot [this message]
2026-04-29 14:32 ` Joel Fernandes
2026-05-01 5:15 ` Eliot Courtney
2026-04-21 8:20 ` [PATCH v3 02/11] gpu: nova-core: vbios: limit `BitToken` entry reads Eliot Courtney
2026-04-29 13:35 ` Alexandre Courbot
2026-05-01 5:38 ` Eliot Courtney
2026-04-21 8:20 ` [PATCH v3 03/11] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode` Eliot Courtney
2026-04-29 13:50 ` Alexandre Courbot
2026-04-21 8:20 ` [PATCH v3 04/11] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header` Eliot Courtney
2026-04-29 13:56 ` Alexandre Courbot
2026-05-01 6:07 ` Eliot Courtney
2026-04-21 8:20 ` [PATCH v3 05/11] gpu: nova-core: vbios: use checked accesses in `setup_falcon_data` Eliot Courtney
2026-04-21 8:20 ` [PATCH v3 06/11] gpu: nova-core: vbios: drop unused falcon_data_offset from FwSecBiosBuilder Eliot Courtney
2026-04-21 8:20 ` [PATCH v3 07/11] gpu: nova-core: vbios: keep PmuLookupTable local in setup_falcon_data Eliot Courtney
2026-04-21 8:20 ` [PATCH v3 08/11] gpu: nova-core: vbios: compute FWSEC-relative Falcon data offset Eliot Courtney
2026-04-21 8:20 ` [PATCH v3 09/11] gpu: nova-core: vbios: simplify setup_falcon_data Eliot Courtney
2026-04-29 14:28 ` Alexandre Courbot
2026-04-21 8:20 ` [PATCH v3 10/11] gpu: nova-core: vbios: construct `FwSecBiosImage` directly from BIOS images Eliot Courtney
2026-04-21 8:20 ` [PATCH v3 11/11] gpu: nova-core: vbios: use the first PCI-AT and FWSEC images Eliot Courtney
2026-04-29 14:32 ` Alexandre Courbot
2026-04-29 17:49 ` Gary Guo
2026-05-01 10:55 ` Eliot Courtney
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=DI5O794IQL4T.1TUZKR7LI9MOS@nvidia.com \
--to=acourbot@nvidia.com \
--cc=airlied@gmail.com \
--cc=aliceryhl@google.com \
--cc=apopple@nvidia.com \
--cc=dakr@kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=ecourtney@nvidia.com \
--cc=jhubbard@nvidia.com \
--cc=joelagnelf@nvidia.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=simona@ffwll.ch \
--cc=ttabi@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 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.