[RFC] Bluetooth: Lock l2cap chan in sockopt

[RFC] Bluetooth: Lock l2cap chan in sockopt

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>


Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
---
 net/bluetooth/l2cap_sock.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index a4bb27e..373ce9c 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -275,6 +275,7 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
 	if (get_user(len, optlen))
 		return -EFAULT;
 
+	l2cap_chan_lock(chan);
 	lock_sock(sk);
 
 	switch (optname) {
@@ -345,6 +346,8 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
 	}
 
 	release_sock(sk);
+	l2cap_chan_unlock(chan);
+
 	return err;
 }
 
@@ -367,6 +370,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
 	if (get_user(len, optlen))
 		return -EFAULT;
 
+	l2cap_chan_lock(chan);
 	lock_sock(sk);
 
 	switch (optname) {
@@ -442,6 +446,8 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
 	}
 
 	release_sock(sk);
+	l2cap_chan_unlock(chan);
+
 	return err;
 }
 
@@ -471,6 +477,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
 
 	BT_DBG("sk %p", sk);
 
+	l2cap_chan_lock(chan);
 	lock_sock(sk);
 
 	switch (optname) {
@@ -556,6 +563,8 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
 	}
 
 	release_sock(sk);
+	l2cap_chan_unlock(chan);
+
 	return err;
 }
 
@@ -577,6 +586,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
 	if (level != SOL_BLUETOOTH)
 		return -ENOPROTOOPT;
 
+	l2cap_chan_lock(chan);
 	lock_sock(sk);
 
 	switch (optname) {
@@ -729,6 +739,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
 	}
 
 	release_sock(sk);
+	l2cap_chan_unlock(chan);
+
 	return err;
 }
 
-- 
1.7.9.5

Re: [RFC] Bluetooth: Lock l2cap chan in sockopt

[Mat Martineau]

Andrei -

On Wed, 6 Jun 2012, Andrei Emeltchenko wrote:

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
>
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Commit message is quite short :)  (I know, it's just an RFC)

> ---
> net/bluetooth/l2cap_sock.c |   12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index a4bb27e..373ce9c 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -275,6 +275,7 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
> 	if (get_user(len, optlen))
> 		return -EFAULT;
>
> +	l2cap_chan_lock(chan);
> 	lock_sock(sk);
>
> 	switch (optname) {
> @@ -345,6 +346,8 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
> 	}
>
> 	release_sock(sk);
> +	l2cap_chan_unlock(chan);
> +
> 	return err;
> }
>
> @@ -367,6 +370,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
> 	if (get_user(len, optlen))
> 		return -EFAULT;
>
> +	l2cap_chan_lock(chan);
> 	lock_sock(sk);
>
> 	switch (optname) {
> @@ -442,6 +446,8 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
> 	}
>
> 	release_sock(sk);
> +	l2cap_chan_unlock(chan);
> +
> 	return err;
> }
>
> @@ -471,6 +477,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
>
> 	BT_DBG("sk %p", sk);
>
> +	l2cap_chan_lock(chan);
> 	lock_sock(sk);
>
> 	switch (optname) {
> @@ -556,6 +563,8 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
> 	}
>
> 	release_sock(sk);
> +	l2cap_chan_unlock(chan);
> +
> 	return err;
> }
>
> @@ -577,6 +586,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
> 	if (level != SOL_BLUETOOTH)
> 		return -ENOPROTOOPT;
>
> +	l2cap_chan_lock(chan);
> 	lock_sock(sk);
>
> 	switch (optname) {
> @@ -729,6 +739,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
> 	}
>
> 	release_sock(sk);
> +	l2cap_chan_unlock(chan);
> +
> 	return err;
> }
>
> -- 
> 1.7.9.5

Code looks good to me.  In the near future, BT_CHANNEL_POLICY will 
call in to the L2CAP core to do channel moves, but I think it helps to 
already have the lock held in that case.


--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum

Re: [RFC] Bluetooth: Lock l2cap chan in sockopt

[Andrei Emeltchenko]

Hi Mat,

On Fri, Jun 08, 2012 at 11:02:47AM -0700, Mat Martineau wrote:
> >Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Commit message is quite short :)  (I know, it's just an RFC)

Maybe I need to put one oops dump, but I was not able to reproduce it.

[  745.772484] BUG: unable to handle kernel NULL pointer dereference at
(null)
[  745.774157] IP: [<f828dccc>] l2cap_sock_getsockopt+0x2ec/0x3f0
[bluetooth]
[  745.775077] *pde = 00000000 
[  745.776483] Oops: 0000 [#1] SMP 


Best regards 
Andrei Emeltchenko