Skip to content

Commit d0b4f23

Browse files
jainanmol84PlaidCat
authored andcommitted
ALSA: usb-audio: Fix out of bounds reads when finding clock sources
jira VULN-46532 cve CVE-2024-53150 commit-author Takashi Iwai <[email protected]> commit a3dd4d6 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 <[email protected]> Cc: <[email protected]> Link: https://lore.kernel.org/[email protected] Link: https://patch.msgid.link/[email protected] Signed-off-by: Takashi Iwai <[email protected]> (cherry picked from commit a3dd4d6) Signed-off-by: Anmol Jain <[email protected]>
1 parent 326cfbf commit d0b4f23

File tree

1 file changed

+23
-1
lines changed

1 file changed

+23
-1
lines changed

sound/usb/clock.c

+23-1
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ union uac23_clock_multiplier_desc {
3636
struct uac_clock_multiplier_descriptor v3;
3737
};
3838

39+
/* check whether the descriptor bLength has the minimal length */
40+
#define DESC_LENGTH_CHECK(p, proto) \
41+
((proto) == UAC_VERSION_3 ? \
42+
((p)->v3.bLength >= sizeof((p)->v3)) : \
43+
((p)->v2.bLength >= sizeof((p)->v2)))
44+
3945
#define GET_VAL(p, proto, field) \
4046
((proto) == UAC_VERSION_3 ? (p)->v3.field : (p)->v2.field)
4147

@@ -58,20 +64,36 @@ static bool validate_clock_source(void *p, int id, int proto)
5864
{
5965
union uac23_clock_source_desc *cs = p;
6066

67+
if (!DESC_LENGTH_CHECK(cs, proto))
68+
return false;
6169
return GET_VAL(cs, proto, bClockID) == id;
6270
}
6371

6472
static bool validate_clock_selector(void *p, int id, int proto)
6573
{
6674
union uac23_clock_selector_desc *cs = p;
6775

68-
return GET_VAL(cs, proto, bClockID) == id;
76+
if (!DESC_LENGTH_CHECK(cs, proto))
77+
return false;
78+
if (GET_VAL(cs, proto, bClockID) != id)
79+
return false;
80+
/* additional length check for baCSourceID array (in bNrInPins size)
81+
* and two more fields (which sizes depend on the protocol)
82+
*/
83+
if (proto == UAC_VERSION_3)
84+
return cs->v3.bLength >= sizeof(cs->v3) + cs->v3.bNrInPins +
85+
4 /* bmControls */ + 2 /* wCSelectorDescrStr */;
86+
else
87+
return cs->v2.bLength >= sizeof(cs->v2) + cs->v2.bNrInPins +
88+
1 /* bmControls */ + 1 /* iClockSelector */;
6989
}
7090

7191
static bool validate_clock_multiplier(void *p, int id, int proto)
7292
{
7393
union uac23_clock_multiplier_desc *cs = p;
7494

95+
if (!DESC_LENGTH_CHECK(cs, proto))
96+
return false;
7597
return GET_VAL(cs, proto, bClockID) == id;
7698
}
7799

0 commit comments

Comments
 (0)