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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux