From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 73E91C433FF for ; Wed, 7 Aug 2019 07:18:55 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5290E219BE for ; Wed, 7 Aug 2019 07:18:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387436AbfHGHSx (ORCPT ); Wed, 7 Aug 2019 03:18:53 -0400 Received: from mx2.suse.de ([195.135.220.15]:54980 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1727281AbfHGHSx (ORCPT ); Wed, 7 Aug 2019 03:18:53 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 7A192AF40; Wed, 7 Aug 2019 07:18:52 +0000 (UTC) Date: Wed, 07 Aug 2019 09:18:52 +0200 Message-ID: From: Takashi Iwai To: Wenwen Wang Cc: "moderated list:SOUND" , Thomas Gleixner , Greg Kroah-Hartman , Allison Randal , Jaroslav Kysela , Takashi Iwai , open list Subject: Re: [PATCH v2] ALSA: pcm: fix multiple memory leak bugs In-Reply-To: References: User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 07 Aug 2019 09:09:59 +0200, Wenwen Wang wrote: > > In hiface_pcm_init(), 'rt' is firstly allocated through kzalloc(). Later > on, hiface_pcm_init_urb() is invoked to initialize 'rt->out_urbs[i]'. In > hiface_pcm_init_urb(), 'rt->out_urbs[i].buffer' is allocated through > kzalloc(). However, if hiface_pcm_init_urb() fails, both 'rt' and > 'rt->out_urbs[i].buffer' are not deallocated, leading to memory leak bugs. > Also, 'rt->out_urbs[i].buffer' is not deallocated if snd_pcm_new() fails. > > To fix the above issues, free 'rt' and 'rt->out_urbs[i].buffer'. > > Signed-off-by: Wenwen Wang > --- > sound/usb/hiface/pcm.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/sound/usb/hiface/pcm.c b/sound/usb/hiface/pcm.c > index 14fc1e1..9b132aa 100644 > --- a/sound/usb/hiface/pcm.c > +++ b/sound/usb/hiface/pcm.c > @@ -599,12 +599,18 @@ int hiface_pcm_init(struct hiface_chip *chip, u8 > extra_freq) > for (i = 0; i < PCM_N_URBS; i++) { > ret = hiface_pcm_init_urb(&rt->out_urbs[i], chip, OUT_EP, > hiface_pcm_out_urb_handler); > - if (ret < 0) > + if (ret < 0) { > + for (; i >= 0; i--) > + kfree(rt->out_urbs[i].buffer); > + kfree(rt); > return ret; > + } > } > > ret = snd_pcm_new(chip->card, "USB-SPDIF Audio", 0, 1, 0, &pcm); > if (ret < 0) { > + for (i = 0; i < PCM_N_URBS; i++) > + kfree(rt->out_urbs[i].buffer); > kfree(rt); > dev_err(&chip->dev->dev, "Cannot create pcm instance\n"); > return ret; The fixes look correct, but since we can unconditionally call kfree() for NULL, both error paths can be unified as: for (i = 0; i < PCM_N_URBS; i++) kfree(rt->out_urbs[i].buffer); kfree(rt); and this would be better to be put in the common path at the end and do "goto error" or such from both places. BTW, your patch doesn't seem cleanly applicable in anyway because the tabs are converted to spaces. Please check the mail setup. Also, please try to make the subject line more unique. This is about hiface driver, so "ALSA: hiface: xxx" should be more appropriate. thanks, Takashi