Closed Bug 1123676 Opened 9 years ago Closed 9 years ago

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

Categories

(Firefox OS Graveyard :: Bluetooth, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:2.2+, b2g-v2.2 fixed, b2g-v2.5 verified, b2g-master verified)

VERIFIED FIXED
2.2 S4 (23jan)
blocking-b2g 2.2+
Tracking Status
b2g-v2.2 --- fixed
b2g-v2.5 --- verified
b2g-master --- verified

People

(Reporter: brsun, Assigned: brsun)

Details

Attachments

(1 file)

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
"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.
blocking-b2g: --- → 2.2?
Triage: blocking
blocking-b2g: 2.2? → 2.2+
Assignee: nobody → brsun
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.
(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?
(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.
(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.
(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.
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).
Attachment #8552923 - Flags: review?(shuang)
Summary: [Nexus 5][Lollipop][Bluetooth] bluetoothd fails to switch bluetooth on/off. → [Nexus 5][Bluetooth] bluetoothd fails to switch bluetooth on/off.
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.
Attachment #8552923 - Flags: feedback?(tzimmermann)
There's also a group called 'net_bt_admin'. Does anyone know what this is used for?
(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 */
(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).
Do that mean that we also need AID_NET_BT for bluetoothd? The daemon creates SCO sockets after all.
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.
We tested without net_bt_admin, that won't impact OPP and SCO socket.
Summary: [Nexus 5][Bluetooth] bluetoothd fails to switch bluetooth on/off. → [Nexus 5][lollipop] [Bluetooth] bluetoothd fails to switch bluetooth on/off.
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:
Attachment #8552923 - Flags: approval-mozilla-b2g37?
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.
Thanks Shawn for helping me continue the request process as mentioned on comment 18. :)
(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.
Flags: needinfo?(brsun)
Master: https://github.com/mozilla-b2g/platform_system_bluetoothd/commit/d8040bc7850e326b297045ccbd51bfd7bf566a53
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 2.2 S4 (23jan)
Attachment #8552923 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
(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.
(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.
(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.
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
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmercado)
^ verified fixed on nexus 5 v2.5 as well.
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmercado)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: