linux-kernel - [PATCH] rfcomm/core.c avoid dangling pointer, check session exists

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]

Message-ID: <20110521190253.GA24600@spacedout.fries.net>
Date:	Sat, 21 May 2011 14:02:53 -0500
From:	David Fries <david@...es.net>
To:	linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:	"Gustavo F. Padovan" <padovan@...fusion.mobi>,
	Marcel Holtmann <marcel@...tmann.org>
Subject: [PATCH] rfcomm/core.c avoid dangling pointer, check session exists

rfcomm_process_sessions is calling rfcomm_process_rx, but
in this case the session is closed and freed leaving a
dangling pointer that blows up when rfcomm_process_rx returns
and rfcomm_process_dlcs is called with the now dangling session
pointer.  Check to see if the entry is still in the list.

Signed-off-by: David Fries <David@...es.net>
Cc: Marcel Holtmann <marcel@...tmann.org>
Cc: "Gustavo F. Padovan" <padovan@...fusion.mobi>
---
I sent out an ealier patch,
Date: Mon, 21 Mar 2011 21:38:10 -0500
Subject: [PATCH] rfcomm/core.c avoid dangling pointer, check session

That version added a return value to rfcomm_session_close to determine
if the session was closed.  I thought this would be cleaner.

I can reproduce using blueman-manager on desktop, and Motorola S305 bluetooth
headset, 2.6.39, but it can take a few attempts.  Start out with the
desktop as the last device the S305 paired with.
desktop, connect to the S305,
S305, turn on
desktop (connection fails)
desktop (connection automatically comes up now that S305 is on)
desktop disconnect S305
desktop (kernel panic)

While rfcomm_process_sessions looks symmetrical,
rfcomm_session_hold(s);
rfcomm_process_rx
rfcomm_process_dlcs
rfcomm_session_put(s);

rfcomm_process_rx
if (sk->sk_state == BT_CLOSED) {
        if (!s->initiator)
                rfcomm_session_put(s);
        rfcomm_session_close(s, sk->sk_err);

Which isn't symmetrical.  I don't know enough about the subsystem to
know if there is a better cleaner way to fix this, or if my patch is
the best solution.

 net/bluetooth/rfcomm/core.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index c997393..ac47ef3 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1952,6 +1952,12 @@ static inline void rfcomm_process_sessions(void)

 		default:
 			rfcomm_process_rx(s);
+			/* The current session can be closed as part of rx
+			 * in which case s is now dangling.  Check if it
+			 * has been removed.
+			 */
+			if(n->prev != p)
+				continue;
 			break;
 		}

-- 
1.7.2.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/