Received: by 2002:a05:6358:111d:b0:dc:6189:e246 with SMTP id f29csp591956rwi; Wed, 2 Nov 2022 15:54:21 -0700 (PDT) X-Google-Smtp-Source: AMsMyM4DqMODzAARmvuVVZvpAvwoNGR/bnu+vZXiJGrsYHpaAoWlk3et/MnkrSpPvGA+yr4Ohu5N X-Received: by 2002:a17:90a:72cb:b0:213:fbf0:f5f1 with SMTP id l11-20020a17090a72cb00b00213fbf0f5f1mr14829623pjk.107.1667429661697; Wed, 02 Nov 2022 15:54:21 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1667429661; cv=none; d=google.com; s=arc-20160816; b=rF/HPBug+t47jV8zyQ+qJHG3uhdO8g18rPRmWJAZ6V5x5SYjYkoYWSgg4cF0+rPG6D N9RT8ptHExYTgynSdHb0BpYHXuGV+p5p4mfSm919WBkEyAFhl4yCFOiXAm75krd69ELi pwv/WbXWRjNYDW22gi6sS9k4w40M19mV8CzbsMIjyBe85HrVQvg3giwkyoA1fbYirEPg PnccLkXyLAebaod9AJZRmBsBjMI1iS30HlFHffjrr3l+0Ki0mpfdEig1vOsGISd1K8BH NZdnZAUjRvlDc5rtlnMZwIHU1fTa6cFJqn3+Ey/0pBILYONnddl9X0eg6FIq2tqD88im 0wmQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :dkim-signature; bh=RPTFgGIFXRwOOZUCBQp0sa+bkyKJyv+KFV30h5GJsp0=; b=aDi5nPL2rK+BDK1qlxAN56oN7PC3DkDgaTFN1myoO8nQvBole+ol5+KkgDq2JqijFt o+czCngeg0cwvJU19LDv5Jgo4OaA2hTkB2qdHNp6nFucA+5tEypIXswU0nCPT+r8RY3O uyQATewqOb1bc2NLvbyve0Z4hOVO8qpnlynS9T2fy7r4gk/zEc6s94umIvEqcsfdtfWv Q68IpFnEBdZEMryzFcLJdDE0R0dK+iXH9rvSe3hO+6sQtGYOv4BlN0FDxgfaDafrV1La yjFqQO+6Z0kgwLE2McjQBkYvlbArgyrnHrjSXcTrBLojIOAUHAoKUIfG9p7YUlXHG69H AmIQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=t9hQ5YIJ; 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=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id p5-20020a170902ebc500b001869b988da6si17012492plg.402.2022.11.02.15.54.09; Wed, 02 Nov 2022 15:54:21 -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; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=t9hQ5YIJ; 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=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231230AbiKBVkT (ORCPT + 97 others); Wed, 2 Nov 2022 17:40:19 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42476 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229590AbiKBVkR (ORCPT ); Wed, 2 Nov 2022 17:40:17 -0400 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1338463D0; Wed, 2 Nov 2022 14:40:17 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 32179B82525; Wed, 2 Nov 2022 21:39:56 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6C7FDC433D6; Wed, 2 Nov 2022 21:39:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1667425194; bh=PmiXzhOEefQJy7eB8akCP/s7HA4Sro+JpQ84GNIMYP4=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=t9hQ5YIJDF4ecTpDvEt0KvC+XB1Hyk/ciVuLGAiJ0DaGLa8dUVqV5T1UBNa5ULgA/ gszs4XrhL1e8t0ZAtqmofo7FJwegfhhCsKkAIjofulxdu+ANZK+BX8bTJ5SoWSvFI1 nuP2U8E6MR9JQjX25/i1UG+aUCBOwuj1lej9ha75EdjISemDyxhsj2JC8pmSdTYM/Z 5k6hyHF6S5jQPFJQe2qnAJLBoeBVnwd3hb2B6FMQMvFIKagfC3uLS1hdxPe3EeSQN0 CqOSGnp7jth9S9zKWMRC83qV9Q621guhB0WYDXE5jww2gRTA9NSVsYvg1DEjU9bgul OMiyvFpD5bPIw== Date: Wed, 2 Nov 2022 14:39:53 -0700 From: Jakub Kicinski To: Tao Chen Cc: "David S. Miller" , Eric Dumazet , Paolo Abeni , Johannes Berg , Oliver Hartkopp , Petr Machata , Kees Cook , Harshit Mogalapalli , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net-next] netlink: Fix potential skb memleak in netlink_ack Message-ID: <20221102143953.001f1247@kernel.org> In-Reply-To: <7a382b9503d10d235238ca55938bc933d92a1de7.1667389213.git.chentao.kernel@linux.alibaba.com> References: <7a382b9503d10d235238ca55938bc933d92a1de7.1667389213.git.chentao.kernel@linux.alibaba.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-8.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, 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: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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); > }