linux-kernel - Re: BUG: KASAN: use-after-free in snd_complete_urb+0x109e/0x1740 [snd_usb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <s5hpn1d1vt6.wl-tiwai@suse.de>
Date:   Sat, 06 Feb 2021 08:48:05 +0100
From:   Takashi Iwai <tiwai@...e.de>
To:     Hillf Danton <hdanton@...a.com>
Cc:     Mikhail Gavrilov <mikhail.v.gavrilov@...il.com>, zonque@...il.com,
        LKML <linux-kernel@...r.kernel.org>, alsa-devel@...a-project.org,
        linux-usb@...r.kernel.org
Subject: Re: BUG: KASAN: use-after-free in snd_complete_urb+0x109e/0x1740 [snd_usb_audio] (5.11-rc6)

On Sat, 06 Feb 2021 06:45:32 +0100,
Hillf Danton wrote:
> 
> Due to the reconnecting key word mentioned, no fix to
> d0f09d1e4a88 ("ALSA: usb-audio: Refactoring endpoint URB deactivation")
> will be added.
> 
> What is added is to capture EP_FLAG_STOPPING and remove the one
> second wait limit if the reconnecting acts may make it easier to
> repro the uaf. The diff is only for idea show.

If my understanding is right, this won't change.  The problem is
rather the lack of this function call itself, i.e. the missing
synchronization for the stream stop.

It worked casually in the past because the endpoint resource is
released at a later point that is after all streams are really closed.
Now it's released earlier and hitting the UAF.


Takashi

> 
> --- a/sound/usb/endpoint.c
> +++ b/sound/usb/endpoint.c
> @@ -832,24 +832,14 @@ void snd_usb_endpoint_suspend(struct snd
>   */
>  static int wait_clear_urbs(struct snd_usb_endpoint *ep)
>  {
> -	unsigned long end_time = jiffies + msecs_to_jiffies(1000);
> -	int alive;
> -
> -	if (!test_bit(EP_FLAG_STOPPING, &ep->flags))
> -		return 0;
> -
> +	WARN_ON_ONCE(!test_bit(EP_FLAG_STOPPING, &ep->flags));
>  	do {
> -		alive = bitmap_weight(&ep->active_mask, ep->nurbs);
> -		if (!alive)
> +		if (!bitmap_weight(&ep->active_mask, ep->nurbs))
>  			break;
>  
>  		schedule_timeout_uninterruptible(1);
> -	} while (time_before(jiffies, end_time));
> +	} while (1);
>  
> -	if (alive)
> -		usb_audio_err(ep->chip,
> -			"timeout: still %d active urbs on EP #%x\n",
> -			alive, ep->ep_num);
>  	clear_bit(EP_FLAG_STOPPING, &ep->flags);
>  
>  	ep->sync_sink = NULL;
>