Re: [PATCH] sticon/parisc: Detect default STI graphics card for console output

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Buffer overflow when formatting hardware path into default_sti_path.
- [High] Invalid downcast of PCI devices to PA-RISC devices causes type confusion and corrupted hardware paths.
--

--- Patch [1]: [PATCH] sticon/parisc: Detect default STI graphics card for console output ---
commit 8dd6681b0fa075edc554f38ec871c9f29bd33be0
Author: Helge Deller <[email protected]>

sticon/parisc: Detect default STI graphics card for console output

This commit adds logic to detect the default graphic card used for firmware
messages. It extracts the hardware path from stable storage or page zero and
stores it as the default STI path for later comparison.

> diff --git a/drivers/video/sticore.c b/drivers/video/sticore.c
> --- a/drivers/video/sticore.c
> +++ b/drivers/video/sticore.c
> @@ -1148,6 +1148,25 @@ static void sti_init_roms(void)
>  	pr_info("STI GSC/PCI core graphics driver "
>  			STI_DRIVERVERSION "\n");
>  
> +	/*
> +	 * Find default console by hardware path which is either stored in
> +	 * console entry in stable storage or alternatively from console path
> +	 * in PAGE0 used by BCH and PDC.
> +	 */
> +	if (!default_sti_path[0]) {
> +		struct pdc_module_path conspath;
> +		struct device *dev = NULL;
> +
> +		if (pdc_stable_read(0x60, &conspath, sizeof(conspath)) == PDC_OK)
> +			dev = hwpath_to_device(&conspath.path);
> +		if (!dev)
> +			dev = hwpath_to_device(&PAGE0->mem_cons.dp.path);
> +		if (dev)
> +			print_pa_hwpath(to_parisc_device(dev),
> +					default_sti_path);

[Severity: Critical]
Does this code overflow default_sti_path[]?

Looking at the definition of default_sti_path in drivers/video/sticore.c,
it is a fixed-size 21-byte array:

static char default_sti_path[21] __read_mostly;

The print_pa_hwpath() function formats a hardware path using unbounded
sprintf calls for up to 6 bus converter components and 1 module component
(e.g., 255/255/...). This can produce a string up to 28 bytes long, which
would write past the end of the 21-byte default_sti_path array and corrupt
adjacent variables in the __read_mostly section.

[Severity: High]
Can this unconditional cast cause type confusion for PCI devices?

If hwpath_to_device() resolves to a PCI device, it returns the generic
struct device embedded within a struct pci_dev. By unconditionally casting
this to a struct parisc_device using to_parisc_device(), we end up with an
invalid pointer.

Due to structural layout differences between pci_dev and parisc_device,
accessing dev->hw_path through this incorrectly cast pointer will read
random bytes from the PCI device memory space instead of the correct path,
causing the console default graphics card matching logic to fail silently
for PCI devices.

> +		pr_debug("default graphic card: %s\n", default_sti_path);
> +	}
> +
>  	/* Register drivers for native & PCI cards */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1