| Message ID | 1668416136-33530-1-git-send-email-zhangchangzhong@huawei.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp2040302wru;
Mon, 14 Nov 2022 00:57:57 -0800 (PST)
X-Google-Smtp-Source:
AA0mqf5RNezRn4pm7WGSAeyenSbD1RY3Y6+fWGoqdM8bz3KwJLt2aqp/9PKdLB8TPl0BG8rE1SkC
X-Received: by 2002:a17:907:76f1:b0:7ae:129b:2d3a with SMTP id
kg17-20020a17090776f100b007ae129b2d3amr9355731ejc.552.1668416277056;
Mon, 14 Nov 2022 00:57:57 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1668416277; cv=none;
d=google.com; s=arc-20160816;
b=v7LB6VuRey7/4mDq46OLggAGokeF15UbcolMbyayeCMHZyEHLWqLpAnYEiD9zhn4rT
DEg3FDlvPsDKXfKOik+EPCBR7YYPzLbq85BLdCelterSKF3ZgWIAnph/nXq4LO2sewnR
6oPyLywv9Bnk6r2aS8Hd3iLeZNkNE1r+VNgAP+CtfHcAzaWW6PK/LVkF359KHX2Pl8Dq
u5zpbPoi+ESVEsSrnk9/CYW0rTVA4pEco9rIlgFJhFRPhTACo45HYyLQMiTGxdVrMZ2/
C6Fzkuz89USvYSedtOVUjg0DlpbtdDMLpH1p7GppYWvgM6udWXYcI/0RvKwTWq+MWWrk
hVsA==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=list-id:precedence:mime-version:message-id:date:subject:cc:to:from;
bh=+0CX1Hi3SCCPcUGghqvxGpzbUpH5idSu6cRySCXORs8=;
b=PlKiA6fv8KfoJ0PaXFbKx2vdQTGWHLee00JS6AjnfAGKL1eZkwNoA3d6pUgdBayRr6
YkYxR1ykXh4Qt07UM5n3MCpUVoA3cUR0OdskecF0s/KY5RureUaEJ4DlNnbAvd8y5bQ4
pTznYhRA8D/YniQpi0wEZhH65WkxldeEYUIvCR5u5ev1w1GHoPpd0UlCooy5ZxKxzsV5
/12OLjxFRVEPBSIMOQhqJFRhGTV4GIaHkJKkHOnb9ro1qrZA+8BvPSB29xviwfokAmFU
MxLYMIfK3EFQZPXuaSnLHOWNHbRrr4lueldNGTVY/7zuJyPbjPA9e0AbYg4Oli/KWYz+
VHIA==
ARC-Authentication-Results: i=1; mx.google.com;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
k9-20020a17090646c900b007ac60b82ea5si6481675ejs.96.2022.11.14.00.57.33;
Mon, 14 Nov 2022 00:57:57 -0800 (PST)
Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
client-ip=2620:137:e000::1:20;
Authentication-Results: mx.google.com;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org;
dmarc=fail (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=huawei.com
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S236533AbiKNIfu (ORCPT <rfc822;winker.wchi@gmail.com>
+ 99 others); Mon, 14 Nov 2022 03:35:50 -0500
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54082 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S236527AbiKNIfr (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Mon, 14 Nov 2022 03:35:47 -0500
Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C580F1B9E8;
Mon, 14 Nov 2022 00:35:46 -0800 (PST)
Received: from dggpeml500020.china.huawei.com (unknown [172.30.72.54])
by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4N9jLD0l7FzHvw1;
Mon, 14 Nov 2022 16:35:16 +0800 (CST)
Received: from dggpeml500006.china.huawei.com (7.185.36.76) by
dggpeml500020.china.huawei.com (7.185.36.88) with Microsoft SMTP Server
(version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
15.1.2375.31; Mon, 14 Nov 2022 16:35:29 +0800
Received: from localhost.localdomain (10.175.112.70) by
dggpeml500006.china.huawei.com (7.185.36.76) with Microsoft SMTP Server
(version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
15.1.2375.31; Mon, 14 Nov 2022 16:35:29 +0800
From: Zhang Changzhong <zhangchangzhong@huawei.com>
To: "David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>,
Paolo Abeni <pabeni@redhat.com>,
Moritz Fischer <mdf@kernel.org>
CC: Zhang Changzhong <zhangchangzhong@huawei.com>,
<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: [PATCH net] net: nixge: fix potential memory leak in
nixge_start_xmit()
Date: Mon, 14 Nov 2022 16:55:35 +0800
Message-ID: <1668416136-33530-1-git-send-email-zhangchangzhong@huawei.com>
X-Mailer: git-send-email 1.8.3.1
MIME-Version: 1.0
Content-Type: text/plain
X-Originating-IP: [10.175.112.70]
X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To
dggpeml500006.china.huawei.com (7.185.36.76)
X-CFilter-Loop: Reflected
X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_MED,
SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1749461266303503846?=
X-GMAIL-MSGID: =?utf-8?q?1749461266303503846?=
|
| Series |
[net] net: nixge: fix potential memory leak in nixge_start_xmit()
|
|
Commit Message
Zhang Changzhong
Nov. 14, 2022, 8:55 a.m. UTC
The nixge_start_xmit() returns NETDEV_TX_OK but does not free skb on two
error handling cases, which can lead to memory leak.
To fix this, return NETDEV_TX_BUSY in case of nixge_check_tx_bd_space()
fails and add dev_kfree_skb_any() in case of dma_map_single() fails.
Fixes: 492caffa8a1a ("net: ethernet: nixge: Add support for National Instruments XGE netdev")
Signed-off-by: Zhang Changzhong <zhangchangzhong@huawei.com>
---
drivers/net/ethernet/ni/nixge.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
Comments
Zhang Changzhong <zhangchangzhong@huawei.com> : > The nixge_start_xmit() returns NETDEV_TX_OK but does not free skb on two > error handling cases, which can lead to memory leak. > > To fix this, return NETDEV_TX_BUSY in case of nixge_check_tx_bd_space() > fails and add dev_kfree_skb_any() in case of dma_map_single() fails. This patch merge two unrelated changes. Please split. > Fixes: 492caffa8a1a ("net: ethernet: nixge: Add support for National Instruments XGE netdev") > Signed-off-by: Zhang Changzhong <zhangchangzhong@huawei.com> > --- > drivers/net/ethernet/ni/nixge.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c > index 19d043b593cc..b9091f9bbc77 100644 > --- a/drivers/net/ethernet/ni/nixge.c > +++ b/drivers/net/ethernet/ni/nixge.c > @@ -521,13 +521,15 @@ static netdev_tx_t nixge_start_xmit(struct sk_buff *skb, > if (nixge_check_tx_bd_space(priv, num_frag)) { > if (!netif_queue_stopped(ndev)) > netif_stop_queue(ndev); > - return NETDEV_TX_OK; > + return NETDEV_TX_BUSY; > } The driver should probably check the available room before returning from hard_start_xmit and turn the check above unlikely(). Btw there is no lock and the Tx completion is irq driven: the driver is racy. :o(
On 2022/11/14 18:40, Francois Romieu wrote: > Zhang Changzhong <zhangchangzhong@huawei.com> : >> The nixge_start_xmit() returns NETDEV_TX_OK but does not free skb on two >> error handling cases, which can lead to memory leak. >> >> To fix this, return NETDEV_TX_BUSY in case of nixge_check_tx_bd_space() >> fails and add dev_kfree_skb_any() in case of dma_map_single() fails. > > This patch merge two unrelated changes. Please split. > >> Fixes: 492caffa8a1a ("net: ethernet: nixge: Add support for National Instruments XGE netdev") >> Signed-off-by: Zhang Changzhong <zhangchangzhong@huawei.com> >> --- >> drivers/net/ethernet/ni/nixge.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c >> index 19d043b593cc..b9091f9bbc77 100644 >> --- a/drivers/net/ethernet/ni/nixge.c >> +++ b/drivers/net/ethernet/ni/nixge.c >> @@ -521,13 +521,15 @@ static netdev_tx_t nixge_start_xmit(struct sk_buff *skb, >> if (nixge_check_tx_bd_space(priv, num_frag)) { >> if (!netif_queue_stopped(ndev)) >> netif_stop_queue(ndev); >> - return NETDEV_TX_OK; >> + return NETDEV_TX_BUSY; >> } > > The driver should probably check the available room before returning > from hard_start_xmit and turn the check above unlikely(). > > Btw there is no lock and the Tx completion is irq driven: the driver > is racy. :o( > Hi Francois, Thanks for you review. I'll make v2 according to your suggestion. Changzhong
On 15 Nov 21:20, Zhang Changzhong wrote: >On 2022/11/14 18:40, Francois Romieu wrote: >> Zhang Changzhong <zhangchangzhong@huawei.com> : >>> The nixge_start_xmit() returns NETDEV_TX_OK but does not free skb on two >>> error handling cases, which can lead to memory leak. >>> >>> To fix this, return NETDEV_TX_BUSY in case of nixge_check_tx_bd_space() >>> fails and add dev_kfree_skb_any() in case of dma_map_single() fails. >> >> This patch merge two unrelated changes. Please split. >> >>> Fixes: 492caffa8a1a ("net: ethernet: nixge: Add support for National Instruments XGE netdev") >>> Signed-off-by: Zhang Changzhong <zhangchangzhong@huawei.com> >>> --- >>> drivers/net/ethernet/ni/nixge.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c >>> index 19d043b593cc..b9091f9bbc77 100644 >>> --- a/drivers/net/ethernet/ni/nixge.c >>> +++ b/drivers/net/ethernet/ni/nixge.c >>> @@ -521,13 +521,15 @@ static netdev_tx_t nixge_start_xmit(struct sk_buff *skb, >>> if (nixge_check_tx_bd_space(priv, num_frag)) { >>> if (!netif_queue_stopped(ndev)) >>> netif_stop_queue(ndev); >>> - return NETDEV_TX_OK; >>> + return NETDEV_TX_BUSY; >>> } >> >> The driver should probably check the available room before returning >> from hard_start_xmit and turn the check above unlikely(). >> >> Btw there is no lock and the Tx completion is irq driven: the driver >> is racy. :o( >> > >Hi Francois, > >Thanks for you review. I'll make v2 according to your suggestion. > you will probably need to check out: Transmit path guidelines: https://www.kernel.org/doc/Documentation/networking/driver.rst
On 2022/11/17 6:36, Saeed Mahameed wrote: > On 15 Nov 21:20, Zhang Changzhong wrote: >> On 2022/11/14 18:40, Francois Romieu wrote: >>> Zhang Changzhong <zhangchangzhong@huawei.com> : >>>> The nixge_start_xmit() returns NETDEV_TX_OK but does not free skb on two >>>> error handling cases, which can lead to memory leak. >>>> >>>> To fix this, return NETDEV_TX_BUSY in case of nixge_check_tx_bd_space() >>>> fails and add dev_kfree_skb_any() in case of dma_map_single() fails. >>> >>> This patch merge two unrelated changes. Please split. >>> >>>> Fixes: 492caffa8a1a ("net: ethernet: nixge: Add support for National Instruments XGE netdev") >>>> Signed-off-by: Zhang Changzhong <zhangchangzhong@huawei.com> >>>> --- >>>> drivers/net/ethernet/ni/nixge.c | 6 ++++-- >>>> 1 file changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c >>>> index 19d043b593cc..b9091f9bbc77 100644 >>>> --- a/drivers/net/ethernet/ni/nixge.c >>>> +++ b/drivers/net/ethernet/ni/nixge.c >>>> @@ -521,13 +521,15 @@ static netdev_tx_t nixge_start_xmit(struct sk_buff *skb, >>>> if (nixge_check_tx_bd_space(priv, num_frag)) { >>>> if (!netif_queue_stopped(ndev)) >>>> netif_stop_queue(ndev); >>>> - return NETDEV_TX_OK; >>>> + return NETDEV_TX_BUSY; >>>> } >>> >>> The driver should probably check the available room before returning >>> from hard_start_xmit and turn the check above unlikely(). >>> >>> Btw there is no lock and the Tx completion is irq driven: the driver >>> is racy. :o( >>> >> >> Hi Francois, >> >> Thanks for you review. I'll make v2 according to your suggestion. >> > > you will probably need to check out: Transmit path guidelines: > https://www.kernel.org/doc/Documentation/networking/driver.rst > Thank! This document is very helpful. > .
diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c index 19d043b593cc..b9091f9bbc77 100644 --- a/drivers/net/ethernet/ni/nixge.c +++ b/drivers/net/ethernet/ni/nixge.c @@ -521,13 +521,15 @@ static netdev_tx_t nixge_start_xmit(struct sk_buff *skb, if (nixge_check_tx_bd_space(priv, num_frag)) { if (!netif_queue_stopped(ndev)) netif_stop_queue(ndev); - return NETDEV_TX_OK; + return NETDEV_TX_BUSY; } cur_phys = dma_map_single(ndev->dev.parent, skb->data, skb_headlen(skb), DMA_TO_DEVICE); - if (dma_mapping_error(ndev->dev.parent, cur_phys)) + if (dma_mapping_error(ndev->dev.parent, cur_phys)) { + dev_kfree_skb_any(skb); goto drop; + } nixge_hw_dma_bd_set_phys(cur_p, cur_phys); cur_p->cntrl = skb_headlen(skb) | XAXIDMA_BD_CTRL_TXSOF_MASK;