1123676 - [Nexus 5][lollipop] [Bluetooth] bluetoothd fails to switch bluetooth on/off.

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[Bruce Sun [:brsun]]

bluetoothd fails to enable/disable the bluetooth function on Nexus 5. Error messages from logcat [1] and dmesg [2] can be noticed.

Thanks :shawnjohnjr, we found one workaround is to use init.hammerhead.rc to modify the value of /sys/class/rfkill/rfkill0/state [3][4].

Probably we need to figure out how to solve this issue by configuring SELinux properly.

[1] E bt_upio : set_bluetooth_power : write(/sys/class/rfkill/rfkill0/state) failed: Operation not permitted (1)
[2] init: Warning!  Service bluetoothd needs a SELinux domain defined; please fix!
[3] https://code.google.com/p/aosp-bluez/source/browse/init.hammerhead.rc?repo=device-lge-ham#485
[4] https://code.google.com/p/aosp-bluez/source/browse/init.hammerhead.rc?repo=device-lge-ham#490

Comment 1

[Bruce Sun [:brsun]]

"Permissive" is displayed by executing "adb shell getenforce", which means SELinux is just permissive and not enforcing on nexus-5-l currently. It seems there is some basic permission issue needed to be fixed first.

Comment 2

[howie [:howie]]

Triage: blocking

Comment 3

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

static struct device_attribute rfkill_dev_attrs[] = {
...
        __ATTR(state, S_IRUGO|S_IWUSR, rfkill_state_show, rfkill_state_store),
...
};

So, the permission is "S_IRUGO|S_IWUSR", it seems to me that only root can write this attribute.

Comment 4

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Shawn Huang [:shawnjohnjr] from comment #3)
> static struct device_attribute rfkill_dev_attrs[] = {
> ...
>         __ATTR(state, S_IRUGO|S_IWUSR, rfkill_state_show,
> rfkill_state_store),
> ...
> };
> 
> So, the permission is "S_IRUGO|S_IWUSR", it seems to me that only root can
> write this attribute.
Oh no, I was wrong, that means user can read/write the file.
I checked again, maybe it's related to this piece of code?

static ssize_t rfkill_state_store(struct device *dev,
                                  struct device_attribute *attr,
                                  const char *buf, size_t count)
{
        struct rfkill *rfkill = to_rfkill(dev);
        unsigned long state;
        int err;

        if (!capable(CAP_NET_ADMIN))
                return -EPERM;


I guess bluetoothd does not have permission "CAP_NET_ADMIN?

Comment 5

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Shawn Huang [:shawnjohnjr] from comment #4)
> (In reply to Shawn Huang [:shawnjohnjr] from comment #3)
> > static struct device_attribute rfkill_dev_attrs[] = {
> > ...
> >         __ATTR(state, S_IRUGO|S_IWUSR, rfkill_state_show,
> > rfkill_state_store),
> > ...
> > };
> > 
> > So, the permission is "S_IRUGO|S_IWUSR", it seems to me that only root can
> > write this attribute.
> Oh no, I was wrong, that means user can read/write the file.
> I checked again, maybe it's related to this piece of code?
> 
> static ssize_t rfkill_state_store(struct device *dev,
>                                   struct device_attribute *attr,
>                                   const char *buf, size_t count)
> {
>         struct rfkill *rfkill = to_rfkill(dev);
>         unsigned long state;
>         int err;
> 
>         if (!capable(CAP_NET_ADMIN))
>                 return -EPERM;
> 
> 
> I guess bluetoothd does not have permission "CAP_NET_ADMIN?

Please add 'group net_admin' in init.bluetooth.rc for bluetooth service.

Comment 6

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Bruce Sun [:brsun] from comment #1)
> "Permissive" is displayed by executing "adb shell getenforce", which means
> SELinux is just permissive and not enforcing on nexus-5-l currently. It
> seems there is some basic permission issue needed to be fixed first.

Yes. It looks like SELinux is been enforced. So it's related to net_admin permission.

Comment 7

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Shawn Huang [:shawnjohnjr] from comment #6)
> (In reply to Bruce Sun [:brsun] from comment #1)
> > "Permissive" is displayed by executing "adb shell getenforce", which means
> > SELinux is just permissive and not enforcing on nexus-5-l currently. It
> > seems there is some basic permission issue needed to be fixed first.
> 
> Yes. It looks like SELinux is been enforced. So it's related to net_admin
> permission.

typo: is "not" been enforced.

Comment 8

[Bruce Sun [:brsun]]

Attachment: https://github.com/mozilla-b2g/platform_system_bluetoothd/pull/11

Give bluetoothd CAP_NET_ADMIN capability to switch on/off the Bluetooth function. If we don't give this permission to bluetoothd on Nexus-5 device, it will fail to switch on/off the Bluetooth function due to failing to write /sys/class/rfkill/rfkill0/state with EPERM (Operation not permitted).

Comment 9

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8552923 [details] [review]
https://github.com/mozilla-b2g/platform_system_bluetoothd/pull/11

I'm ok with this change, but I still prefer to consult with tzimmermann, see if he has any other concern.
I think we need to add group net_admin for bluetoothd, eventually we also need to add PAN profile that need to use net related code, and Android Bluetooth application also has this permission, so it sounds like necessary permission for me.

Comment 10

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 8552923 [details] [review]
https://github.com/mozilla-b2g/platform_system_bluetoothd/pull/11

Looks good.

Comment 11

[Thomas Zimmermann [:tzimmermann] [:tdz]]

There's also a group called 'net_bt_admin'. Does anyone know what this is used for?

Comment 12

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #11)
> There's also a group called 'net_bt_admin'. Does anyone know what this is
> used for?

http://androidxref.com/5.0.0_r2/xref/system/core/include/private/android_filesystem_config.h#88
88#define AID_NET_BT_ADMIN  3001  /* bluetooth: create any socket */
89#define AID_NET_BT        3002  /* bluetooth: create sco, rfcomm or l2cap sockets */

Comment 13

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Shawn Huang [:shawnjohnjr] from comment #12)
> (In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #11)
> > There's also a group called 'net_bt_admin'. Does anyone know what this is
> > used for?
> 
> http://androidxref.com/5.0.0_r2/xref/system/core/include/private/
> android_filesystem_config.h#88
> 88#define AID_NET_BT_ADMIN  3001  /* bluetooth: create any socket */
> 89#define AID_NET_BT        3002  /* bluetooth: create sco, rfcomm or l2cap
> sockets */

https://www.codeaurora.org/cgit/quic/le//kernel/msm/patch/?id=55e91c43eb2867b4f8110cd5a5290f7b5dc99063
Only root or processes with gid AID_NET_BT or AID_NET_BT_ADMIN
can create RFCOMM, SCO and L2CAP sockets.

Only root or process with gid AID_NET_BT_ADMIN can open other
AF_BLUETOOTH sockets (for example HCI).

Comment 14

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Do that mean that we also need AID_NET_BT for bluetoothd? The daemon creates SCO sockets after all.

Comment 15

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Hmm... since bluedroid goes through userspace socket, unlike BlueZ bluetoothd (under kernel/net/bluetooth), which depends on kernel bluetooth socket. Do we still need to add net_bt_admin?
Nick added extra permission check in net/bluetooth/af_bluetooth.c (while Android still uses BlueZ as back-end) but now bluedroid doesn't leverage that part of code. 

Bruce, let's double confirm if there is something wrong without net_bt_admin permission.

Comment 16

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

We tested without net_bt_admin, that won't impact OPP and SCO socket.

Comment 17

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Comment on attachment 8552923 [details] [review]
https://github.com/mozilla-b2g/platform_system_bluetoothd/pull/11

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: We won't be able to use the Bluetooth daemon on v2.2
Testing completed: 
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

Comment 18

[Bruce Sun [:brsun]]

I roughly test enable/disable, pair/unpair, file transfer, music playback, incoming/outgoing call; basically all functions work properly except:
 - only one file can be transferred; the device should disable/enable Bluetooth function again before performing another file transfer.
 - the ringing of outgoing call comes out from the phone at the beginning sometimes; the duration before the ringing comes out from PLT_Legend is a little bit long in this case.

These issues seem not so related to this bug, and these issues should be handled on other bugs instead of this one, so I will continue to request for check-in and uplift-approval.

Comment 19

[Bruce Sun [:brsun]]

Thanks Shawn for helping me continue the request process as mentioned on comment 18. :)

Comment 20

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Bruce Sun [:brsun] from comment #18)
> I roughly test enable/disable, pair/unpair, file transfer, music playback,
> incoming/outgoing call; basically all functions work properly except:
>  - only one file can be transferred; the device should disable/enable
> Bluetooth function again before performing another file transfer.

Can you clarify it's one lollipop specific bug or it can be reproduced on flame-kk?
>  - the ringing of outgoing call comes out from the phone at the beginning
> sometimes; the duration before the ringing comes out from PLT_Legend is a
> little bit long in this case.
> 
> These issues seem not so related to this bug, and these issues should be
> handled on other bugs instead of this one, so I will continue to request for
> check-in and uplift-approval.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Master: https://github.com/mozilla-b2g/platform_system_bluetoothd/commit/d8040bc7850e326b297045ccbd51bfd7bf566a53

Comment 22

[Bruce Sun [:brsun]]

(In reply to Shawn Huang [:shawnjohnjr] from comment #20)
> (In reply to Bruce Sun [:brsun] from comment #18)
> > I roughly test enable/disable, pair/unpair, file transfer, music playback,
> > incoming/outgoing call; basically all functions work properly except:
> >  - only one file can be transferred; the device should disable/enable
> > Bluetooth function again before performing another file transfer.
> 
> Can you clarify it's one lollipop specific bug or it can be reproduced on
> flame-kk?

flame-kk also suffers from this issue. No further file transfer operation can be achieved after receiving one file. Need to disable and enable Bluetooth from the setting app to make it work again.

Comment 23

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

(In reply to Bruce Sun [:brsun] from comment #22)
> (In reply to Shawn Huang [:shawnjohnjr] from comment #20)
> > (In reply to Bruce Sun [:brsun] from comment #18)
> > > I roughly test enable/disable, pair/unpair, file transfer, music playback,
> > > incoming/outgoing call; basically all functions work properly except:
> > >  - only one file can be transferred; the device should disable/enable
> > > Bluetooth function again before performing another file transfer.
> > 
> > Can you clarify it's one lollipop specific bug or it can be reproduced on
> > flame-kk?
> 
> flame-kk also suffers from this issue. No further file transfer operation
> can be achieved after receiving one file. Need to disable and enable
> Bluetooth from the setting app to make it work again.
OK, let's open new bug to track this. This seems to be the high priority to me.

Comment 24

[Bruce Sun [:brsun]]

(In reply to Shawn Huang [:shawnjohnjr] from comment #23)
> OK, let's open new bug to track this. This seems to be the high priority to
> me.

bug 1124983 has been created for tracking this issue.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

v2.2: https://github.com/mozilla-b2g/platform_system_bluetoothd/commit/5ca14c6eccb2079749ff2a4eef56c311bd7b698f

Comment 26

[Pi Wei Cheng [:piwei] (inactive)]

This issue is verified fixed on Nexus 5 central. I was able to turn on bluetooth, pair with another device (Aries in this case), send/receive an image. I didn't see the error mentioned in logcat either.

Note that we're unable to verify this on v2.2 because we don't have builds for it.

Verified fixed on:
Build ID               20151124120300
Gaia Revision          e63c07af8cc636b9ffac2c20f9b8ad7e7fe2b977
Gaia Date              2015-11-24 03:47:05
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/45273bbed8efaface6f5ec56d984cb9faf4fbb6a
Gecko Version          45.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.worker.20151124.111855
Firmware Date          Tue Nov 24 11:19:13 UTC 2015
Bootloader             HHZ12d

Build ID               20151124191011
Gaia Revision          7726bfd621d709fa2d179704a44aebc1c309c296
Gaia Date              2015-11-24 15:12:43
Gecko Revision         http://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/124375497b14260515e8d2af9c8507a9995b7a47
Gecko Version          44.0a2
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.worker.20151124.181638
Firmware Date          Tue Nov 24 18:16:53 UTC 2015
Bootloader             HHZ12d

Comment 27

[Pi Wei Cheng [:piwei] (inactive)]

^ verified fixed on nexus 5 v2.5 as well.