lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 28 Mar 2017 23:10:18 -0700
From:   Michael Scott <michael.scott@...aro.org>
To:     Marcel Holtmann <marcel@...tmann.org>,
        Gustavo Padovan <gustavo@...ovan.org>,
        Johan Hedberg <johan.hedberg@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        Jukka Rissanen <jukka.rissanen@...ux.intel.com>,
        linux-bluetooth@...r.kernel.org, linux-wpan@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        Michael Scott <michael.scott@...aro.org>
Subject: [PATCH] bluetooth: 6lowpan: fix use after free in chan_suspend/resume

A status field in the skb_cb struct was storing a channel status
based on channel suspend/resume events.  This stored status was
then used to return EAGAIN if there were packet sending issues
in snd_pkt().

The issue is that the skb has been freed by the time the callback
to 6lowpan's suspend/resume was called.  So, this generates a
"use after free" issue that was noticed while running kernel tests
with KASAN debug enabled.

Let's eliminate the status field entirely as we can use the channel
tx_credits to indicate whether we should return EAGAIN when handling
packets.

Signed-off-by: Michael Scott <michael.scott@...aro.org>
---
 net/bluetooth/6lowpan.c | 21 +++------------------
 1 file changed, 3 insertions(+), 18 deletions(-)

diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
index d491529332f4..e27be3ca0a0c 100644
--- a/net/bluetooth/6lowpan.c
+++ b/net/bluetooth/6lowpan.c
@@ -38,7 +38,6 @@ struct skb_cb {
 	struct in6_addr addr;
 	struct in6_addr gw;
 	struct l2cap_chan *chan;
-	int status;
 };
 #define lowpan_cb(skb) ((struct skb_cb *)((skb)->cb))
 
@@ -528,7 +527,7 @@ static int send_pkt(struct l2cap_chan *chan, struct sk_buff *skb,
 	}
 
 	if (!err)
-		err = lowpan_cb(skb)->status;
+		err = (!chan->tx_credits ? -EAGAIN : 0);
 
 	if (err < 0) {
 		if (err == -EAGAIN)
@@ -964,26 +963,12 @@ static struct sk_buff *chan_alloc_skb_cb(struct l2cap_chan *chan,
 
 static void chan_suspend_cb(struct l2cap_chan *chan)
 {
-	struct sk_buff *skb = chan->data;
-
-	BT_DBG("chan %p conn %p skb %p", chan, chan->conn, skb);
-
-	if (!skb)
-		return;
-
-	lowpan_cb(skb)->status = -EAGAIN;
+	BT_DBG("chan %p suspend", chan);
 }
 
 static void chan_resume_cb(struct l2cap_chan *chan)
 {
-	struct sk_buff *skb = chan->data;
-
-	BT_DBG("chan %p conn %p skb %p", chan, chan->conn, skb);
-
-	if (!skb)
-		return;
-
-	lowpan_cb(skb)->status = 0;
+	BT_DBG("chan %p resume", chan);
 }
 
 static long chan_get_sndtimeo_cb(struct l2cap_chan *chan)
-- 
2.11.0

Powered by blists - more mailing lists