Skip to contentSkip to conversation

BUG in rfcomm_sock_getsockopt+0x128/0x200

DISCUSSION Fabio Rossi 1 participant 1 message Nov 2, 2013 Bluetooth drivers

lore
Share & tools
lore
#0 lore raw

Hi Marcel,

Quoted 1 line

I finally managed to reproduce it. It does not always happen. And strangely

enough I can only trigger it when enabling experimental features of bluetoothd with -E command line switch.

Quoted 2 lines

But I have no idea why your bisecting points to that specific commit. And

more important it used to work just fine (see below). However I can tell you what makes the code crash.

Quoted 4 lines

0x1313 is in rfcomm_sock_getsockopt (net/bluetooth/rfcomm/sock.c:743). 738
739 static int rfcomm_sock_getsockopt_old(struct socket *sock, int optname,

char __user *optval, int __user *optlen)

Quoted 10 lines

740 {
741 struct sock *sk = sock->sk;
742 struct rfcomm_conninfo cinfo;
743 struct l2cap_conn *conn = l2cap_pi(sk)->chan->conn;
744 int len, err = 0;
745 u32 opt;
746
747 BT_DBG("sk %p", sk);

The l2cap_pi(sk) is fully broken. That is an rfcomm_pi(sk). The commit that

broke this is actually from an earlier time. I have this one:

net/bluetooth/rfcomm/sock.c static int rfcomm_sock_getsockopt_old(struct socket

Quoted 12 lines

commit 8c1d787be4b62d2d1b6f04953eca4bcf7c839d44

Author: Gustavo F. Padovan <[email protected]>
Date:   Wed Apr 13 20:23:55 2011 -0300

Bluetooth: Move conn to struct l2cap_chan

net/bluetooth/rfcomm/sock.c +0 -0
index 66cc1f0c3df8..386cfaffd4b7 100644
@@ -743,6 +743,7 @@ static int rfcomm_sock_getsockopt_old(struct socket 

*sock, int optname, char __u

net/bluetooth/rfcomm/sock.c static int rfcomm_sock_getsockopt_old(struct socket

Quoted 8 lines
net/bluetooth/rfcomm/sock.c +1 -0
       struct sock *sk = sock->sk;
       struct sock *l2cap_sk;
       struct rfcomm_conninfo cinfo;
+       struct l2cap_conn *conn = l2cap_pi(sk)->chan->conn;
       int len, err = 0;
       u32 opt;
 
@@ -787,8 +788,8 @@ static int rfcomm_sock_getsockopt_old(struct socket 

*sock, int optname, char __u

Quoted 10 lines
net/bluetooth/rfcomm/sock.c +2 -2
               l2cap_sk = rfcomm_pi(sk)->dlc->session->sock->sk;
 
-               cinfo.hci_handle = l2cap_pi(l2cap_sk)->conn->hcon->handle;
-               memcpy(cinfo.dev_class, l2cap_pi(l2cap_sk)->conn->hcon-
 dev_class, 3);
+               cinfo.hci_handle = conn->hcon->handle;
+               memcpy(cinfo.dev_class, conn->hcon->dev_class, 3);
 

The conversion is clearly wrong since we used to have a l2cap_sk that was

pointing to the right socket.

Quoted 2 lines

This should have blown up month ago and not just with the latest changes we

have done to the L2CAP layer. Anyhow, you can try this small change and see if it fixes things for you.

net/bluetooth/rfcomm/sock.c static int rfcomm_sock_setsockopt(struct socket *sock,

Quoted 6 lines
net/bluetooth/rfcomm/sock.c +0 -0
index c4d3d423f89b..0be7619c5e5e 100644
@@ -739,8 +739,9 @@ static int rfcomm_sock_setsockopt(struct socket *sock, 

int level, int optname, c

Quoted 1 line

static int rfcomm_sock_getsockopt_old(struct socket *sock, int optname, char

__user *optval, int __user *optlen)

net/bluetooth/rfcomm/sock.c static int rfcomm_sock_getsockopt_old(struct socket

Quoted 10 lines
{
       struct sock *sk = sock->sk;
net/bluetooth/rfcomm/sock.c +2 -1
+       struct sock *l2cap_sk;
+       struct l2cap_conn *conn;
       struct rfcomm_conninfo cinfo;
-       struct l2cap_conn *conn = l2cap_pi(sk)->chan->conn;
       int len, err = 0;
       u32 opt;
@@ -783,6 +784,9 @@ static int rfcomm_sock_getsockopt_old(struct socket 

*sock, int optname, char __u

Quoted 11 lines
net/bluetooth/rfcomm/sock.c +3 -0
                       break;
               }
 
+               l2cap_sk = rfcomm_pi(sk)->dlc->session->sock->sk;
+               conn = l2cap_pi(l2cap_sk)->chan->conn;
+
               memset(&cinfo, 0, sizeof(cinfo));
               cinfo.hci_handle = conn->hcon->handle;
               memcpy(cinfo.dev_class, conn->hcon->dev_class, 3);

Regards

This patch solves the issue, I don't see the crash anymore.

Thanks,
Fabio