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=-6.8 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_GIT 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 3BCD1C7618B for ; Fri, 26 Jul 2019 15:27:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 163EB22CBF for ; Fri, 26 Jul 2019 15:27:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1564154840; bh=ZJ/6kr/qrv+K2zRlwxUORuwT9b27rv8CB0+Q+ZQUMOk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=xF+tQtfngFcEvju5M5nhSOkIc5IQP8ldWc0S0Qsdwauaf9Yjkl+EtCcNl24S2vIi6 yq5bZM8C9XAkc/9g5SwuyNigssvfEXZX/VBnmGMdIZ1fUZ2lRVSyqMSK5wY+DwJ0Ww ZqVK1koKBmNA/m8usWg2hjB52/sLo9nml5aKOT0U= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2388268AbfGZP1S (ORCPT ); Fri, 26 Jul 2019 11:27:18 -0400 Received: from mail.kernel.org ([198.145.29.99]:41508 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388263AbfGZP1R (ORCPT ); Fri, 26 Jul 2019 11:27:17 -0400 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 7FD8D22CC3; Fri, 26 Jul 2019 15:27:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1564154837; bh=ZJ/6kr/qrv+K2zRlwxUORuwT9b27rv8CB0+Q+ZQUMOk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=G7lH53VEw3ARBZZC0YhfBDEzfiY+Ib/vUtb8MYV+WL+lWmrdQCeKLKLyJC4i3kKB2 F7Th8vE36qFgRUWuFuBxYqkpH2agHNHcKH8ZDccjF1/5qAtJdpJrR71VrFem8/XkrO P9Y7PetxX6CvaHbJQYT9Hz08eW0dq0If20eCnQ50= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Florian Westphal , "David S. Miller" Subject: [PATCH 5.2 10/66] net: make skb_dst_force return true when dst is refcounted Date: Fri, 26 Jul 2019 17:24:09 +0200 Message-Id: <20190726152302.988657734@linuxfoundation.org> X-Mailer: git-send-email 2.22.0 In-Reply-To: <20190726152301.936055394@linuxfoundation.org> References: <20190726152301.936055394@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Florian Westphal [ Upstream commit b60a77386b1d4868f72f6353d35dabe5fbe981f2 ] netfilter did not expect that skb_dst_force() can cause skb to lose its dst entry. I got a bug report with a skb->dst NULL dereference in netfilter output path. The backtrace contains nf_reinject(), so the dst might have been cleared when skb got queued to userspace. Other users were fixed via if (skb_dst(skb)) { skb_dst_force(skb); if (!skb_dst(skb)) goto handle_err; } But I think its preferable to make the 'dst might be cleared' part of the function explicit. In netfilter case, skb with a null dst is expected when queueing in prerouting hook, so drop skb for the other hooks. v2: v1 of this patch returned true in case skb had no dst entry. Eric said: Say if we have two skb_dst_force() calls for some reason on the same skb, only the first one will return false. This now returns false even when skb had no dst, as per Erics suggestion, so callers might need to check skb_dst() first before skb_dst_force(). Signed-off-by: Florian Westphal Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- include/net/dst.h | 5 ++++- net/netfilter/nf_queue.c | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) --- a/include/net/dst.h +++ b/include/net/dst.h @@ -302,8 +302,9 @@ static inline bool dst_hold_safe(struct * @skb: buffer * * If dst is not yet refcounted and not destroyed, grab a ref on it. + * Returns true if dst is refcounted. */ -static inline void skb_dst_force(struct sk_buff *skb) +static inline bool skb_dst_force(struct sk_buff *skb) { if (skb_dst_is_noref(skb)) { struct dst_entry *dst = skb_dst(skb); @@ -314,6 +315,8 @@ static inline void skb_dst_force(struct skb->_skb_refdst = (unsigned long)dst; } + + return skb->_skb_refdst != 0UL; } --- a/net/netfilter/nf_queue.c +++ b/net/netfilter/nf_queue.c @@ -190,6 +190,11 @@ static int __nf_queue(struct sk_buff *sk goto err; } + if (!skb_dst_force(skb) && state->hook != NF_INET_PRE_ROUTING) { + status = -ENETDOWN; + goto err; + } + *entry = (struct nf_queue_entry) { .skb = skb, .state = *state, @@ -198,7 +203,6 @@ static int __nf_queue(struct sk_buff *sk }; nf_queue_entry_get_refs(entry); - skb_dst_force(skb); switch (entry->state.pf) { case AF_INET: