linux-kernel - Re: [patch 16/47] bluetooth: fix locking bug in the rfcomm socket cleanup handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a8e1da0806131950i4eb0ea48q5cc34ea07cfd523a@mail.gmail.com>
Date:	Sat, 14 Jun 2008 10:50:51 +0800
From:	"Dave Young" <hidave.darkstar@...il.com>
To:	"Greg KH" <gregkh@...e.de>
Cc:	linux-kernel@...r.kernel.org, stable@...nel.org,
	"Justin Forbes" <jmforbes@...uxtx.org>,
	"Zwane Mwaikambo" <zwane@....linux.org.uk>,
	"Theodore Ts'o" <tytso@....edu>,
	"Randy Dunlap" <rdunlap@...otime.net>,
	"Dave Jones" <davej@...hat.com>,
	"Chuck Wolber" <chuckw@...ntumlinux.com>,
	"Chris Wedgwood" <reviews@...cw.f00f.org>,
	"Michael Krufky" <mkrufky@...uxtv.org>,
	"Chuck Ebbert" <cebbert@...hat.com>,
	"Domenico Andreoli" <cavokz@...il.com>, "Willy Tarreau" <w@....eu>,
	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	alan@...rguk.ukuu.org.uk,
	"Arjan van de Ven" <arjan@...ux.intel.com>,
	"Marcel Holtmann" <marcel@...tmann.org>,
	"David S. Miller" <davem@...emloft.net>,
	"Chris Wright" <chrisw@...s-sol.org>
Subject: Re: [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@...e.de> wrote:
> -stable review patch.  If anyone has any objections, please let us know.
>
> ------------------
> From: Arjan van de Ven <arjan@...ux.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@...ux.intel.com>
> Acked-by: Marcel Holtmann <marcel@...tmann.org>
> Signed-off-by: David S. Miller <davem@...emloft.net>
> Signed-off-by: Chris Wright <chrisw@...s-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@...r.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/