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=-8.0 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,URIBL_BLOCKED,USER_AGENT_SANE_1 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 79945CA9ECF for ; Mon, 4 Nov 2019 11:04:06 +0000 (UTC) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 49E1822472 for ; Mon, 4 Nov 2019 11:04:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=oracle.com header.i=@oracle.com header.b="ZkSFcjBe" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 49E1822472 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=oracle.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=driverdev-devel-bounces@linuxdriverproject.org Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 1EDD98AA4D; Mon, 4 Nov 2019 11:04:06 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id UM8HZiYJzTBk; Mon, 4 Nov 2019 11:04:05 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by hemlock.osuosl.org (Postfix) with ESMTP id 706218AA34; Mon, 4 Nov 2019 11:04:05 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by ash.osuosl.org (Postfix) with ESMTP id 740E81BF361 for ; Mon, 4 Nov 2019 11:04:04 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 6F92C895C9 for ; Mon, 4 Nov 2019 11:04:04 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Umh4j_69OpoP for ; Mon, 4 Nov 2019 11:04:03 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from aserp2120.oracle.com (aserp2120.oracle.com [141.146.126.78]) by fraxinus.osuosl.org (Postfix) with ESMTPS id BF561895C6 for ; Mon, 4 Nov 2019 11:04:03 +0000 (UTC) Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.27/8.16.0.27) with SMTP id xA4Aspen009835; Mon, 4 Nov 2019 11:04:01 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=date : from : to : cc : subject : message-id : references : mime-version : content-type : in-reply-to; s=corp-2019-08-05; bh=oh9GH8wwoClSgQg02TQwxOW2wUym3I4ltRRW2QDdUmI=; b=ZkSFcjBepb5Cn8ydb20IE6DxCgFEHeYhfzbFj/jMRNHUcFgYEUF73K+u9IG1Z72TOpom gy+jXwfwhuMpJPZMZQe0lKHzHVobAHu9SwQXXqH9URNO/lfE2Es+ANEhZx6MOYUf6aKn McjbcHPdnLiBEIixhSGg2MkM9yOfoBsf39C02zisi8bfD/9gwDiNq0YLzqGGYY3KTXQK tznzvi1TxfRU+kiNETpRIBbKnijsgFm+PUfn/mcfNC+/U46xnCd4wn4fJEmfvqYS9gQl 4CC4u5Klhd5JhKh3W8IjPZ8Z3kzjGSP+n/bEj6+g3aKBgI66G6dvgtwgu+LClLuoDN3w tg== Received: from userp3020.oracle.com (userp3020.oracle.com [156.151.31.79]) by aserp2120.oracle.com with ESMTP id 2w11rppg8m-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 04 Nov 2019 11:04:01 +0000 Received: from pps.filterd (userp3020.oracle.com [127.0.0.1]) by userp3020.oracle.com (8.16.0.27/8.16.0.27) with SMTP id xA4Asstb142626; Mon, 4 Nov 2019 11:04:00 GMT Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by userp3020.oracle.com with ESMTP id 2w1kxd2503-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 04 Nov 2019 11:04:00 +0000 Received: from abhmp0004.oracle.com (abhmp0004.oracle.com [141.146.116.10]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id xA4B3xwR006069; Mon, 4 Nov 2019 11:03:59 GMT Received: from kadam (/41.57.98.10) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 04 Nov 2019 03:03:58 -0800 Date: Mon, 4 Nov 2019 14:03:52 +0300 From: Dan Carpenter To: Jerome Pouiller Subject: Re: [PATCH v2] staging: wfx: Fix a memory leak in 'wfx_upload_beacon' Message-ID: <20191104110352.GD10409@kadam> References: <20191102155945.20205-1-christophe.jaillet@wanadoo.fr> <4126113.6ZGdBP45BV@pc-42> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4126113.6ZGdBP45BV@pc-42> User-Agent: Mutt/1.9.4 (2018-02-28) X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9430 signatures=668685 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=2 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1908290000 definitions=main-1911040109 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9430 signatures=668685 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=2 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1908290000 definitions=main-1911040109 X-BeenThere: driverdev-devel@linuxdriverproject.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux Driver Project Developer List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "devel@driverdev.osuosl.org" , "gregkh@linuxfoundation.org" , "kernel-janitors@vger.kernel.org" , Christophe JAILLET , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: "devel" On Mon, Nov 04, 2019 at 10:42:43AM +0000, Jerome Pouiller wrote: > On Saturday 2 November 2019 16:59:45 CET Christophe JAILLET wrote: > > The current code is a no-op, because all it can do is 'dev_kfree_skb(NULL)' > > Remove the test before 'dev_kfree_skb()' > > > > Signed-off-by: Christophe JAILLET > > --- > > V2: remove the 'if(...)', 'dev_kfree_skb()' can handle NULL. > > --- > > drivers/staging/wfx/sta.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c > > index 688586e823c0..93f3739b5f3a 100644 > > --- a/drivers/staging/wfx/sta.c > > +++ b/drivers/staging/wfx/sta.c > > @@ -906,8 +906,7 @@ static int wfx_upload_beacon(struct wfx_vif *wvif) > > wfx_fwd_probe_req(wvif, false); > > > > done: > > - if (!skb) > > - dev_kfree_skb(skb); > > + dev_kfree_skb(skb); > > return ret; > > } > > > > -- > > 2.20.1 > > > > In add, value of skb is tested earlier in function. So, it is guaranteed to be > never NULL. See the start of the function: 868 static int wfx_upload_beacon(struct wfx_vif *wvif) 869 { 870 int ret = 0; 871 struct sk_buff *skb = NULL; 872 struct ieee80211_mgmt *mgmt; 873 struct hif_mib_template_frame *p; 874 875 if (wvif->vif->type == NL80211_IFTYPE_STATION || 876 wvif->vif->type == NL80211_IFTYPE_MONITOR || 877 wvif->vif->type == NL80211_IFTYPE_UNSPECIFIED) 878 goto done; ^^^^^^^^^ This why I argue that goto done is monkey poop. You see this and you wonder "what on earth does goto done do?" We haven't allocated anything so what are we going to free? The name doesn't give any hints at all. So you have to scroll down. Now you have lost your place and cleared out your short term memory about the code that you were reading. But then you get to goto done and you see it is a complicated no op, and it return ret. So you have to scroll back up to the very start of the function to see what ret is. And the you wonder, is this really a success path? It could be deliberate or it could be accidental. Who knows! The "goto done;" is ambiguous and take a long time to understand but "return 0;" is instantly clear. 879 880 skb = ieee80211_beacon_get(wvif->wdev->hw, wvif->vif); 881 882 if (!skb) 883 return -ENOMEM; 884 885 p = (struct hif_mib_template_frame *) skb_push(skb, 4); regards, dan carpenter _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel