On Sat, 2007-06-30 at 16:44 +0100, Ranulf Doswell wrote:
> This is my first patch, so apologies if I've got the protocol wrong!
> I'm not sure if this is the best place to post this patch or even if
> I've done everything correctly (I assume I simply add a signed-off-by
> in my name).
Hi Ranulf,
This is one of the places you should send this patch, the other is the
bluetooth list, which I've CC'ed. The main issue is whether your patch
is safe in the general case, or if it's just a PS3 issue.
You do "simply" add a signed-off-by line, as long as you understand what
it means - read Documentation/SubmittingPatches for the full version.
Basically you're saying that you wrote the code and you're allowed to
release it under the GPL v2.
> I came across this bug when getting sixaxis controllers to work under
> Linux on the PS3, although it actually seems to be a generic bluetooth
> problem under Linux.
>
> Basically, the problem is that unless the bluetooth connection is
> killed prior to reboot, the PS3 controllers continue sending out data
> which the kernel correctly identifies as not associated with an active
> connection. However, it simply logs the error and continues, so the
> syslog rapidly fills up. It addition, because the device it will not
> attempt to re-initiate a new bluetooth connection, so the only current
> solution is to halt the linux box until the controller times out.
>
> This one line addition sends the device a reset which causes it to
> switch off.
Cool, do the sixaxis controllers work with this patch applied?
> Signed-off-by: Ranulf Doswell <ralf at ranulf.net>
>
> =====================================
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 8943c93..77b7eca 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1340,6 +1340,12 @@ static inline void hci_acldata_packet(struct hci_dev *hdev, struct sk_buff *skb)
> } else {
> BT_ERR("%s ACL packet for unknown connection handle %d",
> hdev->name, handle);
> +
> + // Force the device to reset in the hope that it will then leave us alone.
> + // This is needed at least for PS3 sixaxis as the controllers will keep
> + // sending data until they are forcibly terminated. A reboot doesn't provide
> + // enough time for the controller's bluetooth stack to time out.
> + hci_reset_req(hdev, 0);
You should use C-style comments, eg. /* .. */, see
Documentation/CodingStyle for more info.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
Hi Michael,
Thanks for the info, specifically the pointers to the coding style and patch
submission rules, which I neglected to look for in my excitement to create
my first kernel patch!
I'll repost the patch in a few minutes as I've made the comments generic
rather than sounding like a PS3-specific ptroblem.
This is one of the places you should send this patch, the other is the
> bluetooth list, which I've CC'ed. The main issue is whether your patch
> is safe in the general case, or if it's just a PS3 issue.
I haven't got a great deal of BT equipment, so it's hard to test, but my
personal opinion is that any device which sends unsolicited data should
survive having a reset sent to it, as it's not correct for it to be sending
ACL data without a valid connection established. Arguably, the same patch
should be applied when unsolicited SCO data is received, although I have no
way of testing this.
Without the patch, it doesn't make sense to syslog the invalid data as it
opens up the system to an easily exploitable DOS simply by getting within
100m of a machine and flooding it with packets from a handheld device. This
particular device was sending about 100 packets per second.
Cool, do the sixaxis controllers work with this patch applied?
Pascal's hidd patch (patch-hidd-2.24-pabr2) works well to enable the
controllers to be detected. It looks like the most important part of the
patch has already been accepted into the bluez-utils tree, although the MTU
increase part of the patch hasn't been included, so PS3 controllers still
don't work out of the box as they require a slightly larger MTU.
All this patch does is reset the controllers when the machine is rebooted as
they otherwise continue to send packets until the machine is powered down.
It seems that their timeout is about a minute and that linux's stack running
provides sufficient ACK that data is still being received even if the kernel
itself doesn't know anything about the connection.
An additional patch to send every bluetooth device a reset as the kernel is
shutting down would probably also be a good idea, although it is possible to
add "hidd --killall" to a shutdown script to achieve the same result.
Cheers,
Ralf.