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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1013f667-c11f-25a2-ab2b-87b9368ad456@linux.intel.com>
Date:   Thu, 26 Jan 2023 09:50:14 -0600
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Wesley Cheng <quic_wcheng@...cinc.com>,
        srinivas.kandagatla@...aro.org, mathias.nyman@...el.com,
        perex@...ex.cz, lgirdwood@...il.com, andersson@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, gregkh@...uxfoundation.org,
        Thinh.Nguyen@...opsys.com, broonie@...nel.org,
        bgoswami@...cinc.com, tiwai@...e.com, robh+dt@...nel.org,
        agross@...nel.org
Cc:     devicetree@...r.kernel.org, alsa-devel@...a-project.org,
        linux-arm-msm@...r.kernel.org, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, quic_jackp@...cinc.com,
        quic_plai@...cinc.com
Subject: Re: [RFC PATCH v2 12/22] sound: usb: card: Introduce USB SND platform
 op callbacks




> +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.