Re: [PATCH v4 2/2] gpu: nova-core: gsp: fix undefined behavior in command queue code

[Alexandre Courbot]

On Sat Apr 4, 2026 at 9:05 AM JST, Danilo Krummrich wrote:
> On Sat Apr 4, 2026 at 1:47 AM CEST, John Hubbard wrote:
>> On 4/1/26 7:29 AM, Alexandre Courbot wrote:
>>> `driver_read_area` and `driver_write_area` are internal methods that
>>> return slices containing the area of the command queue buffer that the
>>> driver has exclusive read or write access, respectively.
>>> 
>>> While their returned value is correct and safe to use, internally they
>>> temporarily create a reference to the whole command-buffer slice,
>>> including GSP-owned regions. These regions can change without notice,
>>> and thus creating a slice to them, even if never accessed, is undefined
>>> behavior.
>>> 
>>> Fix this by rewriting these methods to use pointer projections in order
>>> to create slices to valid regions only. It should eventually be replaced
>>> by `IoView` and `IoSlice` once they land.
>>> 
>>> Fixes: 75f6b1de8133 ("gpu: nova-core: gsp: Add GSP command queue bindings and handling")
>>> Reported-by: Danilo Krummrich <dakr@xxxxxxxxxx>
>>> Closes: https://lore.kernel.org/all/DH47AVPEKN06.3BERUSJIB4M1R@xxxxxxxxxx/
>>> Reviewed-by: Gary Guo <gary@xxxxxxxxxxx>
>>> Reviewed-by: Danilo Krummrich <dakr@xxxxxxxxxx>
>>> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
>>> ---
>>>  drivers/gpu/nova-core/gsp/cmdq.rs | 114 ++++++++++++++++++++++----------------
>>>  1 file changed, 65 insertions(+), 49 deletions(-)
>>
>> This is causing a build_assert failure in the latest drm-rust-next, with
>> rustc 1.85.0, and also with 1.78.0.
>>
>> rustc 1.93.0 does not show the problem.
>
> Odd, it did pass all the testing at my end. Anyways, it only proves once again
> that this is pretty fragile.

Same here, although I did not test with all the supported rustc versions
(but pretty sure I did test 1.78). This seems to be heavily
configuration dependent.

Gary, do you know of a way to restrict the `project_pointer` macro to
only take constant values as indices? That problem is pretty sneaky and
likely to happen again, so we should probably harden that part a bit.