Re: [PATCH v2 08/31] gpu: nova-core: add optional ucodes firmware loading

[John Hubbard]

On 8/23/26 9:26 AM, M Henning wrote:
> On Fri, Aug 21, 2026 at 9:58 PM John Hubbard <jhubbard@xxxxxxxxxx> wrote:
> > +        // FILE is a basename relative to the TLV's directory, not an arbitrary firmware path.
> > +        if file.is_empty() || matches!(file, "." | "..") || file.contains('/') {
> > +            return Err(EINVAL);
> > +        }
>
> This isn't a sane way to check if a path includes a directory
> component or not. You really want to use path-handling apis, rather
> than open-coding this in a way that rejects a few special cases.

I'll drop these checks entirely for v3. Timur wondered whether they
were needed during the v1 review, for the same reason, and I think
you're both right that the check that matters already happens lower
down.

Although, load_file()'s doccomments do claim that an embedded '/'
is rejected, so I'll need to either delete that, or add in a check
for it.

Here's the calling stack:

    Tlv::load_file()
      firmware::request_into_buf()
        request_firmware_into_buf()
          _request_firmware()

and _request_firmware() already does this:

	if (!name || name[0] == '\0') {
		ret = -EINVAL;
		goto out;
	}
	/*
	 * Reject firmware file names with ".." path components.
	 * There are drivers that construct firmware file names from
	 * device-supplied strings, and we don't want some device to be
	 * able to tell us "I would like to be sent my firmware from
	 * ../../../etc/shadow, please".
	 *
	 * This intentionally only looks at the firmware name, not at
	 * the firmware base directory or at symlink contents.
	 */
	if (name_contains_dotdot(name)) {
		dev_warn(device,
			 "Firmware load for '%s' refused, path contains '..' component\n",
			 name);
		ret = -EINVAL;
		goto out;
	}

On the path-handling API: there isn't one to reach for in Rust for
Linux:

rust/kernel is #![no_std], so no Path or PathBuf, and the C side has
kbasename() and the VFS lookup helpers, but nothing that validates an
unresolved name string beyond the name_contains_dotdot() above.

thanks,
--
John Hubbard