It also have to change the name of the function to
l2cap_get_sock_by_addr() because we do hold the lock inside it now.
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 17 ++++++-----------
1 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 6f931cc..3d48867 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -728,15 +728,18 @@ static inline void l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, stru
}
/* ---- Socket interface ---- */
-static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
+static struct sock *l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
{
struct sock *sk;
struct hlist_node *node;
+
+ write_lock_bh(&l2cap_sk_list.lock);
sk_for_each(sk, node, &l2cap_sk_list.head)
if (l2cap_pi(sk)->sport == psm && !bacmp(&bt_sk(sk)->src, src))
goto found;
sk = NULL;
found:
+ write_unlock_bh(&l2cap_sk_list.lock);
return sk;
}
@@ -1024,9 +1027,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
}
}
- write_lock_bh(&l2cap_sk_list.lock);
-
- if (la.l2_psm && __l2cap_get_sock_by_addr(la.l2_psm, &la.l2_bdaddr)) {
+ if (la.l2_psm && l2cap_get_sock_by_addr(la.l2_psm, &la.l2_bdaddr)) {
err = -EADDRINUSE;
} else {
/* Save source address */
@@ -1040,8 +1041,6 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
l2cap_pi(sk)->sec_level = BT_SECURITY_SDP;
}
- write_unlock_bh(&l2cap_sk_list.lock);
-
done:
release_sock(sk);
return err;
@@ -1257,18 +1256,14 @@ static int l2cap_sock_listen(struct socket *sock, int backlog)
err = -EINVAL;
- write_lock_bh(&l2cap_sk_list.lock);
-
for (psm = 0x1001; psm < 0x1100; psm += 2)
- if (!__l2cap_get_sock_by_addr(cpu_to_le16(psm), src)) {
+ if (!l2cap_get_sock_by_addr(cpu_to_le16(psm), src)) {
l2cap_pi(sk)->psm = cpu_to_le16(psm);
l2cap_pi(sk)->sport = cpu_to_le16(psm);
err = 0;
break;
}
- write_unlock_bh(&l2cap_sk_list.lock);
-
if (err < 0)
goto done;
}
--
1.7.3.1
* Gustavo F. Padovan <[email protected]> [2010-11-17 21:11:41 -0200]:
> * Gustavo F. Padovan <[email protected]> [2010-11-05 10:37:11 -0400]:
>
> > Hi Ville,
> >
> > * Ville Tervo <[email protected]> [2010-11-05 15:49:35 +0200]:
> >
> > > Hi Gustavo,
> > >
> > > On Tue, Nov 02, 2010 at 04:03:12PM +0100, ext Gustavo F. Padovan wrote:
> > > > It also have to change the name of the function to
> > > > l2cap_get_sock_by_addr() because we do hold the lock inside it now.
> > > >
> > > > Signed-off-by: Gustavo F. Padovan <[email protected]>
> > > > ---
> > > > net/bluetooth/l2cap.c | 17 ++++++-----------
> > > > 1 files changed, 6 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > > > index 6f931cc..3d48867 100644
> > > > --- a/net/bluetooth/l2cap.c
> > > > +++ b/net/bluetooth/l2cap.c
> > > > @@ -728,15 +728,18 @@ static inline void l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, stru
> > > > }
> > > >
> > > > /* ---- Socket interface ---- */
> > > > -static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > > +static struct sock *l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > > {
> > > > struct sock *sk;
> > > > struct hlist_node *node;
> > > > +
> > > > + write_lock_bh(&l2cap_sk_list.lock);
> > >
> > > Code is only reading so read_lock_bh would be enough?
> >
> > Sure, I didn't looked to that, I just keept the same code that we were
> > using before. I'll fix it.
>
> I figured out that we need write_lock_bh() here, because set the psm and
> sport is like a new element to the list. l2cap_get_sock_by_addr()
> searches for either psm or sport.
>
> I'm also dropping the option to use RCU on the bt_sk_list(), It does not
> fit on our case. We can't have anyone writing the list while we are
> reading it.
That said, only patch 4 and 5 are still valid (I'll resend them), and 6 is
so trivial that I put it upstream already.
--
Gustavo F. Padovan
http://profusion.mobi
* Gustavo F. Padovan <[email protected]> [2010-11-05 10:37:11 -0400]:
> Hi Ville,
>
> * Ville Tervo <[email protected]> [2010-11-05 15:49:35 +0200]:
>
> > Hi Gustavo,
> >
> > On Tue, Nov 02, 2010 at 04:03:12PM +0100, ext Gustavo F. Padovan wrote:
> > > It also have to change the name of the function to
> > > l2cap_get_sock_by_addr() because we do hold the lock inside it now.
> > >
> > > Signed-off-by: Gustavo F. Padovan <[email protected]>
> > > ---
> > > net/bluetooth/l2cap.c | 17 ++++++-----------
> > > 1 files changed, 6 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > > index 6f931cc..3d48867 100644
> > > --- a/net/bluetooth/l2cap.c
> > > +++ b/net/bluetooth/l2cap.c
> > > @@ -728,15 +728,18 @@ static inline void l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, stru
> > > }
> > >
> > > /* ---- Socket interface ---- */
> > > -static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > +static struct sock *l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > {
> > > struct sock *sk;
> > > struct hlist_node *node;
> > > +
> > > + write_lock_bh(&l2cap_sk_list.lock);
> >
> > Code is only reading so read_lock_bh would be enough?
>
> Sure, I didn't looked to that, I just keept the same code that we were
> using before. I'll fix it.
I figured out that we need write_lock_bh() here, because set the psm and
sport is like a new element to the list. l2cap_get_sock_by_addr()
searches for either psm or sport.
I'm also dropping the option to use RCU on the bt_sk_list(), It does not
fit on our case. We can't have anyone writing the list while we are
reading it.
--
Gustavo F. Padovan
http://profusion.mobi
Hi Marcel,
* Marcel Holtmann <[email protected]> [2010-11-10 14:39:55 +0900]:
> Hi Gustavo,
>
> > > On Tue, Nov 02, 2010 at 04:03:12PM +0100, ext Gustavo F. Padovan wrote:
> > > > It also have to change the name of the function to
> > > > l2cap_get_sock_by_addr() because we do hold the lock inside it now.
> > > >
> > > > Signed-off-by: Gustavo F. Padovan <[email protected]>
> > > > ---
> > > > net/bluetooth/l2cap.c | 17 ++++++-----------
> > > > 1 files changed, 6 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > > > index 6f931cc..3d48867 100644
> > > > --- a/net/bluetooth/l2cap.c
> > > > +++ b/net/bluetooth/l2cap.c
> > > > @@ -728,15 +728,18 @@ static inline void l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, stru
> > > > }
> > > >
> > > > /* ---- Socket interface ---- */
> > > > -static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > > +static struct sock *l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > > {
> > > > struct sock *sk;
> > > > struct hlist_node *node;
> > > > +
> > > > + write_lock_bh(&l2cap_sk_list.lock);
> > >
> > > Code is only reading so read_lock_bh would be enough?
> >
> > Sure, I didn't looked to that, I just keept the same code that we were
> > using before. I'll fix it.
>
> we might also not just bother with read/write locks. Since they are not
> always the right thing to do. In a lot of cases a pure spinlock is just
> better. And in case of Bluetooth I think we would be just fine with
> using a pure spinlock. You might run some tests with this.
Better than spinlock is move everything to RCU, than we will just need a
mutex or spinlock in the write operations. Using a RCU is a lot simpler.
These patches are a clean up to add RCU to sk_list, so I propose to
upstream the new version of my patches and do the RCU change on top
of them.
After all these changes we will be ready for the l2cap.c split.
--
Gustavo F. Padovan
http://profusion.mobi
Hi Gustavo,
> > On Tue, Nov 02, 2010 at 04:03:12PM +0100, ext Gustavo F. Padovan wrote:
> > > It also have to change the name of the function to
> > > l2cap_get_sock_by_addr() because we do hold the lock inside it now.
> > >
> > > Signed-off-by: Gustavo F. Padovan <[email protected]>
> > > ---
> > > net/bluetooth/l2cap.c | 17 ++++++-----------
> > > 1 files changed, 6 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > > index 6f931cc..3d48867 100644
> > > --- a/net/bluetooth/l2cap.c
> > > +++ b/net/bluetooth/l2cap.c
> > > @@ -728,15 +728,18 @@ static inline void l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, stru
> > > }
> > >
> > > /* ---- Socket interface ---- */
> > > -static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > +static struct sock *l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > > {
> > > struct sock *sk;
> > > struct hlist_node *node;
> > > +
> > > + write_lock_bh(&l2cap_sk_list.lock);
> >
> > Code is only reading so read_lock_bh would be enough?
>
> Sure, I didn't looked to that, I just keept the same code that we were
> using before. I'll fix it.
we might also not just bother with read/write locks. Since they are not
always the right thing to do. In a lot of cases a pure spinlock is just
better. And in case of Bluetooth I think we would be just fine with
using a pure spinlock. You might run some tests with this.
Regards
Marcel
Hi Ville,
* Ville Tervo <[email protected]> [2010-11-05 15:49:35 +0200]:
> Hi Gustavo,
>
> On Tue, Nov 02, 2010 at 04:03:12PM +0100, ext Gustavo F. Padovan wrote:
> > It also have to change the name of the function to
> > l2cap_get_sock_by_addr() because we do hold the lock inside it now.
> >
> > Signed-off-by: Gustavo F. Padovan <[email protected]>
> > ---
> > net/bluetooth/l2cap.c | 17 ++++++-----------
> > 1 files changed, 6 insertions(+), 11 deletions(-)
> >
> > diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> > index 6f931cc..3d48867 100644
> > --- a/net/bluetooth/l2cap.c
> > +++ b/net/bluetooth/l2cap.c
> > @@ -728,15 +728,18 @@ static inline void l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, stru
> > }
> >
> > /* ---- Socket interface ---- */
> > -static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > +static struct sock *l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> > {
> > struct sock *sk;
> > struct hlist_node *node;
> > +
> > + write_lock_bh(&l2cap_sk_list.lock);
>
> Code is only reading so read_lock_bh would be enough?
Sure, I didn't looked to that, I just keept the same code that we were
using before. I'll fix it.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
Hi Ville,
* Ville Tervo <[email protected]> [2010-11-05 16:01:58 +0200]:
> Hi Gustavo,
>
> On Tue, Nov 02, 2010 at 04:03:18PM +0100, ext Gustavo F. Padovan wrote:
> > Return 0 in that situation could lead to errors in the caller.
> >
> > Signed-off-by: Gustavo F. Padovan <[email protected]>
> > ---
> > net/bluetooth/rfcomm/sock.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> > index e48fbca..cd7e27a 100644
> > --- a/net/bluetooth/rfcomm/sock.c
> > +++ b/net/bluetooth/rfcomm/sock.c
> > @@ -930,7 +930,7 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
> > /* Check if we have socket listening on channel */
> > parent = rfcomm_get_sock_by_channel(BT_LISTEN, channel, &src);
> > if (!parent)
> > - return 0;
> > + return -EINVAL;
>
> Did you check rfcomm/core.c?
>
> I think this change breaks the logic in there. IOW continues connection
> establisment instead of tearing down the link.
I understood that part of the code wrong, I'm going to drop this patch.
Thanks fo looking into this.
--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi
Hi Gustavo,
On Tue, Nov 02, 2010 at 04:03:18PM +0100, ext Gustavo F. Padovan wrote:
> Return 0 in that situation could lead to errors in the caller.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/rfcomm/sock.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> index e48fbca..cd7e27a 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -930,7 +930,7 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
> /* Check if we have socket listening on channel */
> parent = rfcomm_get_sock_by_channel(BT_LISTEN, channel, &src);
> if (!parent)
> - return 0;
> + return -EINVAL;
Did you check rfcomm/core.c?
I think this change breaks the logic in there. IOW continues connection
establisment instead of tearing down the link.
--
Ville
Hi Gustavo,
On Tue, Nov 02, 2010 at 04:03:12PM +0100, ext Gustavo F. Padovan wrote:
> It also have to change the name of the function to
> l2cap_get_sock_by_addr() because we do hold the lock inside it now.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/l2cap.c | 17 ++++++-----------
> 1 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index 6f931cc..3d48867 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -728,15 +728,18 @@ static inline void l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, stru
> }
>
> /* ---- Socket interface ---- */
> -static struct sock *__l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> +static struct sock *l2cap_get_sock_by_addr(__le16 psm, bdaddr_t *src)
> {
> struct sock *sk;
> struct hlist_node *node;
> +
> + write_lock_bh(&l2cap_sk_list.lock);
Code is only reading so read_lock_bh would be enough?
>
> sk_for_each(sk, node, &l2cap_sk_list.head)
> if (l2cap_pi(sk)->sport == psm && !bacmp(&bt_sk(sk)->src, src))
> goto found;
> sk = NULL;
> found:
> + write_unlock_bh(&l2cap_sk_list.lock);
> return sk;
> }
>
> @@ -1024,9 +1027,7 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
> }
> }
>
> - write_lock_bh(&l2cap_sk_list.lock);
> -
> - if (la.l2_psm && __l2cap_get_sock_by_addr(la.l2_psm, &la.l2_bdaddr)) {
> + if (la.l2_psm && l2cap_get_sock_by_addr(la.l2_psm, &la.l2_bdaddr)) {
> err = -EADDRINUSE;
> } else {
> /* Save source address */
> @@ -1040,8 +1041,6 @@ static int l2cap_sock_bind(struct socket *sock, struct sockaddr *addr, int alen)
> l2cap_pi(sk)->sec_level = BT_SECURITY_SDP;
> }
>
> - write_unlock_bh(&l2cap_sk_list.lock);
> -
> done:
> release_sock(sk);
> return err;
> @@ -1257,18 +1256,14 @@ static int l2cap_sock_listen(struct socket *sock, int backlog)
>
> err = -EINVAL;
>
> - write_lock_bh(&l2cap_sk_list.lock);
> -
> for (psm = 0x1001; psm < 0x1100; psm += 2)
> - if (!__l2cap_get_sock_by_addr(cpu_to_le16(psm), src)) {
> + if (!l2cap_get_sock_by_addr(cpu_to_le16(psm), src)) {
> l2cap_pi(sk)->psm = cpu_to_le16(psm);
> l2cap_pi(sk)->sport = cpu_to_le16(psm);
> err = 0;
> break;
> }
>
> - write_unlock_bh(&l2cap_sk_list.lock);
> -
> if (err < 0)
> goto done;
> }
> --
> 1.7.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Return 0 in that situation could lead to errors in the caller.
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index e48fbca..cd7e27a 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -930,7 +930,7 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
/* Check if we have socket listening on channel */
parent = rfcomm_get_sock_by_channel(BT_LISTEN, channel, &src);
if (!parent)
- return 0;
+ return -EINVAL;
bh_lock_sock(parent);
--
1.7.3.1
Return 0 in that situation could lead to errors in the caller.
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/sco.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 85b5498..f031b62 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -880,7 +880,7 @@ static int sco_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 type)
int lm = 0;
if (type != SCO_LINK && type != ESCO_LINK)
- return 0;
+ return -EINVAL;
BT_DBG("hdev %s, bdaddr %s", hdev->name, batostr(bdaddr));
@@ -906,7 +906,7 @@ static int sco_connect_cfm(struct hci_conn *hcon, __u8 status)
BT_DBG("hcon %p bdaddr %s status %d", hcon, batostr(&hcon->dst), status);
if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK)
- return 0;
+ return -EINVAL;
if (!status) {
struct sco_conn *conn;
@@ -925,7 +925,7 @@ static int sco_disconn_cfm(struct hci_conn *hcon, __u8 reason)
BT_DBG("hcon %p reason %d", hcon, reason);
if (hcon->type != SCO_LINK && hcon->type != ESCO_LINK)
- return 0;
+ return -EINVAL;
sco_conn_del(hcon, bt_err(reason));
--
1.7.3.1
rfcomm_get_sock_by_channel() was the only user of this function, so I merged
both into rfcomm_get_sock_by_channel(). The socket lock now should be hold
outside of rfcomm_get_sock_by_channel() once we hold and release it inside the
same function now.
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 19 +++++++------------
1 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 4ed9499..e48fbca 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -142,11 +142,13 @@ static struct sock *rfcomm_get_sock_by_addr(u8 channel, bdaddr_t *src)
/* Find socket with channel and source bdaddr.
* Returns closest match.
*/
-static struct sock *__rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *src)
+static struct sock *rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *src)
{
struct sock *sk = NULL, *sk1 = NULL;
struct hlist_node *node;
+ read_lock(&rfcomm_sk_list.lock);
+
sk_for_each(sk, node, &rfcomm_sk_list.head) {
if (state && sk->sk_state != state)
continue;
@@ -161,19 +163,10 @@ static struct sock *__rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t
sk1 = sk;
}
}
- return node ? sk : sk1;
-}
-/* Find socket with given address (channel, src).
- * Returns locked socket */
-static inline struct sock *rfcomm_get_sock_by_channel(int state, u8 channel, bdaddr_t *src)
-{
- struct sock *s;
- read_lock(&rfcomm_sk_list.lock);
- s = __rfcomm_get_sock_by_channel(state, channel, src);
- if (s) bh_lock_sock(s);
read_unlock(&rfcomm_sk_list.lock);
- return s;
+
+ return node ? sk : sk1;
}
static void rfcomm_sock_destruct(struct sock *sk)
@@ -939,6 +932,8 @@ int rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc *
if (!parent)
return 0;
+ bh_lock_sock(parent);
+
/* Check for backlog size */
if (sk_acceptq_is_full(parent)) {
BT_DBG("backlog full %d", parent->sk_ack_backlog);
--
1.7.3.1
l2cap_get_sock_by_psm() was the only user of this function, so I merged
both into l2cap_get_sock_by_psm(). The socket lock now should be hold
outside of l2cap_get_sock_by_psm() once we hold and release it inside the
same function now.
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 22 +++++++++-------------
1 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 3d48867..27199bc 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -746,11 +746,13 @@ found:
/* Find socket with psm and source bdaddr.
* Returns closest match.
*/
-static struct sock *__l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src)
+static struct sock *l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src)
{
struct sock *sk = NULL, *sk1 = NULL;
struct hlist_node *node;
+ read_lock(&l2cap_sk_list.lock);
+
sk_for_each(sk, node, &l2cap_sk_list.head) {
if (state && sk->sk_state != state)
continue;
@@ -765,20 +767,10 @@ static struct sock *__l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src
sk1 = sk;
}
}
- return node ? sk : sk1;
-}
-/* Find socket with given address (psm, src).
- * Returns locked socket */
-static inline struct sock *l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src)
-{
- struct sock *s;
- read_lock(&l2cap_sk_list.lock);
- s = __l2cap_get_sock_by_psm(state, psm, src);
- if (s)
- bh_lock_sock(s);
read_unlock(&l2cap_sk_list.lock);
- return s;
+
+ return node ? sk : sk1;
}
static void l2cap_sock_destruct(struct sock *sk)
@@ -2921,6 +2913,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd
goto sendresp;
}
+ bh_lock_sock(parent);
+
/* Check if the ACL is secure enough (if not SDP) */
if (psm != cpu_to_le16(0x0001) &&
!hci_conn_check_link_mode(conn->hcon)) {
@@ -4425,6 +4419,8 @@ static inline int l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, str
if (!sk)
goto drop;
+ bh_lock_sock(sk);
+
BT_DBG("sk %p, len %d", sk, skb->len);
if (sk->sk_state != BT_BOUND && sk->sk_state != BT_CONNECTED)
--
1.7.3.1
It also have to change the name of the function to
rfcomm_get_sock_by_addr() because we do hold the lock inside it now.
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 16 +++++-----------
1 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index aec505f..4ed9499 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -123,16 +123,18 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
}
/* ---- Socket functions ---- */
-static struct sock *__rfcomm_get_sock_by_addr(u8 channel, bdaddr_t *src)
+static struct sock *rfcomm_get_sock_by_addr(u8 channel, bdaddr_t *src)
{
struct sock *sk = NULL;
struct hlist_node *node;
+ write_lock_bh(&rfcomm_sk_list.lock);
sk_for_each(sk, node, &rfcomm_sk_list.head) {
if (rfcomm_pi(sk)->channel == channel &&
!bacmp(&bt_sk(sk)->src, src))
break;
}
+ write_unlock_bh(&rfcomm_sk_list.lock);
return node ? sk : NULL;
}
@@ -374,9 +376,7 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
goto done;
}
- write_lock_bh(&rfcomm_sk_list.lock);
-
- if (sa->rc_channel && __rfcomm_get_sock_by_addr(sa->rc_channel, &sa->rc_bdaddr)) {
+ if (sa->rc_channel && rfcomm_get_sock_by_addr(sa->rc_channel, &sa->rc_bdaddr)) {
err = -EADDRINUSE;
} else {
/* Save source address */
@@ -385,8 +385,6 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
sk->sk_state = BT_BOUND;
}
- write_unlock_bh(&rfcomm_sk_list.lock);
-
done:
release_sock(sk);
return err;
@@ -459,17 +457,13 @@ static int rfcomm_sock_listen(struct socket *sock, int backlog)
err = -EINVAL;
- write_lock_bh(&rfcomm_sk_list.lock);
-
for (channel = 1; channel < 31; channel++)
- if (!__rfcomm_get_sock_by_addr(channel, src)) {
+ if (!rfcomm_get_sock_by_addr(channel, src)) {
rfcomm_pi(sk)->channel = channel;
err = 0;
break;
}
- write_unlock_bh(&rfcomm_sk_list.lock);
-
if (err < 0)
goto done;
}
--
1.7.3.1
It also have to change the name of the function to
sco_get_sock_by_addr() because we do hold the lock inside it now.
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/sco.c | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d0927d1..85b5498 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -276,16 +276,18 @@ drop:
}
/* -------- Socket interface ---------- */
-static struct sock *__sco_get_sock_by_addr(bdaddr_t *ba)
+static struct sock *sco_get_sock_by_addr(bdaddr_t *ba)
{
struct sock *sk;
struct hlist_node *node;
+ write_lock_bh(&sco_sk_list.lock);
sk_for_each(sk, node, &sco_sk_list.head)
if (!bacmp(&bt_sk(sk)->src, ba))
goto found;
sk = NULL;
found:
+ write_unlock_bh(&sco_sk_list.lock);
return sk;
}
@@ -469,9 +471,7 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
goto done;
}
- write_lock_bh(&sco_sk_list.lock);
-
- if (bacmp(src, BDADDR_ANY) && __sco_get_sock_by_addr(src)) {
+ if (bacmp(src, BDADDR_ANY) && sco_get_sock_by_addr(src)) {
err = -EADDRINUSE;
} else {
/* Save source address */
@@ -479,8 +479,6 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
sk->sk_state = BT_BOUND;
}
- write_unlock_bh(&sco_sk_list.lock);
-
done:
release_sock(sk);
return err;
--
1.7.3.1