From: Marcel Holtmann <marcel@holtmann.org>
To: Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com>
Cc: linux-bluetooth@vger.kernel.org
Subject: Re: [PATCHv1 1/2] Bluetooth: Check sk is not owned before freeing l2cap_conn
Date: Tue, 06 Jul 2010 12:21:02 -0300 [thread overview]
Message-ID: <1278429662.2789.52.camel@localhost.localdomain> (raw)
In-Reply-To: <1274883694-18120-2-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> Check that socket sk is not locked in user process before removing
> l2cap connection handler.
>
> krfcommd kernel thread may be preempted with l2cap tasklet which remove
> l2cap_conn structure. If krfcommd is in process of sending of RFCOMM reply
> (like "RFCOMM UA" reply to "RFCOMM DISC") then kernel crash happens.
>
> ...
> [ 694.175933] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [ 694.184936] pgd = c0004000
> [ 694.187683] [00000000] *pgd=00000000
> [ 694.191711] Internal error: Oops: 5 [#1] PREEMPT
> [ 694.196350] last sysfs file: /sys/devices/platform/hci_h4p/firmware/hci_h4p/loading
> [ 694.260375] CPU: 0 Not tainted (2.6.32.10 #1)
> [ 694.265106] PC is at l2cap_sock_sendmsg+0x43c/0x73c [l2cap]
> [ 694.270721] LR is at 0xd7017303
> ...
> [ 694.525085] Backtrace:
> [ 694.527587] [<bf266be0>] (l2cap_sock_sendmsg+0x0/0x73c [l2cap]) from [<c02f2cc8>] (sock_sendmsg+0xb8/0xd8)
> [ 694.537292] [<c02f2c10>] (sock_sendmsg+0x0/0xd8) from [<c02f3044>] (kernel_sendmsg+0x48/0x80)
> ...
>
> Modified version after comments of Gustavo F. Padovan <gustavo@padovan.org>
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
the patch seems to be fine, but I have some extra questions/concerns.
Who is now taking care of deleting the channel in this case? I think you
need to show that the code flow is still valid.
Also the question is how RFCOMM can send this UA or DISC with not
locking the socket. The comment on l2cap_chan_del clearly states that
the socket must be locked and inside L2CAP we do that. Is RFCOMM maybe
at fault here?
Regards
Marcel
next prev parent reply other threads:[~2010-07-06 15:21 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-26 14:21 [PATCHv1 0/2] Don't delete l2cap chan and sk when sk is owned by user Emeltchenko Andrei
2010-05-26 14:21 ` [PATCHv1 1/2] Bluetooth: Check sk is not owned before freeing l2cap_conn Emeltchenko Andrei
2010-07-06 15:21 ` Marcel Holtmann [this message]
2010-08-12 9:49 ` Andrei Emeltchenko
2010-08-12 11:27 ` Marcel Holtmann
2010-08-12 13:33 ` Andrei Emeltchenko
2010-05-26 14:21 ` [PATCHv1 2/2] Bluetooth: Prevent sk freeing in tasklet using refcount Emeltchenko Andrei
2010-06-07 14:11 ` [PATCHv1 0/2] Don't delete l2cap chan and sk when sk is owned by user Andrei Emeltchenko
2010-07-06 10:20 ` Andrei Emeltchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1278429662.2789.52.camel@localhost.localdomain \
--to=marcel@holtmann.org \
--cc=Andrei.Emeltchenko.news@gmail.com \
--cc=linux-bluetooth@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).