diff mbox series

[v2] ALSA: usb-audio: Fix potential out-of-bound accesses for Extigy devices

Message ID 20241119142813.3528722-1-bsevens@google.com (mailing list archive)
State New
Headers show
Series [v2] ALSA: usb-audio: Fix potential out-of-bound accesses for Extigy devices | expand

Commit Message

Benoît Sevens Nov. 19, 2024, 2:28 p.m. UTC 
A bogus devices can provide a bNumConfigurations value that exceeds the
initial value used in usb_get_configuration for allocating dev->config.

This can lead to out-of-bounds accesses later, e.g. in
usb_destroy_configuration.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@kernel.org
Signed-off-by: Benoît Sevens <bsevens@google.com>
---
v2:
 - Also add sanity checks to snd_usb_mbox2_boot_quirk and 
   snd_usb_mbox3_boot_quirk
---
 sound/usb/quirks.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Takashi Iwai Nov. 19, 2024, 2:43 p.m. UTC | #1 
On Tue, 19 Nov 2024 15:28:13 +0100,
Benoît Sevens wrote:
> 
> A bogus devices can provide a bNumConfigurations value that exceeds the
> initial value used in usb_get_configuration for allocating dev->config.
> 
> This can lead to out-of-bounds accesses later, e.g. in
> usb_destroy_configuration.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@kernel.org
> Signed-off-by: Benoît Sevens <bsevens@google.com>
> ---
> v2:
>  - Also add sanity checks to snd_usb_mbox2_boot_quirk and 
>    snd_usb_mbox3_boot_quirk

Thanks for the revised patch.

Although we should fix better in a long term, a duct-tape fix like
this wouldn't hurt.

But one thing I'm considering is...

> @@ -565,8 +566,11 @@ static int snd_usb_extigy_boot_quirk(struct usb_device *dev, struct usb_interfac
>  				      0x10, 0x43, 0x0001, 0x000a, NULL, 0);
>  		if (err < 0)
>  			dev_dbg(&dev->dev, "error sending boot message: %d\n", err);
> +		num_configs = dev->descriptor.bNumConfigurations;
>  		err = usb_get_descriptor(dev, USB_DT_DEVICE, 0,
>  				&dev->descriptor, sizeof(dev->descriptor));
> +		if (dev->descriptor.bNumConfigurations > num_configs)
> +			dev->descriptor.bNumConfigurations = num_configs;

IMO, we can get the new descriptor locally, then do
compare-and-store.  If bNumConfiguration is unexpectedly large, it can
return an error without modifying the original descriptor.


Takashi
diff mbox series

Patch

diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c
index c5fd180357d1..919091c1f17a 100644
--- a/sound/usb/quirks.c
+++ b/sound/usb/quirks.c
@@ -555,6 +555,7 @@  int snd_usb_create_quirk(struct snd_usb_audio *chip,
 static int snd_usb_extigy_boot_quirk(struct usb_device *dev, struct usb_interface *intf)
 {
 	struct usb_host_config *config = dev->actconfig;
+	int num_configs;
 	int err;
 
 	if (le16_to_cpu(get_cfg_desc(config)->wTotalLength) == EXTIGY_FIRMWARE_SIZE_OLD ||
@@ -565,8 +566,11 @@  static int snd_usb_extigy_boot_quirk(struct usb_device *dev, struct usb_interfac
 				      0x10, 0x43, 0x0001, 0x000a, NULL, 0);
 		if (err < 0)
 			dev_dbg(&dev->dev, "error sending boot message: %d\n", err);
+		num_configs = dev->descriptor.bNumConfigurations;
 		err = usb_get_descriptor(dev, USB_DT_DEVICE, 0,
 				&dev->descriptor, sizeof(dev->descriptor));
+		if (dev->descriptor.bNumConfigurations > num_configs)
+			dev->descriptor.bNumConfigurations = num_configs;
 		config = dev->actconfig;
 		if (err < 0)
 			dev_dbg(&dev->dev, "error usb_get_descriptor: %d\n", err);
@@ -901,6 +905,7 @@  static void mbox2_setup_48_24_magic(struct usb_device *dev)
 static int snd_usb_mbox2_boot_quirk(struct usb_device *dev)
 {
 	struct usb_host_config *config = dev->actconfig;
+	int num_configs;
 	int err;
 	u8 bootresponse[0x12];
 	int fwsize;
@@ -935,8 +940,11 @@  static int snd_usb_mbox2_boot_quirk(struct usb_device *dev)
 
 	dev_dbg(&dev->dev, "device initialised!\n");
 
+	num_configs = dev->descriptor.bNumConfigurations;
 	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0,
 		&dev->descriptor, sizeof(dev->descriptor));
+	if (dev->descriptor.bNumConfigurations > num_configs)
+		dev->descriptor.bNumConfigurations = num_configs;
 	config = dev->actconfig;
 	if (err < 0)
 		dev_dbg(&dev->dev, "error usb_get_descriptor: %d\n", err);
@@ -1249,6 +1257,7 @@  static void mbox3_setup_defaults(struct usb_device *dev)
 static int snd_usb_mbox3_boot_quirk(struct usb_device *dev)
 {
 	struct usb_host_config *config = dev->actconfig;
+	int num_configs;
 	int err;
 	int descriptor_size;
 
@@ -1261,8 +1270,11 @@  static int snd_usb_mbox3_boot_quirk(struct usb_device *dev)
 
 	dev_dbg(&dev->dev, "MBOX3: device initialised!\n");
 
+	num_configs = dev->descriptor.bNumConfigurations;
 	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0,
 		&dev->descriptor, sizeof(dev->descriptor));
+	if (dev->descriptor.bNumConfigurations > num_configs)
+		dev->descriptor.bNumConfigurations = num_configs;
 	config = dev->actconfig;
 	if (err < 0)
 		dev_dbg(&dev->dev, "MBOX3: error usb_get_descriptor: %d\n", err);