Re: [PATCH v2 2/2] usb: typec: fsa4480: Report switch & mux events to codec

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: typec: fsa4480: use-after-free in fsa4480_remove() due to teardown order
--

commit 75fde299467e8267560849403da73d3c9962ad9c
Author: Luca Weiss <luca.weiss@fairphone.com>

usb: typec: fsa4480: Report switch & mux events to codec

This patch updates the FSA4480 driver to acquire a downstream Type-C mux
and orientation switch for the audio codec and forward mux and orientation
events to it. It also enables the SENSE and MIC bits when entering audio
accessory mode.

> diff --git a/drivers/usb/typec/mux/fsa4480.c b/drivers/usb/typec/mux/fsa4480.c
> index bea0c1deec94d..93faa81d08797 100644
> --- a/drivers/usb/typec/mux/fsa4480.c
> +++ b/drivers/usb/typec/mux/fsa4480.c

[ ... ]

> @@ -319,18 +347,33 @@ static int fsa4480_probe(struct i2c_client *client)
>  
>  	fsa->mux = typec_mux_register(dev, &mux_desc);
>  	if (IS_ERR(fsa->mux)) {
> -		typec_switch_unregister(fsa->sw);
> -		return dev_err_probe(dev, PTR_ERR(fsa->mux), "failed to register typec mux\n");
> +		ret = dev_err_probe(dev, PTR_ERR(fsa->mux),
> +				    "failed to register typec mux\n");
> +		goto err_unregister_switch;
>  	}
>  
>  	i2c_set_clientdata(client, fsa);
>  	return 0;
> +
> +err_unregister_switch:
> +	typec_switch_unregister(fsa->sw);
> +
> +err_put_switch:
> +	typec_switch_put(fsa->codec_switch);
> +
> +err_put_mux:
> +	typec_mux_put(fsa->codec);
> +
> +	return ret;
>  }
>  
>  static void fsa4480_remove(struct i2c_client *client)
>  {
>  	struct fsa4480 *fsa = i2c_get_clientdata(client);
>  
> +	typec_mux_put(fsa->codec);
> +	typec_switch_put(fsa->codec_switch);
> +
>  	typec_mux_unregister(fsa->mux);
>  	typec_switch_unregister(fsa->sw);
>  }

[Severity: High]
Can releasing fsa->codec and fsa->codec_switch before unregistering
fsa->mux and fsa->sw cause a use-after-free in fsa4480_mux_set() or
fsa4480_switch_set()?

In drivers/usb/typec/mux.c, both typec_mux_put() and typec_switch_put()
drop their underlying device and module references and immediately call
kfree() on the handle:

drivers/usb/typec/mux.c:typec_mux_put() {
	...
	for (i = 0; i < mux->num_mux_devs; i++) {
		mux_dev = mux->mux_devs[i];
		module_put(mux_dev->dev.parent->driver->owner);
		put_device(&mux_dev->dev);
	}
	kfree(mux);
}

Because fsa->mux and fsa->sw are still registered when typec_mux_put()
and typec_switch_put() are called in fsa4480_remove(), a concurrent call
to fsa4480_mux_set() or fsa4480_switch_set() during removal will pass the
freed fsa->codec or fsa->codec_switch pointer to typec_mux_set() or
typec_switch_set().

Should fsa4480_remove() unregister fsa->mux and fsa->sw before calling
typec_switch_put() and typec_mux_put(), matching the reverse order used
in the error unwind path of fsa4480_probe()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020081343.1507986-1-val@packett.cool?part=2