All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org,
	syzbot+6b825a6494a04cc0e3f7@syzkaller.appspotmail.com,
	Eric Biggers <ebiggers@google.com>,
	Jakub Kicinski <jakub.kicinski@netronome.com>
Subject: [PATCH 4.4 40/46] llc: fix sk_buff leak in llc_conn_service()
Date: Mon,  4 Nov 2019 22:45:11 +0100	[thread overview]
Message-ID: <20191104211913.156429047@linuxfoundation.org> (raw)
In-Reply-To: <20191104211830.912265604@linuxfoundation.org>

From: Eric Biggers <ebiggers@google.com>

commit b74555de21acd791f12c4a1aeaf653dd7ac21133 upstream.

syzbot reported:

    BUG: memory leak
    unreferenced object 0xffff88811eb3de00 (size 224):
       comm "syz-executor559", pid 7315, jiffies 4294943019 (age 10.300s)
       hex dump (first 32 bytes):
         00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
         00 a0 38 24 81 88 ff ff 00 c0 f2 15 81 88 ff ff  ..8$............
       backtrace:
         [<000000008d1c66a1>] kmemleak_alloc_recursive  include/linux/kmemleak.h:55 [inline]
         [<000000008d1c66a1>] slab_post_alloc_hook mm/slab.h:439 [inline]
         [<000000008d1c66a1>] slab_alloc_node mm/slab.c:3269 [inline]
         [<000000008d1c66a1>] kmem_cache_alloc_node+0x153/0x2a0 mm/slab.c:3579
         [<00000000447d9496>] __alloc_skb+0x6e/0x210 net/core/skbuff.c:198
         [<000000000cdbf82f>] alloc_skb include/linux/skbuff.h:1058 [inline]
         [<000000000cdbf82f>] llc_alloc_frame+0x66/0x110 net/llc/llc_sap.c:54
         [<000000002418b52e>] llc_conn_ac_send_sabme_cmd_p_set_x+0x2f/0x140  net/llc/llc_c_ac.c:777
         [<000000001372ae17>] llc_exec_conn_trans_actions net/llc/llc_conn.c:475  [inline]
         [<000000001372ae17>] llc_conn_service net/llc/llc_conn.c:400 [inline]
         [<000000001372ae17>] llc_conn_state_process+0x1ac/0x640  net/llc/llc_conn.c:75
         [<00000000f27e53c1>] llc_establish_connection+0x110/0x170  net/llc/llc_if.c:109
         [<00000000291b2ca0>] llc_ui_connect+0x10e/0x370 net/llc/af_llc.c:477
         [<000000000f9c740b>] __sys_connect+0x11d/0x170 net/socket.c:1840
         [...]

The bug is that most callers of llc_conn_send_pdu() assume it consumes a
reference to the skb, when actually due to commit b85ab56c3f81 ("llc:
properly handle dev_queue_xmit() return value") it doesn't.

Revert most of that commit, and instead make the few places that need
llc_conn_send_pdu() to *not* consume a reference call skb_get() before.

Fixes: b85ab56c3f81 ("llc: properly handle dev_queue_xmit() return value")
Reported-by: syzbot+6b825a6494a04cc0e3f7@syzkaller.appspotmail.com
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 include/net/llc_conn.h |    2 +-
 net/llc/llc_c_ac.c     |    8 ++++++--
 net/llc/llc_conn.c     |   32 +++++++++-----------------------
 3 files changed, 16 insertions(+), 26 deletions(-)

--- a/include/net/llc_conn.h
+++ b/include/net/llc_conn.h
@@ -104,7 +104,7 @@ void llc_sk_reset(struct sock *sk);
 
 /* Access to a connection */
 int llc_conn_state_process(struct sock *sk, struct sk_buff *skb);
-int llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb);
+void llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb);
 void llc_conn_rtn_pdu(struct sock *sk, struct sk_buff *skb);
 void llc_conn_resend_i_pdu_as_cmd(struct sock *sk, u8 nr, u8 first_p_bit);
 void llc_conn_resend_i_pdu_as_rsp(struct sock *sk, u8 nr, u8 first_f_bit);
--- a/net/llc/llc_c_ac.c
+++ b/net/llc/llc_c_ac.c
@@ -372,6 +372,7 @@ int llc_conn_ac_send_i_cmd_p_set_1(struc
 	llc_pdu_init_as_i_cmd(skb, 1, llc->vS, llc->vR);
 	rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
 	if (likely(!rc)) {
+		skb_get(skb);
 		llc_conn_send_pdu(sk, skb);
 		llc_conn_ac_inc_vs_by_1(sk, skb);
 	}
@@ -389,7 +390,8 @@ static int llc_conn_ac_send_i_cmd_p_set_
 	llc_pdu_init_as_i_cmd(skb, 0, llc->vS, llc->vR);
 	rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
 	if (likely(!rc)) {
-		rc = llc_conn_send_pdu(sk, skb);
+		skb_get(skb);
+		llc_conn_send_pdu(sk, skb);
 		llc_conn_ac_inc_vs_by_1(sk, skb);
 	}
 	return rc;
@@ -406,6 +408,7 @@ int llc_conn_ac_send_i_xxx_x_set_0(struc
 	llc_pdu_init_as_i_cmd(skb, 0, llc->vS, llc->vR);
 	rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
 	if (likely(!rc)) {
+		skb_get(skb);
 		llc_conn_send_pdu(sk, skb);
 		llc_conn_ac_inc_vs_by_1(sk, skb);
 	}
@@ -916,7 +919,8 @@ static int llc_conn_ac_send_i_rsp_f_set_
 	llc_pdu_init_as_i_cmd(skb, llc->ack_pf, llc->vS, llc->vR);
 	rc = llc_mac_hdr_init(skb, llc->dev->dev_addr, llc->daddr.mac);
 	if (likely(!rc)) {
-		rc = llc_conn_send_pdu(sk, skb);
+		skb_get(skb);
+		llc_conn_send_pdu(sk, skb);
 		llc_conn_ac_inc_vs_by_1(sk, skb);
 	}
 	return rc;
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -30,7 +30,7 @@
 #endif
 
 static int llc_find_offset(int state, int ev_type);
-static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *skb);
+static void llc_conn_send_pdus(struct sock *sk);
 static int llc_conn_service(struct sock *sk, struct sk_buff *skb);
 static int llc_exec_conn_trans_actions(struct sock *sk,
 				       struct llc_conn_state_trans *trans,
@@ -193,11 +193,11 @@ out_skb_put:
 	return rc;
 }
 
-int llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb)
+void llc_conn_send_pdu(struct sock *sk, struct sk_buff *skb)
 {
 	/* queue PDU to send to MAC layer */
 	skb_queue_tail(&sk->sk_write_queue, skb);
-	return llc_conn_send_pdus(sk, skb);
+	llc_conn_send_pdus(sk);
 }
 
 /**
@@ -255,7 +255,7 @@ void llc_conn_resend_i_pdu_as_cmd(struct
 	if (howmany_resend > 0)
 		llc->vS = (llc->vS + 1) % LLC_2_SEQ_NBR_MODULO;
 	/* any PDUs to re-send are queued up; start sending to MAC */
-	llc_conn_send_pdus(sk, NULL);
+	llc_conn_send_pdus(sk);
 out:;
 }
 
@@ -296,7 +296,7 @@ void llc_conn_resend_i_pdu_as_rsp(struct
 	if (howmany_resend > 0)
 		llc->vS = (llc->vS + 1) % LLC_2_SEQ_NBR_MODULO;
 	/* any PDUs to re-send are queued up; start sending to MAC */
-	llc_conn_send_pdus(sk, NULL);
+	llc_conn_send_pdus(sk);
 out:;
 }
 
@@ -340,16 +340,12 @@ out:
 /**
  *	llc_conn_send_pdus - Sends queued PDUs
  *	@sk: active connection
- *	@hold_skb: the skb held by caller, or NULL if does not care
  *
- *	Sends queued pdus to MAC layer for transmission. When @hold_skb is
- *	NULL, always return 0. Otherwise, return 0 if @hold_skb is sent
- *	successfully, or 1 for failure.
+ *	Sends queued pdus to MAC layer for transmission.
  */
-static int llc_conn_send_pdus(struct sock *sk, struct sk_buff *hold_skb)
+static void llc_conn_send_pdus(struct sock *sk)
 {
 	struct sk_buff *skb;
-	int ret = 0;
 
 	while ((skb = skb_dequeue(&sk->sk_write_queue)) != NULL) {
 		struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(skb);
@@ -361,20 +357,10 @@ static int llc_conn_send_pdus(struct soc
 			skb_queue_tail(&llc_sk(sk)->pdu_unack_q, skb);
 			if (!skb2)
 				break;
-			dev_queue_xmit(skb2);
-		} else {
-			bool is_target = skb == hold_skb;
-			int rc;
-
-			if (is_target)
-				skb_get(skb);
-			rc = dev_queue_xmit(skb);
-			if (is_target)
-				ret = rc;
+			skb = skb2;
 		}
+		dev_queue_xmit(skb);
 	}
-
-	return ret;
 }
 
 /**



  parent reply	other threads:[~2019-11-04 21:49 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-04 21:44 [PATCH 4.4 00/46] 4.4.199-stable review Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 01/46] dm snapshot: use mutex instead of rw_semaphore Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 02/46] dm snapshot: introduce account_start_copy() and account_end_copy() Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 03/46] dm snapshot: rework COW throttling to fix deadlock Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 04/46] dm: Use kzalloc for all structs with embedded biosets/mempools Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 05/46] sc16is7xx: Fix for "Unexpected interrupt: 8" Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 06/46] x86/cpu: Add Atom Tremont (Jacobsville) Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 07/46] scripts/setlocalversion: Improve -dirty check with git-status --no-optional-locks Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 08/46] usb: handle warm-reset port requests on hub resume Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 09/46] exec: load_script: Do not exec truncated interpreter path Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 10/46] iio: fix center temperature of bmc150-accel-core Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 11/46] perf map: Fix overlapped map handling Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 12/46] RDMA/iwcm: Fix a lock inversion issue Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 13/46] fs: cifs: mute -Wunused-const-variable message Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 14/46] serial: mctrl_gpio: Check for NULL pointer Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 15/46] efi/cper: Fix endianness of PCIe class code Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 16/46] efi/x86: Do not clean dummy variable in kexec path Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 17/46] fs: ocfs2: fix possible null-pointer dereferences in ocfs2_xa_prepare_entry() Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 18/46] fs: ocfs2: fix a possible null-pointer dereference in ocfs2_info_scan_inode_alloc() Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 19/46] MIPS: fw: sni: Fix out of bounds init of o32 stack Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 20/46] NFSv4: Fix leak of clp->cl_acceptor string Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 21/46] tracing: Initialize iter->seq after zeroing in tracing_read_pipe() Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 22/46] USB: legousbtower: fix a signedness bug in tower_probe() Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 23/46] thunderbolt: Use 32-bit writes when writing ring producer/consumer Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 24/46] ath6kl: fix a NULL-ptr-deref bug in ath6kl_usb_alloc_urb_from_pipe() Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 25/46] fuse: flush dirty data/metadata before non-truncate setattr Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 26/46] fuse: truncate pending writes on O_TRUNC Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 27/46] ALSA: bebob: Fix prototype of helper function to return negative value Greg Kroah-Hartman
2019-11-04 21:44 ` [PATCH 4.4 28/46] UAS: Revert commit 3ae62a42090f ("UAS: fix alignment of scatter/gather segments") Greg Kroah-Hartman
2019-11-05 14:26   ` Oliver Neukum
2019-11-04 21:45 ` [PATCH 4.4 29/46] USB: gadget: Reject endpoints with 0 maxpacket value Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 30/46] USB: ldusb: fix ring-buffer locking Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 31/46] USB: ldusb: fix control-message timeout Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 32/46] USB: serial: whiteheat: fix potential slab corruption Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 33/46] USB: serial: whiteheat: fix line-speed endianness Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 34/46] HID: Fix assumption that devices have inputs Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 35/46] HID: fix error message in hid_open_report() Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 36/46] nl80211: fix validation of mesh path nexthop Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 37/46] s390/cmm: fix information leak in cmm_timeout_handler() Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 38/46] rtlwifi: Fix potential overflow on P2P code Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 39/46] llc: fix sk_buff leak in llc_sap_state_process() Greg Kroah-Hartman
2019-11-04 21:45 ` Greg Kroah-Hartman [this message]
2019-11-04 21:45 ` [PATCH 4.4 41/46] bonding: fix potential NULL deref in bond_update_slave_arr Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 42/46] net: usb: sr9800: fix uninitialized local variable Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 43/46] sch_netem: fix rcu splat in netem_enqueue() Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 44/46] sctp: fix the issue that flags are ignored when using kernel_connect Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 45/46] sctp: not bind the socket in sctp_connect Greg Kroah-Hartman
2019-11-04 21:45 ` [PATCH 4.4 46/46] xfs: Correctly invert xfs_buftarg LRU isolation logic Greg Kroah-Hartman
2019-11-05  1:36 ` [PATCH 4.4 00/46] 4.4.199-stable review kernelci.org bot
2019-11-05  6:44 ` Naresh Kamboju
2019-11-05 14:23 ` Guenter Roeck
2019-11-05 23:36 ` Jon Hunter
2019-11-05 23:36   ` Jon Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191104211913.156429047@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=ebiggers@google.com \
    --cc=jakub.kicinski@netronome.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=syzbot+6b825a6494a04cc0e3f7@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.