* [PATCH 5.10.y] ALSA: usb-audio: Fix out of bounds reads when finding clock sources
@ 2024-12-05 9:29 Benoît Sevens
2024-12-05 9:48 ` Greg KH
2024-12-05 13:34 ` Sasha Levin
0 siblings, 2 replies; 3+ messages in thread
From: Benoît Sevens @ 2024-12-05 9:29 UTC (permalink / raw)
To: stable; +Cc: Takashi Iwai, Benoît Sevens
From: Takashi Iwai <tiwai@suse.de>
The current USB-audio driver code doesn't check bLength of each
descriptor at traversing for clock descriptors. That is, when a
device provides a bogus descriptor with a shorter bLength, the driver
might hit out-of-bounds reads.
For addressing it, this patch adds sanity checks to the validator
functions for the clock descriptor traversal. When the descriptor
length is shorter than expected, it's skipped in the loop.
For the clock source and clock multiplier descriptors, we can just
check bLength against the sizeof() of each descriptor type.
OTOH, the clock selector descriptor of UAC2 and UAC3 has an array
of bNrInPins elements and two more fields at its tail, hence those
have to be checked in addition to the sizeof() check.
Reported-by: Benoît Sevens <bsevens@google.com>
Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/20241121140613.3651-1-bsevens@google.com
Link: https://patch.msgid.link/20241125144629.20757-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
(cherry picked from commit a3dd4d63eeb452cfb064a13862fb376ab108f6a6)
Signed-off-by: Benoît Sevens <bsevens@google.com>
---
sound/usb/clock.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index 514d18a3e07a..197a6b7d8ad6 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -21,6 +21,10 @@
#include "clock.h"
#include "quirks.h"
+/* check whether the descriptor bLength has the minimal length */
+#define DESC_LENGTH_CHECK(p) \
+ (p->bLength >= sizeof(*p))
+
static void *find_uac_clock_desc(struct usb_host_interface *iface, int id,
bool (*validator)(void *, int), u8 type)
{
@@ -38,36 +42,60 @@ static void *find_uac_clock_desc(struct usb_host_interface *iface, int id,
static bool validate_clock_source_v2(void *p, int id)
{
struct uac_clock_source_descriptor *cs = p;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
return cs->bClockID == id;
}
static bool validate_clock_source_v3(void *p, int id)
{
struct uac3_clock_source_descriptor *cs = p;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
return cs->bClockID == id;
}
static bool validate_clock_selector_v2(void *p, int id)
{
struct uac_clock_selector_descriptor *cs = p;
- return cs->bClockID == id;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
+ if (cs->bClockID != id)
+ return false;
+ /* additional length check for baCSourceID array (in bNrInPins size)
+ * and two more fields (which sizes depend on the protocol)
+ */
+ return cs->bLength >= sizeof(*cs) + cs->bNrInPins +
+ 1 /* bmControls */ + 1 /* iClockSelector */;
}
static bool validate_clock_selector_v3(void *p, int id)
{
struct uac3_clock_selector_descriptor *cs = p;
- return cs->bClockID == id;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
+ if (cs->bClockID != id)
+ return false;
+ /* additional length check for baCSourceID array (in bNrInPins size)
+ * and two more fields (which sizes depend on the protocol)
+ */
+ return cs->bLength >= sizeof(*cs) + cs->bNrInPins +
+ 4 /* bmControls */ + 2 /* wCSelectorDescrStr */;
}
static bool validate_clock_multiplier_v2(void *p, int id)
{
struct uac_clock_multiplier_descriptor *cs = p;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
return cs->bClockID == id;
}
static bool validate_clock_multiplier_v3(void *p, int id)
{
struct uac3_clock_multiplier_descriptor *cs = p;
+ if (!DESC_LENGTH_CHECK(cs))
+ return false;
return cs->bClockID == id;
}
--
2.47.0.338.g60cca15819-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 5.10.y] ALSA: usb-audio: Fix out of bounds reads when finding clock sources
2024-12-05 9:29 [PATCH 5.10.y] ALSA: usb-audio: Fix out of bounds reads when finding clock sources Benoît Sevens
@ 2024-12-05 9:48 ` Greg KH
2024-12-05 13:34 ` Sasha Levin
1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2024-12-05 9:48 UTC (permalink / raw)
To: Benoît Sevens; +Cc: stable, Takashi Iwai
On Thu, Dec 05, 2024 at 09:29:25AM +0000, Benoît Sevens wrote:
> From: Takashi Iwai <tiwai@suse.de>
>
> The current USB-audio driver code doesn't check bLength of each
> descriptor at traversing for clock descriptors. That is, when a
> device provides a bogus descriptor with a shorter bLength, the driver
> might hit out-of-bounds reads.
>
> For addressing it, this patch adds sanity checks to the validator
> functions for the clock descriptor traversal. When the descriptor
> length is shorter than expected, it's skipped in the loop.
>
> For the clock source and clock multiplier descriptors, we can just
> check bLength against the sizeof() of each descriptor type.
> OTOH, the clock selector descriptor of UAC2 and UAC3 has an array
> of bNrInPins elements and two more fields at its tail, hence those
> have to be checked in addition to the sizeof() check.
>
> Reported-by: Benoît Sevens <bsevens@google.com>
> Cc: <stable@vger.kernel.org>
> Link: https://lore.kernel.org/20241121140613.3651-1-bsevens@google.com
> Link: https://patch.msgid.link/20241125144629.20757-1-tiwai@suse.de
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> (cherry picked from commit a3dd4d63eeb452cfb064a13862fb376ab108f6a6)
You did _MUCH_ more than just cherry picking this. Please document your
changes somehow, this is much different from the original commit.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 5.10.y] ALSA: usb-audio: Fix out of bounds reads when finding clock sources
2024-12-05 9:29 [PATCH 5.10.y] ALSA: usb-audio: Fix out of bounds reads when finding clock sources Benoît Sevens
2024-12-05 9:48 ` Greg KH
@ 2024-12-05 13:34 ` Sasha Levin
1 sibling, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2024-12-05 13:34 UTC (permalink / raw)
To: stable; +Cc: Benoît Sevens, Sasha Levin
[ Sasha's backport helper bot ]
Hi,
Found matching upstream commit: a3dd4d63eeb452cfb064a13862fb376ab108f6a6
WARNING: Author mismatch between patch and found commit:
Backport author: "=?UTF-8?q?Beno=C3=AEt=20Sevens?=" <bsevens@google.com>
Commit author: Takashi Iwai <tiwai@suse.de>
Status in newer kernel trees:
6.12.y | Present (different SHA1: 164800d08b1a)
6.11.y | Present (different SHA1: 827fb529653f)
6.6.y | Present (different SHA1: e257c95fc2e3)
6.1.y | Present (different SHA1: 88bbac17b356)
5.15.y | Present (different SHA1: a53c8f496d27)
5.10.y | Not found
Note: The patch differs from the upstream commit:
---
1: a3dd4d63eeb45 < -: ------------- ALSA: usb-audio: Fix out of bounds reads when finding clock sources
-: ------------- > 1: ffc4b5492cccd ALSA: usb-audio: Fix out of bounds reads when finding clock sources
---
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.10.y | Success | Success |
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-12-05 14:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-05 9:29 [PATCH 5.10.y] ALSA: usb-audio: Fix out of bounds reads when finding clock sources Benoît Sevens
2024-12-05 9:48 ` Greg KH
2024-12-05 13:34 ` Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox