Message ID | 7a382b9503d10d235238ca55938bc933d92a1de7.1667389213.git.chentao.kernel@linux.alibaba.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 l7csp3581360wru; Wed, 2 Nov 2022 05:14:56 -0700 (PDT) X-Google-Smtp-Source: AMsMyM66YTXdwBvdbyowQIfkQsn2x/eh+Din1WQt/Qi+QJrS5J2Ot9tBAnphWNQZ9OP/g4Mrigd4 X-Received: by 2002:a17:906:7313:b0:78e:c2a:a3fa with SMTP id di19-20020a170906731300b0078e0c2aa3famr22498032ejc.581.1667391296353; Wed, 02 Nov 2022 05:14:56 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667391296; cv=none; d=google.com; s=arc-20160816; b=FbuZoY/IZaUJKbOkBowKhyskrQd6vkDNreksTVnuE4eIaKo3rWGnlbFAsZNYJtLxHw d97QyvtinTRYz4bnFza2niFl5mIgV13V05bz1zZ+4KNeBMWzw06xpsq67FA+4H/Fir0g HFpa1VVAt5zR0BbiGhrM1tq3X+Q2xDvyU1ycnJCH309Iafdk6lYV6nEXZsYxfJhLXX6o e/pcsQu6KRqF5ePCDqAgTi8LRyAvmyp0gmpcFiklHRGW6/8X5bI6uCgEl7EIqlrwLAhR JPd141hLKVJkxRJW9k+C/fvI3BjC+Le+4qWPFHhS7qGRSopBE5QRUodJy7JsGR8X01XB 037w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:message-id:date:subject:cc:to:from; bh=0ly3+kR1TxlQepikVDpnjbbBO9FZ/y+wEtPmrVFp5Gk=; b=T3Obpij+0mBb3uwvd34tjz86w7Qz8vWwmkUj7o2kG02qFudGx+zzt3D4DdCJI09HjY rMydHVj0JlZXWbRXrLsRI90WJtwHeKAbGhjKo9teh+LkgiN2JxgH9ce3KN/fTbC+Z0Pz 4jeq2T4DTwzjIUQPZd0ezwaEwksdNFYb+fgT9goGWqFonB4fvUQwJSul80Vucj6rOGYH AUugftZRPUrqHu6A4tM59MfI9+5dlHXROIT0ij5R8ch8nvMJe/WKTW2pyJwzlLRk+qbt OaRnIuEY9o0HUs6d0imoa8I2SAC/BSGC69V6Im3I/eFFqjFXOeNQSMee2ToUw1IpWLp9 EJqw== 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=NONE sp=NONE dis=NONE) header.from=alibaba.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id du10-20020a17090772ca00b0073d751c96adsi16248198ejc.1000.2022.11.02.05.14.32; Wed, 02 Nov 2022 05:14:56 -0700 (PDT) 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=NONE sp=NONE dis=NONE) header.from=alibaba.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230515AbiKBMIi (ORCPT <rfc822;billy.jones8454@gmail.com> + 99 others); Wed, 2 Nov 2022 08:08:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35842 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230366AbiKBMIg (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 2 Nov 2022 08:08:36 -0400 Received: from out30-43.freemail.mail.aliyun.com (out30-43.freemail.mail.aliyun.com [115.124.30.43]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D43B722516; Wed, 2 Nov 2022 05:08:34 -0700 (PDT) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R831e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018045168;MF=chentao.kernel@linux.alibaba.com;NM=1;PH=DS;RN=12;SR=0;TI=SMTPD_---0VTo8Xcs_1667390902; Received: from VM20210331-5.tbsite.net(mailfrom:chentao.kernel@linux.alibaba.com fp:SMTPD_---0VTo8Xcs_1667390902) by smtp.aliyun-inc.com; Wed, 02 Nov 2022 20:08:30 +0800 From: Tao Chen <chentao.kernel@linux.alibaba.com> To: "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Johannes Berg <johannes@sipsolutions.net>, Oliver Hartkopp <socketcan@hartkopp.net>, Petr Machata <petrm@nvidia.com>, Kees Cook <keescook@chromium.org>, Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Tao Chen <chentao.kernel@linux.alibaba.com> Subject: [PATCH net-next] netlink: Fix potential skb memleak in netlink_ack Date: Wed, 2 Nov 2022 20:08:20 +0800 Message-Id: <7a382b9503d10d235238ca55938bc933d92a1de7.1667389213.git.chentao.kernel@linux.alibaba.com> X-Mailer: git-send-email 2.2.1 X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2, SPF_HELO_NONE,SPF_PASS,UNPARSEABLE_RELAY,USER_IN_DEF_SPF_WL 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?1748386495897032265?= X-GMAIL-MSGID: =?utf-8?q?1748386495897032265?= |
Series |
[net-next] netlink: Fix potential skb memleak in netlink_ack
|
|
Commit Message
Tao Chen
Nov. 2, 2022, 12:08 p.m. UTC
We should clean the skb resource if nlmsg_put/append failed
, so fix it.
Fiexs: commit 738136a0e375 ("netlink: split up copies in the
ack construction")
Signed-off-by: Tao Chen <chentao.kernel@linux.alibaba.com>
---
net/netlink/af_netlink.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Comments
On November 2, 2022 5:08:20 AM PDT, Tao Chen <chentao.kernel@linux.alibaba.com> wrote: >We should clean the skb resource if nlmsg_put/append failed >, so fix it. > >Fiexs: commit 738136a0e375 ("netlink: split up copies in the >ack construction") >Signed-off-by: Tao Chen <chentao.kernel@linux.alibaba.com> >--- > net/netlink/af_netlink.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c >index c6b8207e..9d73dae 100644 >--- a/net/netlink/af_netlink.c >+++ b/net/netlink/af_netlink.c >@@ -2500,7 +2500,7 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, > > skb = nlmsg_new(payload + tlvlen, GFP_KERNEL); > if (!skb) >- goto err_bad_put; >+ goto err_skb; > > rep = nlmsg_put(skb, NETLINK_CB(in_skb).portid, nlh->nlmsg_seq, > NLMSG_ERROR, sizeof(*errmsg), flags); >@@ -2528,6 +2528,8 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, > return; > > err_bad_put: >+ kfree_skb(skb); >+err_skb: > NETLINK_CB(in_skb).sk->sk_err = ENOBUFS; > sk_error_report(NETLINK_CB(in_skb).sk); > } It didn't do this before... Is this right?
On Wed, 2 Nov 2022 20:08:20 +0800 Tao Chen wrote: > We should clean the skb resource if nlmsg_put/append failed > , so fix it. The comma should be at the end of the previous line. But really the entire ", so fix it." is redundant. > Fiexs: commit 738136a0e375 ("netlink: split up copies in the > ack construction") Please look around to see how to correctly format a Fixes tag (including not line wrapping it). How did you find this bug? An automated tool? Syzbot? One more note below on the code itself. > Signed-off-by: Tao Chen <chentao.kernel@linux.alibaba.com> > --- > net/netlink/af_netlink.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c > index c6b8207e..9d73dae 100644 > --- a/net/netlink/af_netlink.c > +++ b/net/netlink/af_netlink.c > @@ -2500,7 +2500,7 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, > > skb = nlmsg_new(payload + tlvlen, GFP_KERNEL); > if (!skb) > - goto err_bad_put; > + goto err_skb; > > rep = nlmsg_put(skb, NETLINK_CB(in_skb).portid, nlh->nlmsg_seq, > NLMSG_ERROR, sizeof(*errmsg), flags); > @@ -2528,6 +2528,8 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, > return; > > err_bad_put: > + kfree_skb(skb); Please use nlmsg_free() since we allocated with nlmsg_new(). > +err_skb: > NETLINK_CB(in_skb).sk->sk_err = ENOBUFS; > sk_error_report(NETLINK_CB(in_skb).sk); > }
在 2022/11/3 上午5:39, Jakub Kicinski 写道: > On Wed, 2 Nov 2022 20:08:20 +0800 Tao Chen wrote: >> We should clean the skb resource if nlmsg_put/append failed >> , so fix it. > > The comma should be at the end of the previous line. > But really the entire ", so fix it." is redundant. > Thank you, i will pay attention next time >> Fiexs: commit 738136a0e375 ("netlink: split up copies in the >> ack construction") > > Please look around to see how to correctly format a Fixes tag > (including not line wrapping it). > > How did you find this bug? An automated tool? Syzbot? > > One more note below on the code itself. > This was found by the coverity tool, i will add it. >> Signed-off-by: Tao Chen <chentao.kernel@linux.alibaba.com> >> --- >> net/netlink/af_netlink.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c >> index c6b8207e..9d73dae 100644 >> --- a/net/netlink/af_netlink.c >> +++ b/net/netlink/af_netlink.c >> @@ -2500,7 +2500,7 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, >> >> skb = nlmsg_new(payload + tlvlen, GFP_KERNEL); >> if (!skb) >> - goto err_bad_put; >> + goto err_skb; >> >> rep = nlmsg_put(skb, NETLINK_CB(in_skb).portid, nlh->nlmsg_seq, >> NLMSG_ERROR, sizeof(*errmsg), flags); >> @@ -2528,6 +2528,8 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, >> return; >> >> err_bad_put: >> + kfree_skb(skb); > > Please use nlmsg_free() since we allocated with nlmsg_new(). > Ok, i will send it in v2. >> +err_skb: >> NETLINK_CB(in_skb).sk->sk_err = ENOBUFS; >> sk_error_report(NETLINK_CB(in_skb).sk); >> }
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index c6b8207e..9d73dae 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -2500,7 +2500,7 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, skb = nlmsg_new(payload + tlvlen, GFP_KERNEL); if (!skb) - goto err_bad_put; + goto err_skb; rep = nlmsg_put(skb, NETLINK_CB(in_skb).portid, nlh->nlmsg_seq, NLMSG_ERROR, sizeof(*errmsg), flags); @@ -2528,6 +2528,8 @@ void netlink_ack(struct sk_buff *in_skb, struct nlmsghdr *nlh, int err, return; err_bad_put: + kfree_skb(skb); +err_skb: NETLINK_CB(in_skb).sk->sk_err = ENOBUFS; sk_error_report(NETLINK_CB(in_skb).sk); }