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

[Alexander Holler <holler@ahsoftware.de>]

Am 27.04.2015 um 06:40 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?

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. ;)

Regards,

Alexander Holler