Re: [PATCH] bluetoothd: add option to automatically power on the first adapter found

[Alexander Holler <holler@ahsoftware.de>]

Am 27.04.2015 um 20:53 schrieb Marcel Holtmann:
> Hi Alexander,
>
>>>>>> Your race power on vs keys and known devices programmed into the
>>>>>> kernel. It needs to be done in the right order.
>>>>>
>>>>> Hmm, I still seem to function, no sign of races, besides that I'm
>>>>> happily working without a Linux kernel.
>>>>>
>>>>> But in regard to the patch. Maybe. No idea. It works for me (tm) and is
>>>>> faster and more reliable here than other stuff.
>>>>
>>>> To pick up that thread again, if you want that I remove a race in that patch I don't see by looking at it, you have to be a bit more verbose about the race.
>>>
>>> it is pretty simple, you want to configure the list of known devices and its keys before you turn on your device for general operation. If you don't do that, you have a race condition.
>>>
>>>> For me it currently reads like you're more talking about an existing problem in the kernel. If the kernel doesn't like it that a bt-device will be turned on before something like keys and known devices have been setup, then the kernel should throw an error back to userspace.
>>>
>>> The kernel is not enforcing policy. We can not require to have keys loaded first. There might be no keys or you want to use the Bluetooth subsystem for something else. The kernel mgmt interface provides a flexible way into the general procedures that Bluetooth requires. With bluetoothd we enforce profiles and other behaviors to be consistent. That is not the kernel's job.
>>>
>>> And even if we wanted to restrict this heavily, we can not easily do that without potentially breaking existing userspace.
>>>
>>>> But that's just an assumption from me, I haven't read through all the source in bluetoothd or the kernel, but instead just turned on a knob like it would be turned on by issuing "power on" in bluetoothctl.
>>>
>>> With bluetoothctl you talk over the D-Bus interface to bluetoothd. So when you turn power on there, then it is ensured that everything happens in the right order. That is what bluetoothd enforces. It is not the kernels job to do that.
>>
>> For me this reads like kernel <-> userspace API is very fragile.
>>
>> Also I haven't looked at the source for that, I would assume that it should be no problem to submit an empty list of keys and devices to the kernel. So a probably stable design of that part of the API would be:
>>
>> - Require that userspace sets up various stuff
>> - Require that userspace sends a list of keys (maybe empty)
>> - Require that userspace sends a list of devices (maybe empty)
>> - Turn on devices which are requested to be turned on and start operation.
>>
>> I'm aware that this might involve problems with older userspace, but just trying to discuss a problem away (e.g. by murdering the messenger) is usually a bad solution.
>>
>> Right now it looks like userspace can confuse the kernel quiet a lot which it should not be able to do.
>>
>> For example you are calling it a race if my patch for bluetoothd is used. Which bad things might happen? Do I have to fear my machine blows up and will erase my storage? Or, as we are talking about wireless stuff, do I have to fear that security is absent and everyone might be able to connect to my machine?
>
> the kernel is different from bluetoothd. You are changing code in bluetoothd and thus lets do it correctly. I really have no idea what you are arguing here for.

You are saying that it's a race to power on the dongle before keys and 
devices are setup. And I'm assuming the keys and/or devices are managed, 
at least in part, by the kernel.

If not, then it's a of course just an userspace only problem. Also I'm 
still interested what might go wrong, it's unlikely I will get an answer 
as I seem to have to look up that myself.

>> Sorry that I don't understand your reasoning, but if I would translate that, it would mean the same like if it's a race to call unlink(2) on a non-existing file. Of course, it would be much easier for the kernel to not have to handle all the possible error conditions, but that's the dream of any sw-dev and will never become reality. Handling all the possible error conditions just has to be part of almost any sw, regardless if it's a kernel or not.
>>
>> Anyway, to come back to the problem of easy and reliable turning on bt-devices at startup, I will look if I find the place in bluetoothd where the keys and devices are submitted to the kernel in order to put the requested power-on after that. But as I'm still happy with my current solution, I don't make any promises on a timeline. ;)
>
> If you want the patch to get upstream, I clearly gave you instructions on what has to change to make it upstream acceptable. The work here is on you.

Sorry, but as said several times before, I don't care much for any Linux 
upstream anymore. It's just too cumbersome to discuss about maintainer 
specific styles and other maintainer specific preferences and I've 
already wasted much too much time in making patches ready for submission 
which never ended up anywhere (but for which I still receive 
mails/requests from other users). I now just care for other users and 
post a patch if I really think it might help several people or if I'm 
annoyed because I've read too much complaints about a problem. So no 
work is on me.

Regards,

Alexander Holler