[PATCH v19 7/8] i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality

[sashiko-bot at kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] i3c: UAF of i3c_master_controller due to devres and device_unregister mismatch
- [High] i3c: Race condition in target port connection causes early transfers to fail
--

commit 9976472402f2b50b56d71088f609b4b246b5e249
Author: Aman Kumar Pandey <aman.kumarpandey at nxp.com>

    i3c: hub: p3h2x4x: Add support for NXP P3H2x4x I3C hub functionality
    
    Add I3C hub functionality for the NXP P3H2x4x family of multiport hubs.
    These devices support downstream target ports that can be configured
    as I3C, I2C, or SMBus.

> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_common.c b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
> --- /dev/null
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_common.c
[ ... ]
> +static int p3h2x4x_i3c_hub_probe(struct platform_device *pdev)
> +{
> +	struct p3h2x4x *p3h2x4x = dev_get_drvdata(pdev->dev.parent);
> +	struct p3h2x4x_i3c_hub_dev *p3h2x4x_i3c_hub;
> +	struct device *dev = &pdev->dev;
> +	struct device_node *node;
> +	int ret, i;
> +
> +	p3h2x4x_i3c_hub = devm_kzalloc(dev, sizeof(*p3h2x4x_i3c_hub), GFP_KERNEL);
> +	if (!p3h2x4x_i3c_hub)
> +		return -ENOMEM;

[ ... ]

> diff --git a/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
> --- /dev/null
> +++ b/drivers/i3c/hub/p3h2840_i3c_hub_i3c.c
[ ... ]
> +static void p3h2x4x_unregister_i3c_master(void *data)
> +{
> +	struct i3c_master_controller *controller = data;
> +
> +	i3c_master_unregister(controller);
> +}
[ ... ]
> +int p3h2x4x_tp_i3c_algo(struct p3h2x4x_i3c_hub_dev *p3h2x4x_hub)
> +{
[ ... ]
> +	for (tp = 0; tp < p3h2x4x->num_target_ports; tp++) {
[ ... ]
> +		ret = i3c_master_register_fwnode(controller,
> +						 p3h2x4x_hub->dev,
> +						 of_fwnode_handle(p3h2x4x_hub->tp_bus[tp].of_node),
> +						 &i3c_hub_master_ops,
> +						 false);
> +		if (ret)
> +			return ret;
> +
> +		ret = devm_add_action_or_reset(p3h2x4x_hub->dev,
> +					       p3h2x4x_unregister_i3c_master,
> +					       controller);
> +		if (ret)
> +			return ret;

[Severity: High]
Does this create a use-after-free risk during driver removal?

The p3h2x4x_i3c_hub structure embeds the i3c_master_controller instances
and is allocated with devm_kzalloc() in p3h2x4x_i3c_hub_probe(). When the
driver unbinds, devres will execute p3h2x4x_unregister_i3c_master() which
calls i3c_master_unregister().

Unlike i2c_del_adapter(), i3c_master_unregister() calls device_unregister()
without waiting for the device reference count to reach zero. Because the
devm cleanup phase executes without blocking for final device release, the
devm_kzalloc memory can be freed before the deferred release callback 
(i3c_masterdev_release) runs. If userspace holds a reference (e.g., via
sysfs), this will result in a use-after-free crash when the final reference
is dropped.

> +
> +		ntwk_mask |= p3h2x4x_hub->tp_bus[tp].tp_mask;
> +		p3h2x4x_hub->tp_bus[tp].is_registered = true;
> +		p3h2x4x_hub->hub_config.tp_config[tp].always_enable = true;
> +	}
> +	return regmap_write(p3h2x4x_hub->regmap, P3H2X4X_TP_NET_CON_CONF, ntwk_mask);
> +}

[Severity: High]
Does this leave a race window where early transfers on newly registered ports
can fail?

When i3c_master_register_fwnode() (called earlier in the loop) returns, the
virtual master is exposed to the system and can begin accepting transfers.
Immediately after, always_enable is set to true, disabling the software
enable_port callback logic. However, the hardware port enable write to
P3H2X4X_TP_NET_CON_CONF happens outside the loop.

If a child device probes or a transfer is initiated on a port before the
loop finishes, the enable_port callback will check always_enable, see it is
true, and return without enabling the port in hardware. This would send the
transfer to a disconnected physical port and cause I/O errors or timeouts.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916070728.1088801-1-lakshay.piplani@nxp.com?part=7