Re: [PATCH 5/9] watchdog: w83627hf_wdt: Add Super I/O configuration port as a platform device resource

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized stack memory in `struct resource res` causes a kernel panic during device registration.
- [High] Modeling the globally shared Super I/O configuration port as an exclusive platform device resource breaks mutual exclusion (muxing) for other drivers.
--

--- Patch [5]: [PATCH 5/9] watchdog: w83627hf_wdt: Add Super I/O configuration port as a platform device resource ---
commit da82fc7c463fccfdf1ba03b6c7b6e7de29a800a0
Author: Paul Louvel <paul.louvel@bootlin.com>

watchdog: w83627hf_wdt: Add Super I/O configuration port as a platform device resource

This patch modifies the w83627hf watchdog driver to pass the Super I/O
configuration port as a platform device resource instead of using a global
variable. It updates the driver initialization to retrieve the base address
from the platform resource during probe.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index f5333c4321984..6363d571182fa 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
[ ... ]
> @@ -598,9 +601,14 @@ static int __init wdt_init(void)
>  	if (ret)
>  		return ret;
>  
> -	wdt_pdev = platform_device_register_data(NULL, wdt_ids[chip].name,
> -						 PLATFORM_DEVID_NONE, &pdata,
> -						 sizeof(pdata));
> +	res.name = "Super I/O port";
> +	res.flags = IORESOURCE_IO;
> +	res.start = sioaddr;
> +	res.end = sioaddr + 1;

[Severity: High]
Is this struct resource fully initialized before being passed to
platform_device_register_resndata()?

Because it is allocated on the stack in wdt_init(), fields like parent,
sibling, and child will contain uninitialized stack data. When the resource
is copied during platform device registration, these uninitialized pointers
are preserved.

Later, when platform_device_add() is called, it checks the parent pointer.
If the stack garbage is non-zero, it skips the fallback to ioport_resource
and passes the garbage pointer to insert_resource(), which will cause a
kernel panic when traversing the resource tree.

Could this be fixed by zero-initializing the struct when it is declared?

> +
> +	wdt_pdev = platform_device_register_resndata(NULL, wdt_ids[chip].name,
> +						     PLATFORM_DEVID_NONE, &res,
> +						     1, &pdata, sizeof(pdata));

[Severity: High]
Will registering the Super I/O configuration port as a static platform device
resource break the mutual exclusion for other drivers sharing this port?

The Super I/O configuration port is a globally shared resource, and drivers
like hwmon dynamically access it using request_muxed_region(). Registering
it statically here inserts it into the global I/O resource tree as belonging
to the watchdog device.

If another driver holds the muxed region and the watchdog module is
unloaded, release_resource() will forcefully remove the watchdog's resource
and orphan any children. This breaks the IORESOURCE_MUXED wait queue logic,
allowing a third driver to concurrently claim the region, potentially leading
to hardware data corruption or a deadlock when the original driver fails to
release the region.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725-w83627hf_wdt-improvements-v1-0-4e9a1b4e8297@bootlin.com?part=5