lkml.org 
[lkml]   [2023]   [Jan]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RFC PATCH v2 12/22] sound: usb: card: Introduce USB SND platform op callbacks
From
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

\
 
 \ /
  Last update: 2023-03-27 00:02    [W:0.184 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site