From patchwork Mon Feb 7 17:13:18 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antoine Tenart X-Patchwork-Id: 12737707 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DD0B2C4332F for ; Mon, 7 Feb 2022 17:21:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1383519AbiBGRSN (ORCPT ); Mon, 7 Feb 2022 12:18:13 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38510 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1378622AbiBGRN2 (ORCPT ); Mon, 7 Feb 2022 12:13:28 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AC6A3C0401DA for ; Mon, 7 Feb 2022 09:13:26 -0800 (PST) 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 dfw.source.kernel.org (Postfix) with ESMTPS id 445F560FC4 for ; Mon, 7 Feb 2022 17:13:26 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 06F45C004E1; Mon, 7 Feb 2022 17:13:24 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1644254005; bh=VDDO+foV3TK2cHCK8eXPS1Yzlk3UQhjEB/c6SY7m18k=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=X+txThXsbZOE3ik4OMPVt9ckgeYBcNvfeVT8jNX70k2mls/UeosS2IJDTfcIYnE+f cM0QexSd1uTX7ZjKwwxT8lE8/fIvUSX0C7dJj22H639kcLUTAKKeK23gmFZhNRbj7H 3ZC/3gtE7PxCPX4d8BphxJv6ylmqZGhOfuiZmjZYgf9S6H37QaG2XwWKVzbH6pgIjs sm8zrYOtpKfRASST5ETV/5SI0roH6kvt4vwQHE6zSI4/HbpUO0H/pMxhhOAPspUQlo 2GsQQCT0HeZ4A9Epd49iQf/3rQr2qC9OrYhFsd6rV2/Z9pg1dky+usppBOaNYlPqTG EjJ8IxXLlTJLg== From: Antoine Tenart To: davem@davemloft.net, kuba@kernel.org Cc: Antoine Tenart , netdev@vger.kernel.org, vladbu@nvidia.com, pabeni@redhat.com, pshelar@ovn.org, daniel@iogearbox.net Subject: [PATCH net v2 1/2] net: do not keep the dst cache when uncloning an skb dst and its metadata Date: Mon, 7 Feb 2022 18:13:18 +0100 Message-Id: <20220207171319.157775-2-atenart@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220207171319.157775-1-atenart@kernel.org> References: <20220207171319.157775-1-atenart@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org When uncloning an skb dst and its associated metadata a new dst+metadata is allocated and the tunnel information from the old metadata is copied over there. The issue is the tunnel metadata has references to cached dst, which are copied along the way. When a dst+metadata refcount drops to 0 the metadata is freed including the cached dst entries. As they are also referenced in the initial dst+metadata, this ends up in UaFs. In practice the above did not happen because of another issue, the dst+metadata was never freed because its refcount never dropped to 0 (this will be fixed in a subsequent patch). Fix this by initializing the dst cache after copying the tunnel information from the old metadata to also unshare the dst cache. Fixes: d71785ffc7e7 ("net: add dst_cache to ovs vxlan lwtunnel") Cc: Paolo Abeni Reported-by: Vlad Buslov Tested-by: Vlad Buslov Signed-off-by: Antoine Tenart Acked-by: Paolo Abeni --- include/net/dst_metadata.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h index 14efa0ded75d..b997e0c1e362 100644 --- a/include/net/dst_metadata.h +++ b/include/net/dst_metadata.h @@ -123,6 +123,19 @@ static inline struct metadata_dst *tun_dst_unclone(struct sk_buff *skb) memcpy(&new_md->u.tun_info, &md_dst->u.tun_info, sizeof(struct ip_tunnel_info) + md_size); +#ifdef CONFIG_DST_CACHE + /* Unclone the dst cache if there is one */ + if (new_md->u.tun_info.dst_cache.cache) { + int ret; + + ret = dst_cache_init(&new_md->u.tun_info.dst_cache, GFP_ATOMIC); + if (ret) { + metadata_dst_free(new_md); + return ERR_PTR(ret); + } + } +#endif + skb_dst_drop(skb); dst_hold(&new_md->dst); skb_dst_set(skb, &new_md->dst); From patchwork Mon Feb 7 17:13:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Antoine Tenart X-Patchwork-Id: 12737706 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A58BC433EF for ; Mon, 7 Feb 2022 17:21:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243235AbiBGRSK (ORCPT ); Mon, 7 Feb 2022 12:18:10 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38548 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S239082AbiBGRNd (ORCPT ); Mon, 7 Feb 2022 12:13:33 -0500 Received: from sin.source.kernel.org (sin.source.kernel.org [IPv6:2604:1380:40e1:4800::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3AFAEC0401D5 for ; Mon, 7 Feb 2022 09:13:32 -0800 (PST) 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 sin.source.kernel.org (Postfix) with ESMTPS id A895CCE1189 for ; Mon, 7 Feb 2022 17:13:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4B146C004E1; Mon, 7 Feb 2022 17:13:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1644254009; bh=6ZGM40P2SsVBWWyd3LNjeReC73h//VpV7MUdg9dXnY4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aF1QCbakuB/WKqPnxejm/LeRtCWl64V3IJOkXltyU+XyLrSqCwEzSAHupaxnHENez R2nmDoYKbC1Ou/dl4YMwrwlugdf5GzFiZYMwdTSeBdmrtAKOyWT7bsoA+mjaMfeame qorGLA6WSUK0Qo8ZSspLIngycHb/kf26eSDLy71/ABSvSGVSAKonVhHtVSIlF2oZjr ar0QKgw/gXDqFslFERXVq97zs4id5uoJ/u3/9LU4VFdREh9jtN9j6J6EBUyd/F5gHf KVKTJcx/CwrOn7mPBXWhR9ZK0hMhQdIO5QHGyUVofNjyqpXgcWDWr0oJnvpdrzLqwD LOf+lhoXxnI0Q== From: Antoine Tenart To: davem@davemloft.net, kuba@kernel.org Cc: Antoine Tenart , netdev@vger.kernel.org, vladbu@nvidia.com, pabeni@redhat.com, pshelar@ovn.org, daniel@iogearbox.net Subject: [PATCH net v2 2/2] net: fix a memleak when uncloning an skb dst and its metadata Date: Mon, 7 Feb 2022 18:13:19 +0100 Message-Id: <20220207171319.157775-3-atenart@kernel.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20220207171319.157775-1-atenart@kernel.org> References: <20220207171319.157775-1-atenart@kernel.org> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org When uncloning an skb dst and its associated metadata, a new dst+metadata is allocated and later replaces the old one in the skb. This is helpful to have a non-shared dst+metadata attached to a specific skb. The issue is the uncloned dst+metadata is initialized with a refcount of 1, which is increased to 2 before attaching it to the skb. When tun_dst_unclone returns, the dst+metadata is only referenced from a single place (the skb) while its refcount is 2. Its refcount will never drop to 0 (when the skb is consumed), leading to a memory leak. Fix this by removing the call to dst_hold in tun_dst_unclone, as the dst+metadata refcount is already 1. Fixes: fc4099f17240 ("openvswitch: Fix egress tunnel info.") Cc: Pravin B Shelar Reported-by: Vlad Buslov Tested-by: Vlad Buslov Signed-off-by: Antoine Tenart --- include/net/dst_metadata.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h index b997e0c1e362..adab27ba1ecb 100644 --- a/include/net/dst_metadata.h +++ b/include/net/dst_metadata.h @@ -137,7 +137,6 @@ static inline struct metadata_dst *tun_dst_unclone(struct sk_buff *skb) #endif skb_dst_drop(skb); - dst_hold(&new_md->dst); skb_dst_set(skb, &new_md->dst); return new_md; }