Re: [PATCH v4 3/3] platform/x86: lenovo-yoga-tab2-pro-1380-fastcharger: Bind through normal serdev matching

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] serdev: type confusion in serdev_device_match() via driver id_table
--

commit bc4039918b4164f5e3178978c756b91d89a4bde7
Author: ChunAn Wu <an.wu@canonical.com>

platform/x86: lenovo-yoga-tab2-pro-1380-fastcharger: Bind through normal serdev matching

This commit switches the Lenovo Yoga Tab 2 Pro 1380 fastcharger driver from
manual driver attachment via device_driver_attach() to standard serdev modalias
matching by passing KBUILD_MODNAME to serdev_device_add() and adding an
id_table to yt2_1380_fc_serdev_driver.

> diff --git a/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c b/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c
> index 8551ab4d2c7db..d0bd9d4e3137c 100644
> --- a/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c
> +++ b/drivers/platform/x86/lenovo/yoga-tab2-pro-1380-fastcharger.c
> @@ -219,11 +219,17 @@ static int yt2_1380_fc_serdev_probe(struct serdev_device *serdev)
>  	return 0;
>  }
>  
> +static const struct serdev_device_id yt2_1380_fc_serdev_id[] = {
> +	{ KBUILD_MODNAME },
> +	{ }
> +};
> +
>  static struct serdev_device_driver yt2_1380_fc_serdev_driver = {
>  	.probe = yt2_1380_fc_serdev_probe,
>  	.driver = {
>  		.name = KBUILD_MODNAME,
>  	},
> +	.id_table = yt2_1380_fc_serdev_id,
>  };

[Severity: High]
Does adding id_table to yt2_1380_fc_serdev_driver trigger a type confusion and
out-of-bounds read in serdev_device_match() when matching against
serdev_controller devices on serdev_bus_type?

In drivers/tty/serdev/core.c, serdev_controller_alloc() places controller
devices on serdev_bus_type:

    device_initialize(&ctrl->dev);
    ctrl->dev.type = &serdev_ctrl_type;
    ctrl->dev.bus = &serdev_bus_type;

When yt2_1380_fc_module_init() calls serdev_device_driver_register() for
yt2_1380_fc_serdev_driver (or when a new serdev_controller is added), the
driver core runs serdev_device_match() against every device on
serdev_bus_type, including struct serdev_controller instances:

static int serdev_device_match(struct device *dev, const struct device_driver *drv)
{
	const struct serdev_device *serdev = to_serdev_device(dev);
	const struct serdev_device_driver *sdrv = to_serdev_device_driver(drv);

	if (acpi_driver_match_device(dev, drv))
		return 1;

	if (of_driver_match_device(dev, drv))
		return 1;

	return !!serdev_match_id(sdrv->id_table, serdev->modalias);
}

Because serdev_device_match() does not check is_serdev_device(dev) before
casting dev with to_serdev_device(dev), and yt2_1380_fc_serdev_driver has no
ACPI or OF match table, serdev_device_match() falls through to
serdev_match_id(sdrv->id_table, serdev->modalias) on struct serdev_controller
devices.

In struct serdev_device, modalias[SERDEV_NAME_SIZE] starts at offset
sizeof(struct device) + 4, which in struct serdev_controller overlaps the
struct device *host pointer, ctrl->nr, ctrl->serdev, ctrl->ops, and extends 12
bytes past sizeof(struct serdev_controller). When the overlapping byte at
modalias[0] is non-zero, serdev_match_id() passes that non-string memory and
trailing out-of-bounds bytes to strcmp().

Should serdev_device_match() verify is_serdev_device(dev) before casting dev
to struct serdev_device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024022930.2704030-1-an.wu@canonical.com?part=3