From patchwork Fri Jan 24 09:32:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Greg KH X-Patchwork-Id: 233123 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=-9.8 required=3.0 tests=DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH, MAILING_LIST_MULTI, SIGNED_OFF_BY, SPF_HELO_NONE, SPF_PASS, USER_AGENT_GIT autolearn=unavailable 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 13EE4C35242 for ; Fri, 24 Jan 2020 10:06:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DB4F4206F0 for ; Fri, 24 Jan 2020 10:06:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1579860360; bh=swPqSJ+snYtwmzKXodk19Jh68Zxu28ks3LjycSs1/Cg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:List-ID:From; b=JWJ9NsS1vEroUwgcS1gIJUwZN441nxlDNq9HTiE5JnKrteyykymYNLOYmuyg3mWqr agO+dF7omSUmMhHT6Qd/hKBJvH3aSgs2cbfCkfdCt6sXCw+f8gG9Ng4VhVXmMizUvb EQ+t2nTixO3bvjV6xpZQC8J9LT+190ffULTL35dU= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731753AbgAXKF5 (ORCPT ); Fri, 24 Jan 2020 05:05:57 -0500 Received: from mail.kernel.org ([198.145.29.99]:42440 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2389249AbgAXKFz (ORCPT ); Fri, 24 Jan 2020 05:05:55 -0500 Received: from localhost (unknown [145.15.244.15]) (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 67018206F0; Fri, 24 Jan 2020 10:05:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1579860354; bh=swPqSJ+snYtwmzKXodk19Jh68Zxu28ks3LjycSs1/Cg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=c6CryPtfnPH9oW+yo3Zl/78zIOVCHoHeb4aCpychUT1Z4ydmbdowxTpm/3Yb8sCLl USvDPjhwFRlCBhjljl4F7C/Wxbzwi7gvPmMXAq8/Dg9w4wpOBCNZUp8P0yXtx0/fdl uyDdr9gPNb8+Qbt2yzCAoyiWRT89kkQhs7a+qHf0= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Eric Biggers , Jakub Kicinski , Sasha Levin Subject: [PATCH 4.14 319/343] llc: fix sk_buff refcounting in llc_conn_state_process() Date: Fri, 24 Jan 2020 10:32:17 +0100 Message-Id: <20200124093001.796388948@linuxfoundation.org> X-Mailer: git-send-email 2.25.0 In-Reply-To: <20200124092919.490687572@linuxfoundation.org> References: <20200124092919.490687572@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Eric Biggers [ Upstream commit 36453c852816f19947ca482a595dffdd2efa4965 ] If llc_conn_state_process() sees that llc_conn_service() put the skb on a list, it will drop one fewer references to it. This is wrong because the current behavior is that llc_conn_service() never consumes a reference to the skb. The code also makes the number of skb references being dropped conditional on which of ind_prim and cfm_prim are nonzero, yet neither of these affects how many references are *acquired*. So there is extra code that tries to fix this up by sometimes taking another reference. Remove the unnecessary/broken refcounting logic and instead just add an skb_get() before the only two places where an extra reference is actually consumed. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Eric Biggers Signed-off-by: Jakub Kicinski Signed-off-by: Sasha Levin --- net/llc/llc_conn.c | 33 ++++++--------------------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c index 7340f23e16de3..7fbc682aff043 100644 --- a/net/llc/llc_conn.c +++ b/net/llc/llc_conn.c @@ -64,12 +64,6 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) struct llc_sock *llc = llc_sk(skb->sk); struct llc_conn_state_ev *ev = llc_conn_ev(skb); - /* - * We have to hold the skb, because llc_conn_service will kfree it in - * the sending path and we need to look at the skb->cb, where we encode - * llc_conn_state_ev. - */ - skb_get(skb); ev->ind_prim = ev->cfm_prim = 0; /* * Send event to state machine @@ -77,21 +71,12 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) rc = llc_conn_service(skb->sk, skb); if (unlikely(rc != 0)) { printk(KERN_ERR "%s: llc_conn_service failed\n", __func__); - goto out_kfree_skb; - } - - if (unlikely(!ev->ind_prim && !ev->cfm_prim)) { - /* indicate or confirm not required */ - if (!skb->next) - goto out_kfree_skb; goto out_skb_put; } - if (unlikely(ev->ind_prim && ev->cfm_prim)) /* Paranoia */ - skb_get(skb); - switch (ev->ind_prim) { case LLC_DATA_PRIM: + skb_get(skb); llc_save_primitive(sk, skb, LLC_DATA_PRIM); if (unlikely(sock_queue_rcv_skb(sk, skb))) { /* @@ -108,6 +93,7 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) * skb->sk pointing to the newly created struct sock in * llc_conn_handler. -acme */ + skb_get(skb); skb_queue_tail(&sk->sk_receive_queue, skb); sk->sk_state_change(sk); break; @@ -123,7 +109,6 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) sk->sk_state_change(sk); } } - kfree_skb(skb); sock_put(sk); break; case LLC_RESET_PRIM: @@ -132,14 +117,11 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) * RESET is not being notified to upper layers for now */ printk(KERN_INFO "%s: received a reset ind!\n", __func__); - kfree_skb(skb); break; default: - if (ev->ind_prim) { + if (ev->ind_prim) printk(KERN_INFO "%s: received unknown %d prim!\n", __func__, ev->ind_prim); - kfree_skb(skb); - } /* No indication */ break; } @@ -181,15 +163,12 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb) printk(KERN_INFO "%s: received a reset conf!\n", __func__); break; default: - if (ev->cfm_prim) { + if (ev->cfm_prim) printk(KERN_INFO "%s: received unknown %d prim!\n", __func__, ev->cfm_prim); - break; - } - goto out_skb_put; /* No confirmation */ + /* No confirmation */ + break; } -out_kfree_skb: - kfree_skb(skb); out_skb_put: kfree_skb(skb); return rc;