Return-Path: MIME-Version: 1.0 In-Reply-To: <20101210071716.GK874@null> References: <20101206211516.GH883@vigoh> <4CFE32A0.6090601@nokia.com> <20101207155037.GA2944@vigoh> <4CFF6359.7000305@nokia.com> <20101210071716.GK874@null> Date: Thu, 13 Jan 2011 16:37:37 +0200 Message-ID: Subject: Re: [PATCH 1/1] bluetooth: Fix NULL pointer dereference issue From: Andrei Emeltchenko To: Ville Tervo Cc: Yuri Ershov , "ext Gustavo F. Padovan" , andrei.emeltchenko@nokia.com, "linux-bluetooth@vger.kernel.org" Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi, On Fri, Dec 10, 2010 at 9:17 AM, Ville Tervo wrote: > Hi Yuri, > > On Wed, Dec 08, 2010 at 01:52:09PM +0300, Yuri Ershov wrote: > > >> >>>So in which situations (n == p), or (p == p->next)? That should happen only >> >>>when p is the only element in the list, then p == head, right? >> >>The (n == p) is in situation, when sk is unlinked by task >> >>responsible for handling connect/disconnect requests while the >> >>"bt_accept_dequeue". This condition is indirect checking of sk >> >>validity. >> > >> >Why not using a list lock here instead? Fits a way better. >> > >> Yes, it's better. I tried to use the locks in this function, but it >> slows down the task handling connect/disconnect/etc. events and the >> task skips some events from fast clients. What about preventing tasklet from removing socket with disabling bh? diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c index 8cfb5a8..7ac4ba5 100644 --- a/net/bluetooth/af_bluetooth.c +++ b/net/bluetooth/af_bluetooth.c @@ -199,6 +199,7 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock) BT_DBG("parent %p", parent); + local_bh_disable(); list_for_each_safe(p, n, &bt_sk(parent)->accept_q) { sk = (struct sock *) list_entry(p, struct bt_sock, accept_q); @@ -217,11 +218,14 @@ struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock) if (newsock) sock_graft(sk, newsock); release_sock(sk); + local_bh_enable(); return sk; } release_sock(sk); } + local_bh_enable(); + return NULL; } EXPORT_SYMBOL(bt_accept_dequeue);