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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The function `hwpath_to_device` returns an unrefcounted pointer to a `struct device`, leading to a potential Use-After-Free if the device is concurrently removed.

Pre-existing issues:
- [High] Unbounded sprintf formatting in `print_pa_hwpath` and `print_pci_hwpath` can overflow the statically sized 24-byte `pa_path` buffer inside `struct sti_struct`.
--

--- Patch [1]: [PATCH v2] sticon/parisc: Detect default STI graphics card for console output ---
commit 29c9337b8cdefb96f4918bde459cea7d9955e767
Author: Helge Deller <deller@gmx.de>

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

This commit detects the graphics card used for firmware messages and sets
it as the default for sticon and fbcon. It extracts the hardware path
from stable storage or PAGE0 and compares it to identify the default device.
The default_sti_path string length is increased to 32 characters to prevent
buffer overflows during hardware path formatting.

>     Increase max string size for default_sti_path to 32 chars as 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/...), which can produce a string up to 28 bytes long.

[Severity: High]
This is a pre-existing issue, but since the commit message correctly identifies
that print_pa_hwpath() and print_pci_hwpath() can produce a string up to
28 bytes long, does this also overflow the pa_path buffer inside
struct sti_struct?

include/video/sticore.h:
    struct sti_struct {
        ...
        char pa_path[24];
        ...
    }

Since sticore_pa_init() and sticore_pci_init() pass sti->pa_path directly to
these functions, could this overflow the 24-byte pa_path array and corrupt
adjacent heap memory during device enumeration?

> diff --git a/drivers/video/sticore.c b/drivers/video/sticore.c
> index 0d37e4b10447e..1d4477f204504 100644
> --- a/drivers/video/sticore.c
> +++ b/drivers/video/sticore.c

[ ... ]

> @@ -1148,6 +1148,26 @@ 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 && dev_is_pci(dev))

[Severity: Medium]
Does hwpath_to_device() return an unrefcounted device pointer?

Looking at hwpath_to_device() in arch/parisc/kernel/drivers.c, it seems to
use device_for_each_child() to find the device but returns the pointer without
elevating its reference count.

If the console hardware is concurrently hot-unplugged after hwpath_to_device()
returns, could this lead to a use-after-free when calling dev_is_pci(dev) or
to_pci_dev(dev) on the freed device pointer?

> +			print_pci_hwpath(to_pci_dev(dev), default_sti_path);
> +		else if (dev && !dev_is_pci(dev))
> +			print_pa_hwpath(to_parisc_device(dev), default_sti_path);
> +		pr_debug("default graphic card: %s\n", default_sti_path);
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806150414.7870-2-deller@kernel.org?part=1