linux-kernel - Re: [PATCH] ALSA: usb-audio: Fix double-free in snd_usb_add_audio

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-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 31 Mar 2016 10:03:55 -0400 (EDT)
From:	Vladis Dronov <vdronov@...hat.com>
To:	Takashi Iwai <tiwai@...e.de>
Cc:	Jaroslav Kysela <perex@...ex.cz>, alsa-devel@...a-project.org,
	linux-kernel@...r.kernel.org, linux-sound@...r.kernel.org
Subject: Re: [PATCH] ALSA: usb-audio: Fix double-free in
 snd_usb_add_audio_stream()

Hello, Takashi, all,

> No, it has nothing to do with the double-free bug itself.  Such an
> optimization shouldn't be put in a fix patch

This piece of code move alone fixes the double-free bug in
create_fixed_stream_quirk(), so I believe it is related. Besides, a lot of stuff
is created and initialized in snd_usb_add_audio_stream() and while I do not see
another use-after-free bug, it could be there. By moving this code we avoid
these potential bugs we have not hit yet.

But anyway. If you still believe this code should not be moved, please, confirm,
I'll suggest the next patch version without it.

> Vladis, if you take someone's patch as the base, you have to carry the
> original authorship somewhere...

Yes, I was thinking about it, I was just not sure how should I do it. Will the
following form be fine? Or somehow else?

Based on a patch by Takashi Iwai" <tiwai@...e.de>

> > + * if not, create a new pcm stream. the caller must remove fp from
> > + * the substream fmt_list in the error path to avoid double-free.
>
> This comment isn't true.  The caller may leave it as is, too, like my
> first version.  It just depends on the code.

Yes. Is the following rewrite acceptable for the next patch version?

 * if not, create a new pcm stream. Note, fp is added to the substream fmt_list
 * and will be freed on the chip instance release. Do not free fp or do remove
 * it from the substream fmt_list to avoid double-free.

Best regards,
Vladis Dronov | Red Hat, Inc. | Product Security Engineer