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: 7edf3b5e6a45 ("ALSA: usb-audio: AudioStreaming Power Domain parsing")
Link: https://patch.msgid.link/20260427151508.12544-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index 7b803ad58487f9c1ce5a993b29157871ff67a45d..0b4ecc2c6bcc40aeace8b33b544e1c27f13c5513 100644 (file)
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -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;
 

diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index 6c51226f771b67b2c21f04100e083f13ec7db266..f8f56ace56523550442e54bec2d73a185e2da71a 100644 (file)
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -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 stream,
-                                     struct audioformat *fp,
-                                     struct snd_usb_power_domain *pd)
+int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
+                            int stream,
+                            struct audioformat *fp,
+                            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,8 +1102,7 @@ found_clock:
                }
        }
 
-       if (pd)
-               *pd_out = 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;
                }
 

diff --git a/sound/usb/stream.h b/sound/usb/stream.h
index d92e18d5818feebbf1ce8f6b6b4290fecd5d46d0..61b9a133da018a08d36dea28247266c8cc3113a9 100644 (file)
--- a/sound/usb/stream.h
+++ b/sound/usb/stream.h
@@ -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 */