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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ee459198-04f2-4bae-8e1f-4ec413d92f89@nvidia.com>
Date: Wed, 21 Jan 2026 19:26:16 -0800
From: John Hubbard <jhubbard@...dia.com>
To: Eliot Courtney <ecourtney@...dia.com>, Danilo Krummrich
 <dakr@...nel.org>, Alexandre Courbot <acourbot@...dia.com>,
 Alice Ryhl <aliceryhl@...gle.com>, David Airlie <airlied@...il.com>,
 Simona Vetter <simona@...ll.ch>, Alistair Popple <apopple@...dia.com>
Cc: nouveau@...ts.freedesktop.org, rust-for-linux@...r.kernel.org,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] gpu: nova-core: gsp: fix improper handling of empty
 slot in cmdq

On 1/21/26 6:59 PM, Eliot Courtney wrote:
> +            // The area is contiguous and we leave an empty slot before `rx`.
> +            // PANIC: since `rx > tx` we have `rx - tx - 1 >= 0`
> +            // PANIC: since `tx < rx < MSGQ_NUM_PAGES && after_tx.len() == MSGQ_NUM_PAGES - tx`:
> +            //   `rx - 1 <= MSGQ_NUM_PAGES` -> `rx - tx - 1 <= MSGQ_NUM_PAGES - tx`
> +            //   -> `rx - tx - 1 <= after_tx.len()`

Hi Eliot,

Documentation nit: the proofs are great, but the above just does
not go into my head easily, because it's a proof, rather than a
sentence.

Can you please reword these PANIC comments so that they are complete
sentences, along the lines of:

    // PANIC: a > b and therefore c cannot overflow, therefore this
    // cannot panic.

And please also use words "and", "therefore", "because", instead of 
symbols such as "&&".

Same for the other patch with PANIC comments.

I did a quick search in Rust for Linux, before writing this, in order to
ensure that what I'm recommending is How It Is Done. (Not to claim that
Nova in particular is fully correct, though.)


thanks,
-- 
John Hubbard


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ