Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757126AbcCaOEG (ORCPT ); Thu, 31 Mar 2016 10:04:06 -0400 Received: from mx5-phx2.redhat.com ([209.132.183.37]:37302 "EHLO mx5-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752504AbcCaOEE (ORCPT ); Thu, 31 Mar 2016 10:04:04 -0400 Date: Thu, 31 Mar 2016 10:03:55 -0400 (EDT) From: Vladis Dronov To: Takashi Iwai Cc: Jaroslav Kysela , alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, linux-sound@vger.kernel.org Message-ID: <805340847.43676741.1459433035733.JavaMail.zimbra@redhat.com> In-Reply-To: References: <1459364602-28997-1-git-send-email-vdronov@redhat.com> <1459364602-28997-2-git-send-email-vdronov@redhat.com> <882494379.43642280.1459427790768.JavaMail.zimbra@redhat.com> Subject: Re: [PATCH] ALSA: usb-audio: Fix double-free in snd_usb_add_audio_stream() MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.40.1.159] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF45 (Linux)/8.0.6_GA_5922) Thread-Topic: ALSA: usb-audio: Fix double-free in snd_usb_add_audio_stream() Thread-Index: Au3J1+0NoenQW3EL6FxOlLxGmVbYGA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1512 Lines: 36 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" > > + * 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