Commit c39f0bc0 authored by Takashi Iwai's avatar Takashi Iwai
Browse files

ALSA: usb-audio: Fix potential leak of pd at parsing UAC3 streams 

At parsing UAC3 streams, we allocate a PD object at each time, and
either assign or free it.  But there is a case where the PD object may
be leaked; namely, in __snd_usb_parse_audio_interface() loop, when an
audioformat shares the same endpoint with others, it's put to a link
and returns from snd_usb_add_audio_stream(), but the PD is forgotten
afterwards.  Overall, the treatment of PD object in the parser code is
a bit flaky, and we should be more careful about the object ownership.

This patch tries to fix the above case and improve the code a bit.
The pd object is now managed with the auto-cleanup in the loop, and
the ownership is updated when the pd object gets assigned to the
stream, which guarantees the release of the leftover object.

Fixes: 7edf3b5e ("ALSA: usb-audio: AudioStreaming Power Domain parsing")
Link: https://patch.msgid.link/20260427151508.12544-1-tiwai@suse.de


Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent b32ae47a

sound/usb/quirks.c

+1 −1
Original line number Diff line number Diff line
@@ -125,7 +125,7 @@ static int add_audio_stream_from_fixed_fmt(struct snd_usb_audio *chip, 


	snd_usb_audioformat_set_sync_ep(chip, fp);



	err = snd_usb_add_audio_stream(chip, stream, fp);

	err = snd_usb_add_audio_stream(chip, stream, fp, NULL);

	if (err < 0)

		return err;

sound/usb/stream.c

+22 −36
Original line number Diff line number Diff line
@@ -79,7 +79,7 @@ static void snd_usb_audio_pcm_free(struct snd_pcm *pcm) 
static void snd_usb_init_substream(struct snd_usb_stream *as,

				   int stream,

				   struct audioformat *fp,

				   struct snd_usb_power_domain *pd)

				   struct snd_usb_power_domain **pdptr)

{

	struct snd_usb_substream *subs = &as->substream[stream];
@@ -105,10 +105,11 @@ static void snd_usb_init_substream(struct snd_usb_stream *as, 
	if (fp->channels > subs->channels_max)

		subs->channels_max = fp->channels;



	if (pd) {

		subs->str_pd = pd;

	if (pdptr && *pdptr) {

		subs->str_pd = *pdptr;

		*pdptr = NULL; /* assigned */

		/* Initialize Power Domain to idle status D1 */

		snd_usb_power_domain_set(subs->stream->chip, pd,

		snd_usb_power_domain_set(subs->stream->chip, subs->str_pd,

					 UAC3_PD_STATE_D1);

	}
@@ -492,11 +493,14 @@ snd_pcm_chmap_elem *convert_chmap_v3(struct uac3_cluster_header_descriptor 
 * if not, create a new pcm stream. note, fp is added to the substream

 * fmt_list and will be freed on the chip instance release. do not free

 * fp or do remove it from the substream fmt_list to avoid double-free.

 *

 * pdptr is optional and can be NULL.  When it's non-NULL and the PD gets

 * assigned to the stream, *pdptr is cleared to NULL upon return.

 */

static int __snd_usb_add_audio_stream(struct snd_usb_audio *chip,

int snd_usb_add_audio_stream(struct snd_usb_audio *chip,

			     int stream,

			     struct audioformat *fp,

				      struct snd_usb_power_domain *pd)

			     struct snd_usb_power_domain **pdptr)



{

	struct snd_usb_stream *as;
@@ -529,7 +533,7 @@ static int __snd_usb_add_audio_stream(struct snd_usb_audio *chip, 
		err = snd_pcm_new_stream(as->pcm, stream, 1);

		if (err < 0)

			return err;

		snd_usb_init_substream(as, stream, fp, pd);

		snd_usb_init_substream(as, stream, fp, pdptr);

		return add_chmap(as->pcm, stream, subs);

	}
@@ -558,7 +562,7 @@ static int __snd_usb_add_audio_stream(struct snd_usb_audio *chip, 
	else

		strscpy(pcm->name, "USB Audio");



	snd_usb_init_substream(as, stream, fp, pd);

	snd_usb_init_substream(as, stream, fp, pdptr);



	/*

	 * Keep using head insertion for M-Audio Audiophile USB (tm) which has a
@@ -576,21 +580,6 @@ static int __snd_usb_add_audio_stream(struct snd_usb_audio *chip, 
	return add_chmap(pcm, stream, &as->substream[stream]);

}



int snd_usb_add_audio_stream(struct snd_usb_audio *chip,

			     int stream,

			     struct audioformat *fp)

{

	return __snd_usb_add_audio_stream(chip, stream, fp, NULL);

}



static int snd_usb_add_audio_stream_v3(struct snd_usb_audio *chip,

				       int stream,

				       struct audioformat *fp,

				       struct snd_usb_power_domain *pd)

{

	return __snd_usb_add_audio_stream(chip, stream, fp, pd);

}



static int parse_uac_endpoint_attributes(struct snd_usb_audio *chip,

					 struct usb_host_interface *alts,

					 int protocol, int iface_no)
@@ -1113,7 +1102,6 @@ snd_usb_get_audioformat_uac3(struct snd_usb_audio *chip, 
		}

	}



	if (pd)

	*pd_out = pd;



	return fp;
@@ -1129,7 +1117,6 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip, 
	struct usb_interface_descriptor *altsd;

	int i, altno, err, stream;

	struct audioformat *fp = NULL;

	struct snd_usb_power_domain *pd = NULL;

	bool set_iface_first;

	int num, protocol;
@@ -1171,6 +1158,12 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip, 
		if (snd_usb_apply_interface_quirk(chip, iface_no, altno))

			continue;



		/* pd may be allocated at snd_usb_get_audioformat_uac3() and

		 * assigned at snd_usb_add_audio_stream(); otherwise it'll be

		 * freed automatically by cleanup at each loop.

		 */

		struct snd_usb_power_domain *pd __free(kfree) = NULL;



		/*

		 * Roland audio streaming interfaces are marked with protocols

		 * 0/1/2, but are UAC 1 compatible.
@@ -1226,23 +1219,16 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip, 
			*has_non_pcm = true;

		if ((fp->fmt_type == UAC_FORMAT_TYPE_I) == non_pcm) {

			audioformat_free(fp);

			kfree(pd);

			fp = NULL;

			pd = NULL;

			continue;

		}



		snd_usb_audioformat_set_sync_ep(chip, fp);



		dev_dbg(&dev->dev, "%u:%d: add audio endpoint %#x\n", iface_no, altno, fp->endpoint);

		if (protocol == UAC_VERSION_3)

			err = snd_usb_add_audio_stream_v3(chip, stream, fp, pd);

		else

			err = snd_usb_add_audio_stream(chip, stream, fp);



		err = snd_usb_add_audio_stream(chip, stream, fp, &pd);

		if (err < 0) {

			audioformat_free(fp);

			kfree(pd);

			return err;

		}

sound/usb/stream.h

+2 −1
Original line number Diff line number Diff line
@@ -7,7 +7,8 @@ int snd_usb_parse_audio_interface(struct snd_usb_audio *chip, 


int snd_usb_add_audio_stream(struct snd_usb_audio *chip,

			     int stream,

			     struct audioformat *fp);

			     struct audioformat *fp,

			     struct snd_usb_power_domain **pdptr);



#endif /* __USBAUDIO_STREAM_H */