On Thu, Apr 18, 2024 at 04:35:07PM +0000, Chris Wulff wrote:
> Hang on to the control IDs instead of pointers since those are correctly handled with locks.
> Prevent use of the uac data structure after it has been freed.
> Mark the endpoint as disabled sooner so that freed requests aren't used.
Nit, please wrap your changelog text at 72 columns. running
scripts/checkpatch.pl should show this.
>
> Signed-off-by: Chris Wulff <chris.wulff@xxxxxxxxx>
What commit id does this fix?
> ---
> drivers/usb/gadget/function/u_audio.c | 31 ++++++++++++++++-----------
> 1 file changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index 4a42574b4a7f..bcae95472455 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -57,13 +57,13 @@ struct uac_rtd_params {
>
> /* Volume/Mute controls and their state */
> int fu_id; /* Feature Unit ID */
> - struct snd_kcontrol *snd_kctl_volume;
> - struct snd_kcontrol *snd_kctl_mute;
> + struct snd_ctl_elem_id snd_kctl_volume_id;
> + struct snd_ctl_elem_id snd_kctl_mute_id;
> s16 volume_min, volume_max, volume_res;
> s16 volume;
> int mute;
No tabs? Odd. Not your fault, just a meta-comment.
>
> - struct snd_kcontrol *snd_kctl_rate; /* read-only current rate */
> + struct snd_ctl_elem_id snd_kctl_rate_id; /* read-only current rate */
> int srate; /* selected samplerate */
> int active; /* playback/capture running */
>
> @@ -447,6 +447,8 @@ static inline void free_ep(struct uac_rtd_params *prm, struct usb_ep *ep)
> if (!prm->ep_enabled)
> return;
>
> + prm->ep_enabled = false;
> +
> audio_dev = uac->audio_dev;
> params = &audio_dev->params;
>
> @@ -464,8 +466,6 @@ static inline void free_ep(struct uac_rtd_params *prm, struct usb_ep *ep)
> }
> }
>
> - prm->ep_enabled = false;
> -
> if (usb_ep_disable(ep))
> dev_err(uac->card->dev, "%s:%d Error!\n", __func__, __LINE__);
> }
> @@ -494,14 +494,13 @@ static inline void free_ep_fback(struct uac_rtd_params *prm, struct usb_ep *ep)
> static void set_active(struct uac_rtd_params *prm, bool active)
> {
> // notifying through the Rate ctrl
> - struct snd_kcontrol *kctl = prm->snd_kctl_rate;
> unsigned long flags;
>
> spin_lock_irqsave(&prm->lock, flags);
> if (prm->active != active) {
> prm->active = active;
> snd_ctl_notify(prm->uac->card, SNDRV_CTL_EVENT_MASK_VALUE,
> - &kctl->id);
> + &prm->snd_kctl_rate_id);
> }
> spin_unlock_irqrestore(&prm->lock, flags);
> }
> @@ -792,6 +791,9 @@ int u_audio_set_volume(struct g_audio *audio_dev, int playback, s16 val)
> unsigned long flags;
> int change = 0;
>
> + if (!uac)
> + return 0;
> +
> if (playback)
> prm = &uac->p_prm;
> else
> @@ -807,7 +809,7 @@ int u_audio_set_volume(struct g_audio *audio_dev, int playback, s16 val)
>
> if (change)
> snd_ctl_notify(uac->card, SNDRV_CTL_EVENT_MASK_VALUE,
> - &prm->snd_kctl_volume->id);
> + &prm->snd_kctl_volume_id);
>
> return 0;
> }
> @@ -840,6 +842,9 @@ int u_audio_set_mute(struct g_audio *audio_dev, int playback, int val)
> int change = 0;
> int mute;
>
> + if (!uac)
> + return 0;
How can this happen? Is this a separate fix? Or the same issue?
thanks,
greg k-h
