On Mon, 03 Dec 2018 14:40:19 +0100, Mathias Payer wrote:

Hi there,

We are submitting a patch for an UAF write in usb_audio_probe. A malicious USB device can send a broken device configuration that will trigger a free of the underlying object before it is decremented. I'm sending the patch and KASAN report after discussing with Takashi on the security mailing list so that he can merge.

The attacker needs local access to plug in a malicious USB device that replays the trace (e.g., through FaceDancer) to get read/write primitives in the kernel. For, e.g., Android or locked Desktops this becomes security critical.

This bug is present in 4.14.81 through to HEAD (i.e., all versions we tested).

The place where UAF is triggered is in `usb_audio_probe` in `sound/usb/card.c`. In the error handling code in `usb_audio_probe`, if the number of interfaces read from the device side is zero, `chip->card` object is freed. Unfortunately, the `chip` object is part of the `card` object, thus, when `chip->card` is freed, `chip` is also freed.

However, in the error handling code, the `active` field will be written to by an `atomic_dec` operation, resulting in an UAF write.

static int usb_audio_probe(struct usb_interface *intf,
   						const struct usb_device_id *usb_id)
.....

 __error:
if (chip) {
   	if (!chip->num_interfaces)
   		snd_card_free(chip->card);
   	// UAF write
   	atomic_dec(&chip->active);
   }
   mutex_unlock(&register_mutex);
   return err;

The attacker can race the snd_card_free and the atomic_dec by attaching new USB devices (the attacker can time when what parts of the first malicious device are read then play with attaching the second device; the race is arbitrarily repeatable). The new USB device descriptors will use the recently freed memory which is then modified by the atomic_dec, resulting in an attacker-controlled decrement operation to, e.g., a USB data structure under the attacker's control.

We have also attached the KASAN report from when the bug was triggered.

The patch for card.c is attached. It moves the atomic_dec to above the free of the chip memory object, ensuring that, if no cards are left, the decrement happens before the free. We've also added a comment to clarify that the two objects are dependent.

Thanks for the report. I applied the patch now in for-linus branch.

Takashi