Hi Pierre,
On 1/26/2023 7:50 AM, Pierre-Louis Bossart wrote:
+int snd_usb_register_platform_ops(struct snd_usb_platform_ops *ops) +{
- if (platform_ops)
return -EEXIST;- platform_ops = ops;
- return 0;
+} +EXPORT_SYMBOL_GPL(snd_usb_register_platform_ops);
+int snd_usb_unregister_platform_ops(void) +{
- platform_ops = NULL;
- return 0;
+} +EXPORT_SYMBOL_GPL(snd_usb_unregister_platform_ops);
I find this super-racy.
If the this function is called just before ...
/*
- disconnect streams
@@ -910,6 +928,10 @@ static int usb_audio_probe(struct usb_interface *intf, usb_set_intfdata(intf, chip); atomic_dec(&chip->active); mutex_unlock(®ister_mutex);
- if (platform_ops->connect_cb)
platform_ops->connect_cb(intf, chip);... this, then you have a risk of using a dandling pointer.
You also didn't test that the platform_ops != NULL, so there's a risk of dereferencing a NULL pointer.
Not so good, eh?
It's a classic (I've had the same sort of issues with SoundWire), when you export ops from one driver than can be removed, then additional protection is needed when using those callbacks.
Yep, will take a look at this a bit more to improve it.
Thanks Wesley Cheng