lkml.org 
[lkml]   [2008]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 16/47] bluetooth: fix locking bug in the rfcomm socket cleanup handling
    On Sat, Jun 14, 2008 at 8:10 AM, Greg KH <gregkh@suse.de> wrote:
    > -stable review patch. If anyone has any objections, please let us know.
    >
    > ------------------
    > From: Arjan van de Ven <arjan@linux.intel.com>
    >
    > [ Upstream commit: 7dccf1f4e1696c79bff064c3770867cc53cbc71c ]

    Hi greg
    Please including following commit as well because it will cause another bug:
    http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=537d59af73d894750cff14f90fe2b6d77fbab15b

    >
    > in net/bluetooth/rfcomm/sock.c, rfcomm_sk_state_change() does the
    > following operation:
    >
    > if (parent && sock_flag(sk, SOCK_ZAPPED)) {
    > /* We have to drop DLC lock here, otherwise
    > * rfcomm_sock_destruct() will dead lock. */
    > rfcomm_dlc_unlock(d);
    > rfcomm_sock_kill(sk);
    > rfcomm_dlc_lock(d);
    > }
    > }
    >
    > which is fine, since rfcomm_sock_kill() will call sk_free() which will call
    > rfcomm_sock_destruct() which takes the rfcomm_dlc_lock()... so far so good.
    >
    > HOWEVER, this assumes that the rfcomm_sk_state_change() function always gets
    > called with the rfcomm_dlc_lock() taken. This is the case for all but one
    > case, and in that case where we don't have the lock, we do a double unlock
    > followed by an attempt to take the lock, which due to underflow isn't
    > going anywhere fast.
    >
    > This patch fixes this by moving the stragling case inside the lock, like
    > the other usages of the same call are doing in this code.
    >
    > This was found with the help of the www.kerneloops.org project, where this
    > deadlock was observed 51 times at this point in time:
    > http://www.kerneloops.org/search.php?search=rfcomm_sock_destruct
    >
    > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
    > Acked-by: Marcel Holtmann <marcel@holtmann.org>
    > Signed-off-by: David S. Miller <davem@davemloft.net>
    > Signed-off-by: Chris Wright <chrisw@sous-sol.org>
    > ---
    > net/bluetooth/rfcomm/core.c | 2 +-
    > 1 file changed, 1 insertion(+), 1 deletion(-)
    >
    > --- a/net/bluetooth/rfcomm/core.c
    > +++ b/net/bluetooth/rfcomm/core.c
    > @@ -423,8 +423,8 @@ static int __rfcomm_dlc_close(struct rfc
    >
    > rfcomm_dlc_lock(d);
    > d->state = BT_CLOSED;
    > - rfcomm_dlc_unlock(d);
    > d->state_change(d, err);
    > + rfcomm_dlc_unlock(d);
    >
    > skb_queue_purge(&d->tx_queue);
    > rfcomm_dlc_unlink(d);
    >
    > --
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > Please read the FAQ at http://www.tux.org/lkml/
    >



    --
    Regards
    dave


    \
     
     \ /
      Last update: 2008-06-14 04:55    [W:0.029 / U:60.492 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site