Re: [RFC PATCH v2 12/22] sound: usb: card: Introduce USB SND platform op callbacks

From: Wesley Cheng
Date: Mon Jan 30 2023 - 18:20:36 EST


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(&register_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