* [RFC 2/4] android/hal: Update property used for start/stop services
2014-01-14 15:28 [RFC 1/4] android: Add sample init.bluetooth.rc file Szymon Janc
@ 2014-01-14 15:28 ` Szymon Janc
2014-01-14 15:28 ` [RFC 3/4] android/system-emulator: " Szymon Janc
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Szymon Janc @ 2014-01-14 15:28 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
---
android/hal-bluetooth.c | 4 ++--
android/hal-ipc.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index be45836..76560da 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -822,12 +822,12 @@ static int config_hci_snoop_log(uint8_t enable)
{
DBG("enable %u", enable);
- if (enable && property_set("ctl.start", SNOOP_SERVICE_NAME) < 0) {
+ if (enable && property_set("bluetooth.start", SNOOP_SERVICE_NAME) < 0) {
error("Failed to start service %s", SNOOP_SERVICE_NAME);
return BT_STATUS_FAIL;
}
- if (!enable && property_set("ctl.stop", SNOOP_SERVICE_NAME) < 0) {
+ if (!enable && property_set("bluetooth.stop", SNOOP_SERVICE_NAME) < 0) {
error("Failed to stop service %s", SNOOP_SERVICE_NAME);
return BT_STATUS_FAIL;
}
diff --git a/android/hal-ipc.c b/android/hal-ipc.c
index 97f1bcd..56165df 100644
--- a/android/hal-ipc.c
+++ b/android/hal-ipc.c
@@ -259,7 +259,7 @@ bool hal_ipc_init(void)
}
/* Start Android Bluetooth daemon service */
- if (property_set("ctl.start", SERVICE_NAME) < 0) {
+ if (property_set("bluetooth.start", SERVICE_NAME) < 0) {
error("Failed to start service %s", SERVICE_NAME);
close(sk);
return false;
--
1.8.3.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [RFC 3/4] android/system-emulator: Update property used for start/stop services
2014-01-14 15:28 [RFC 1/4] android: Add sample init.bluetooth.rc file Szymon Janc
2014-01-14 15:28 ` [RFC 2/4] android/hal: Update property used for start/stop services Szymon Janc
@ 2014-01-14 15:28 ` Szymon Janc
2014-01-14 15:28 ` [RFC 4/4] android: Update README with init.rc updates Szymon Janc
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Szymon Janc @ 2014-01-14 15:28 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
---
android/system-emulator.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/android/system-emulator.c b/android/system-emulator.c
index f1c6622..0966a02 100644
--- a/android/system-emulator.c
+++ b/android/system-emulator.c
@@ -139,17 +139,17 @@ static void system_socket_callback(int fd, uint32_t events, void *user_data)
printf("Received %s\n", buf);
- if (!strcmp(buf, "ctl.start=bluetoothd")) {
+ if (!strcmp(buf, "bluetooth.start=bluetoothd")) {
if (daemon_pid > 0)
return;
ctl_start();
- } else if (!strcmp(buf, "ctl.start=bluetoothd-snoop")) {
+ } else if (!strcmp(buf, "bluetooth.start=bluetoothd-snoop")) {
if (snoop_pid > 0)
return;
snoop_start();
- } else if (!strcmp(buf, "ctl.stop=bluetoothd-snoop")) {
+ } else if (!strcmp(buf, "bluetooth.stop=bluetoothd-snoop")) {
if (snoop_pid > 0)
snoop_stop();
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [RFC 4/4] android: Update README with init.rc updates
2014-01-14 15:28 [RFC 1/4] android: Add sample init.bluetooth.rc file Szymon Janc
2014-01-14 15:28 ` [RFC 2/4] android/hal: Update property used for start/stop services Szymon Janc
2014-01-14 15:28 ` [RFC 3/4] android/system-emulator: " Szymon Janc
@ 2014-01-14 15:28 ` Szymon Janc
2014-01-14 21:17 ` [RFC 1/4] android: Add sample init.bluetooth.rc file Andrzej Kaczmarek
2014-01-14 21:20 ` Marcel Holtmann
4 siblings, 0 replies; 11+ messages in thread
From: Szymon Janc @ 2014-01-14 15:28 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
---
android/README | 32 +++++++-------------------------
1 file changed, 7 insertions(+), 25 deletions(-)
diff --git a/android/README b/android/README
index 717ffa2..24ed703 100644
--- a/android/README
+++ b/android/README
@@ -36,31 +36,13 @@ Runtime requirements
====================
BlueZ HAL library requires 'bluetoothd' and 'bluetoothd-snoop' services to be
-available on Android system. This can be done by defining following services in
-init.rc file of targeted board:
-
-service bluetoothd /system/bin/logwrapper /system/bin/bluetoothd
- class main
- group bluetooth net_admin
- disabled
- oneshot
-
-service bluetoothd-snoop /system/bin/bluetoothd-snoop
- class main
- group bluetooth net_admin
- disabled
- oneshot
-
-It is required that bluetooth user could start and stop bluetoothd and
-bluetoothd-snoop services by setting 'ctl.start' or 'ctl.stop' property. This
-can be achieved by whitelisting bluetooth user and bluetoothd and
-bluetoothd-snoop services in init source code.
-
-Required Android init system modifications can be found at
-https://code.google.com/p/aosp-bluez.platform-system-core/
-
-Some configuration changes like setting permissions, starting hciattach
-services etc. are device specific. For convenience examples are provided at:
+available on Android system. Some permissions settings are also required.
+
+This can be done by importing init.bluetooth.rc file in init.rc file of targeted
+board:
+import init.bluetooth.rc
+
+For convenience examples are provided at:
https://code.google.com/p/aosp-bluez.device-lge-mako/ (Nexus 4)
https://code.google.com/p/aosp-bluez.device-asus-flo/ (Nexus 7 2013)
--
1.8.3.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC 1/4] android: Add sample init.bluetooth.rc file
2014-01-14 15:28 [RFC 1/4] android: Add sample init.bluetooth.rc file Szymon Janc
` (2 preceding siblings ...)
2014-01-14 15:28 ` [RFC 4/4] android: Update README with init.rc updates Szymon Janc
@ 2014-01-14 21:17 ` Andrzej Kaczmarek
2014-01-14 21:20 ` Marcel Holtmann
4 siblings, 0 replies; 11+ messages in thread
From: Andrzej Kaczmarek @ 2014-01-14 21:17 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
Hi Szymon,
On 14 January 2014 16:28, Szymon Janc <szymon.janc@tieto.com> wrote:
> This file is intended to be included from device init.rc.
> ---
> android/Android.mk | 16 +++++++++++++++-
> android/Makefile.am | 1 +
> android/init.bluetooth.rc | 27 +++++++++++++++++++++++++++
> 3 files changed, 43 insertions(+), 1 deletion(-)
> create mode 100644 android/init.bluetooth.rc
>
> diff --git a/android/Android.mk b/android/Android.mk
> index ae52ab4..f13e703 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -109,7 +109,7 @@ LOCAL_MODULE := bluetooth.default
> LOCAL_MODULE_PATH := $(TARGET_OUT_SHARED_LIBRARIES)/hw
> LOCAL_MODULE_TAGS := optional
> LOCAL_MODULE_CLASS := SHARED_LIBRARIES
> -LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop
> +LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop init.bluetooth.rc
>
> include $(BUILD_SHARED_LIBRARY)
>
> @@ -263,3 +263,17 @@ LOCAL_MODULE_TAGS := optional
> LOCAL_MODULE := bluetoothd-snoop
>
> include $(BUILD_EXECUTABLE)
> +
> +#
> +# init.bleutooth.rc
> +#
> +
> +include $(CLEAR_VARS)
> +
> +LOCAL_MODULE := init.bluetooth.rc
> +LOCAL_MODULE_CLASS := ETC
> +LOCAL_SRC_FILES := $(LOCAL_MODULE)
> +LOCAL_MODULE_TAGS := optional
> +LOCAL_MODULE_PATH := $(TARGET_ROOT_OUT)
> +
> +include $(BUILD_PREBUILT)
> diff --git a/android/Makefile.am b/android/Makefile.am
> index 7806f79..b205019 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -134,6 +134,7 @@ android_audio_a2dp_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
> endif
>
> EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
> + android/init.bluetooth.rc \
> android/pics-gap.txt android/pics-hid.txt \
> android/pics-pan.txt android/pics-did.txt \
> android/pics-opp.txt android/pics-pbap.txt \
> diff --git a/android/init.bluetooth.rc b/android/init.bluetooth.rc
> new file mode 100644
> index 0000000..e2be9e8
> --- /dev/null
> +++ b/android/init.bluetooth.rc
> @@ -0,0 +1,27 @@
> +chown bluetooth bluetooth /data/misc/bluetooth
> +
> +chown bluetooth bluetooth /dev/uhid
There should be some action trigger at the beginning, e.g. 'on boot'
should be fine.
BR,
Andrzej
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC 1/4] android: Add sample init.bluetooth.rc file
2014-01-14 15:28 [RFC 1/4] android: Add sample init.bluetooth.rc file Szymon Janc
` (3 preceding siblings ...)
2014-01-14 21:17 ` [RFC 1/4] android: Add sample init.bluetooth.rc file Andrzej Kaczmarek
@ 2014-01-14 21:20 ` Marcel Holtmann
2014-01-17 8:34 ` Szymon Janc
4 siblings, 1 reply; 11+ messages in thread
From: Marcel Holtmann @ 2014-01-14 21:20 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org development
Hi Szymon,
> This file is intended to be included from device init.rc.
> ---
> android/Android.mk | 16 +++++++++++++++-
> android/Makefile.am | 1 +
> android/init.bluetooth.rc | 27 +++++++++++++++++++++++++++
> 3 files changed, 43 insertions(+), 1 deletion(-)
> create mode 100644 android/init.bluetooth.rc
>
> diff --git a/android/Android.mk b/android/Android.mk
> index ae52ab4..f13e703 100644
> --- a/android/Android.mk
> +++ b/android/Android.mk
> @@ -109,7 +109,7 @@ LOCAL_MODULE := bluetooth.default
> LOCAL_MODULE_PATH := $(TARGET_OUT_SHARED_LIBRARIES)/hw
> LOCAL_MODULE_TAGS := optional
> LOCAL_MODULE_CLASS := SHARED_LIBRARIES
> -LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop
> +LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop init.bluetooth.rc
>
> include $(BUILD_SHARED_LIBRARY)
>
> @@ -263,3 +263,17 @@ LOCAL_MODULE_TAGS := optional
> LOCAL_MODULE := bluetoothd-snoop
>
> include $(BUILD_EXECUTABLE)
> +
> +#
> +# init.bleutooth.rc
> +#
> +
> +include $(CLEAR_VARS)
> +
> +LOCAL_MODULE := init.bluetooth.rc
> +LOCAL_MODULE_CLASS := ETC
> +LOCAL_SRC_FILES := $(LOCAL_MODULE)
> +LOCAL_MODULE_TAGS := optional
> +LOCAL_MODULE_PATH := $(TARGET_ROOT_OUT)
> +
> +include $(BUILD_PREBUILT)
> diff --git a/android/Makefile.am b/android/Makefile.am
> index 7806f79..b205019 100644
> --- a/android/Makefile.am
> +++ b/android/Makefile.am
> @@ -134,6 +134,7 @@ android_audio_a2dp_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
> endif
>
> EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
> + android/init.bluetooth.rc \
> android/pics-gap.txt android/pics-hid.txt \
> android/pics-pan.txt android/pics-did.txt \
> android/pics-opp.txt android/pics-pbap.txt \
> diff --git a/android/init.bluetooth.rc b/android/init.bluetooth.rc
> new file mode 100644
> index 0000000..e2be9e8
> --- /dev/null
> +++ b/android/init.bluetooth.rc
> @@ -0,0 +1,27 @@
> +chown bluetooth bluetooth /data/misc/bluetooth
> +
> +chown bluetooth bluetooth /dev/uhid
> +
> +on property:bluetooth.start=bluetoothd
> + start bluetoothd
> +
> +on property:bluetooth.stop=bluetoothd
> + stop bluetoothd
> +
> +on property:bluetooth.start=bluetoothd-snoop
> + start bluetoothd-snoop
> +
> +on property:bluetooth.stop=bluetoothd-snoop
> + stop bluetoothd-snoop
I think this is actually racy. The ctl.start and ctl.stop sound like they have special handling inside Android init. If you accidentally set bluetooth.start , then bad things happen.
So I would propose doing bluetooth.daemon=on/off and bluetooth.snoop=on/off
We just have to make sure that when bluetoothd or bluetoothd-snoop die unexpectedly that these properties get set back to off.
> +
> +service bluetoothd /system/bin/logwrapper /system/bin/bluetoothd
> + class main
> + group bluetooth net_admin
> + disabled
> + oneshot
> +
> +service bluetoothd-snoop /system/bin/bluetoothd-snoop
> + class main
> + group bluetooth net_admin
> + disabled
> + oneshot
Does net_admin include CAP_NET_RAW actually or are we using some sort of patched module for this?
Regards
Marcel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC 1/4] android: Add sample init.bluetooth.rc file
2014-01-14 21:20 ` Marcel Holtmann
@ 2014-01-17 8:34 ` Szymon Janc
2014-01-17 8:53 ` Marcel Holtmann
2014-01-17 8:56 ` Luiz Augusto von Dentz
0 siblings, 2 replies; 11+ messages in thread
From: Szymon Janc @ 2014-01-17 8:34 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
Hi Marcel,
On Tuesday 14 of January 2014 13:20:32 Marcel Holtmann wrote:
> Hi Szymon,
>
> > This file is intended to be included from device init.rc.
> > ---
> > android/Android.mk | 16 +++++++++++++++-
> > android/Makefile.am | 1 +
> > android/init.bluetooth.rc | 27 +++++++++++++++++++++++++++
> > 3 files changed, 43 insertions(+), 1 deletion(-)
> > create mode 100644 android/init.bluetooth.rc
> >
> > diff --git a/android/Android.mk b/android/Android.mk
> > index ae52ab4..f13e703 100644
> > --- a/android/Android.mk
> > +++ b/android/Android.mk
> > @@ -109,7 +109,7 @@ LOCAL_MODULE := bluetooth.default
> > LOCAL_MODULE_PATH := $(TARGET_OUT_SHARED_LIBRARIES)/hw
> > LOCAL_MODULE_TAGS := optional
> > LOCAL_MODULE_CLASS := SHARED_LIBRARIES
> > -LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop
> > +LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop init.bluetooth.rc
> >
> > include $(BUILD_SHARED_LIBRARY)
> >
> > @@ -263,3 +263,17 @@ LOCAL_MODULE_TAGS := optional
> > LOCAL_MODULE := bluetoothd-snoop
> >
> > include $(BUILD_EXECUTABLE)
> > +
> > +#
> > +# init.bleutooth.rc
> > +#
> > +
> > +include $(CLEAR_VARS)
> > +
> > +LOCAL_MODULE := init.bluetooth.rc
> > +LOCAL_MODULE_CLASS := ETC
> > +LOCAL_SRC_FILES := $(LOCAL_MODULE)
> > +LOCAL_MODULE_TAGS := optional
> > +LOCAL_MODULE_PATH := $(TARGET_ROOT_OUT)
> > +
> > +include $(BUILD_PREBUILT)
> > diff --git a/android/Makefile.am b/android/Makefile.am
> > index 7806f79..b205019 100644
> > --- a/android/Makefile.am
> > +++ b/android/Makefile.am
> > @@ -134,6 +134,7 @@ android_audio_a2dp_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
> > endif
> >
> > EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
> > + android/init.bluetooth.rc \
> > android/pics-gap.txt android/pics-hid.txt \
> > android/pics-pan.txt android/pics-did.txt \
> > android/pics-opp.txt android/pics-pbap.txt \
> > diff --git a/android/init.bluetooth.rc b/android/init.bluetooth.rc
> > new file mode 100644
> > index 0000000..e2be9e8
> > --- /dev/null
> > +++ b/android/init.bluetooth.rc
> > @@ -0,0 +1,27 @@
> > +chown bluetooth bluetooth /data/misc/bluetooth
> > +
> > +chown bluetooth bluetooth /dev/uhid
> > +
> > +on property:bluetooth.start=bluetoothd
> > + start bluetoothd
> > +
> > +on property:bluetooth.stop=bluetoothd
> > + stop bluetoothd
> > +
> > +on property:bluetooth.start=bluetoothd-snoop
> > + start bluetoothd-snoop
> > +
> > +on property:bluetooth.stop=bluetoothd-snoop
> > + stop bluetoothd-snoop
>
> I think this is actually racy. The ctl.start and ctl.stop sound like they have special handling inside Android init. If you accidentally set bluetooth.start , then bad things happen.
on property:foo=bar is also triggered if foo is already set to bar so I don't
see what could happen wrong here.
>
> So I would propose doing bluetooth.daemon=on/off and bluetooth.snoop=on/off
>
> We just have to make sure that when bluetoothd or bluetoothd-snoop die unexpectedly that these properties get set back to off.
I'm not sure if we are able to set properties back to proper state if service
exited in such case (that would trigger loop in init).
My idea was to use bluetooth.start/stop to start/stop (actually stop is needed
only for bluetoothd-snoop service) service by bluetooth user (sort of "sudo"
wrapper) but don't care about values in those props and if one want to use
props to track service lifetime then just use init.svc.bluetoothd{-snoop}
property.
>
> > +
> > +service bluetoothd /system/bin/logwrapper /system/bin/bluetoothd
> > + class main
> > + group bluetooth net_admin
> > + disabled
> > + oneshot
> > +
> > +service bluetoothd-snoop /system/bin/bluetoothd-snoop
> > + class main
> > + group bluetooth net_admin
> > + disabled
> > + oneshot
>
> Does net_admin include CAP_NET_RAW actually or are we using some sort of patched module for this?
I'm not sure, this was probably just copied from <4.2 android. Since we
start as root and drop caps I think this can be removed.
--
Best regards,
Szymon Janc
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC 1/4] android: Add sample init.bluetooth.rc file
2014-01-17 8:34 ` Szymon Janc
@ 2014-01-17 8:53 ` Marcel Holtmann
2014-01-17 9:06 ` Szymon Janc
2014-01-17 8:56 ` Luiz Augusto von Dentz
1 sibling, 1 reply; 11+ messages in thread
From: Marcel Holtmann @ 2014-01-17 8:53 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org development
Hi Szymon,
>>> This file is intended to be included from device init.rc.
>>> ---
>>> android/Android.mk | 16 +++++++++++++++-
>>> android/Makefile.am | 1 +
>>> android/init.bluetooth.rc | 27 +++++++++++++++++++++++++++
>>> 3 files changed, 43 insertions(+), 1 deletion(-)
>>> create mode 100644 android/init.bluetooth.rc
>>>
>>> diff --git a/android/Android.mk b/android/Android.mk
>>> index ae52ab4..f13e703 100644
>>> --- a/android/Android.mk
>>> +++ b/android/Android.mk
>>> @@ -109,7 +109,7 @@ LOCAL_MODULE := bluetooth.default
>>> LOCAL_MODULE_PATH := $(TARGET_OUT_SHARED_LIBRARIES)/hw
>>> LOCAL_MODULE_TAGS := optional
>>> LOCAL_MODULE_CLASS := SHARED_LIBRARIES
>>> -LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop
>>> +LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop init.bluetooth.rc
>>>
>>> include $(BUILD_SHARED_LIBRARY)
>>>
>>> @@ -263,3 +263,17 @@ LOCAL_MODULE_TAGS := optional
>>> LOCAL_MODULE := bluetoothd-snoop
>>>
>>> include $(BUILD_EXECUTABLE)
>>> +
>>> +#
>>> +# init.bleutooth.rc
>>> +#
>>> +
>>> +include $(CLEAR_VARS)
>>> +
>>> +LOCAL_MODULE := init.bluetooth.rc
>>> +LOCAL_MODULE_CLASS := ETC
>>> +LOCAL_SRC_FILES := $(LOCAL_MODULE)
>>> +LOCAL_MODULE_TAGS := optional
>>> +LOCAL_MODULE_PATH := $(TARGET_ROOT_OUT)
>>> +
>>> +include $(BUILD_PREBUILT)
>>> diff --git a/android/Makefile.am b/android/Makefile.am
>>> index 7806f79..b205019 100644
>>> --- a/android/Makefile.am
>>> +++ b/android/Makefile.am
>>> @@ -134,6 +134,7 @@ android_audio_a2dp_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
>>> endif
>>>
>>> EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
>>> + android/init.bluetooth.rc \
>>> android/pics-gap.txt android/pics-hid.txt \
>>> android/pics-pan.txt android/pics-did.txt \
>>> android/pics-opp.txt android/pics-pbap.txt \
>>> diff --git a/android/init.bluetooth.rc b/android/init.bluetooth.rc
>>> new file mode 100644
>>> index 0000000..e2be9e8
>>> --- /dev/null
>>> +++ b/android/init.bluetooth.rc
>>> @@ -0,0 +1,27 @@
>>> +chown bluetooth bluetooth /data/misc/bluetooth
>>> +
>>> +chown bluetooth bluetooth /dev/uhid
>>> +
>>> +on property:bluetooth.start=bluetoothd
>>> + start bluetoothd
>>> +
>>> +on property:bluetooth.stop=bluetoothd
>>> + stop bluetoothd
>>> +
>>> +on property:bluetooth.start=bluetoothd-snoop
>>> + start bluetoothd-snoop
>>> +
>>> +on property:bluetooth.stop=bluetoothd-snoop
>>> + stop bluetoothd-snoop
>>
>> I think this is actually racy. The ctl.start and ctl.stop sound like they have special handling inside Android init. If you accidentally set bluetooth.start , then bad things happen.
>
> on property:foo=bar is also triggered if foo is already set to bar so I don't
> see what could happen wrong here.
>
>>
>> So I would propose doing bluetooth.daemon=on/off and bluetooth.snoop=on/off
>>
>> We just have to make sure that when bluetoothd or bluetoothd-snoop die unexpectedly that these properties get set back to off.
>
> I'm not sure if we are able to set properties back to proper state if service
> exited in such case (that would trigger loop in init).
there is a service-exited-<name> type of action. I think we should look into using that one somehow.
> My idea was to use bluetooth.start/stop to start/stop (actually stop is needed
> only for bluetoothd-snoop service) service by bluetooth user (sort of "sudo"
> wrapper) but don't care about values in those props and if one want to use
> props to track service lifetime then just use init.svc.bluetoothd{-snoop}
> property.
Fair enough.
However then I would prefer if we do bluetooth.{start,stop}=daemon and bluetooth.{start,stop}=snoop to make this really tight into how our solution works.
>>> +service bluetoothd /system/bin/logwrapper /system/bin/bluetoothd
>>> + class main
>>> + group bluetooth net_admin
>>> + disabled
>>> + oneshot
>>> +
>>> +service bluetoothd-snoop /system/bin/bluetoothd-snoop
>>> + class main
>>> + group bluetooth net_admin
>>> + disabled
>>> + oneshot
>>
>> Does net_admin include CAP_NET_RAW actually or are we using some sort of patched module for this?
>
> I'm not sure, this was probably just copied from <4.2 android. Since we
> start as root and drop caps I think this can be removed.
The snoop process just needs CAP_NET_RAW and write access to the file location. I would prefer to really make sure it runs with as less right as possible.
Regards
Marcel
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC 1/4] android: Add sample init.bluetooth.rc file
2014-01-17 8:53 ` Marcel Holtmann
@ 2014-01-17 9:06 ` Szymon Janc
0 siblings, 0 replies; 11+ messages in thread
From: Szymon Janc @ 2014-01-17 9:06 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth@vger.kernel.org development
Hi Marcel,
On Friday 17 of January 2014 00:53:54 Marcel Holtmann wrote:
> Hi Szymon,
>
> >>> This file is intended to be included from device init.rc.
> >>> ---
> >>> android/Android.mk | 16 +++++++++++++++-
> >>> android/Makefile.am | 1 +
> >>> android/init.bluetooth.rc | 27 +++++++++++++++++++++++++++
> >>> 3 files changed, 43 insertions(+), 1 deletion(-)
> >>> create mode 100644 android/init.bluetooth.rc
> >>>
> >>> diff --git a/android/Android.mk b/android/Android.mk
> >>> index ae52ab4..f13e703 100644
> >>> --- a/android/Android.mk
> >>> +++ b/android/Android.mk
> >>> @@ -109,7 +109,7 @@ LOCAL_MODULE := bluetooth.default
> >>> LOCAL_MODULE_PATH := $(TARGET_OUT_SHARED_LIBRARIES)/hw
> >>> LOCAL_MODULE_TAGS := optional
> >>> LOCAL_MODULE_CLASS := SHARED_LIBRARIES
> >>> -LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop
> >>> +LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop init.bluetooth.rc
> >>>
> >>> include $(BUILD_SHARED_LIBRARY)
> >>>
> >>> @@ -263,3 +263,17 @@ LOCAL_MODULE_TAGS := optional
> >>> LOCAL_MODULE := bluetoothd-snoop
> >>>
> >>> include $(BUILD_EXECUTABLE)
> >>> +
> >>> +#
> >>> +# init.bleutooth.rc
> >>> +#
> >>> +
> >>> +include $(CLEAR_VARS)
> >>> +
> >>> +LOCAL_MODULE := init.bluetooth.rc
> >>> +LOCAL_MODULE_CLASS := ETC
> >>> +LOCAL_SRC_FILES := $(LOCAL_MODULE)
> >>> +LOCAL_MODULE_TAGS := optional
> >>> +LOCAL_MODULE_PATH := $(TARGET_ROOT_OUT)
> >>> +
> >>> +include $(BUILD_PREBUILT)
> >>> diff --git a/android/Makefile.am b/android/Makefile.am
> >>> index 7806f79..b205019 100644
> >>> --- a/android/Makefile.am
> >>> +++ b/android/Makefile.am
> >>> @@ -134,6 +134,7 @@ android_audio_a2dp_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
> >>> endif
> >>>
> >>> EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
> >>> + android/init.bluetooth.rc \
> >>> android/pics-gap.txt android/pics-hid.txt \
> >>> android/pics-pan.txt android/pics-did.txt \
> >>> android/pics-opp.txt android/pics-pbap.txt \
> >>> diff --git a/android/init.bluetooth.rc b/android/init.bluetooth.rc
> >>> new file mode 100644
> >>> index 0000000..e2be9e8
> >>> --- /dev/null
> >>> +++ b/android/init.bluetooth.rc
> >>> @@ -0,0 +1,27 @@
> >>> +chown bluetooth bluetooth /data/misc/bluetooth
> >>> +
> >>> +chown bluetooth bluetooth /dev/uhid
> >>> +
> >>> +on property:bluetooth.start=bluetoothd
> >>> + start bluetoothd
> >>> +
> >>> +on property:bluetooth.stop=bluetoothd
> >>> + stop bluetoothd
> >>> +
> >>> +on property:bluetooth.start=bluetoothd-snoop
> >>> + start bluetoothd-snoop
> >>> +
> >>> +on property:bluetooth.stop=bluetoothd-snoop
> >>> + stop bluetoothd-snoop
> >>
> >> I think this is actually racy. The ctl.start and ctl.stop sound like they have special handling inside Android init. If you accidentally set bluetooth.start , then bad things happen.
> >
> > on property:foo=bar is also triggered if foo is already set to bar so I don't
> > see what could happen wrong here.
> >
> >>
> >> So I would propose doing bluetooth.daemon=on/off and bluetooth.snoop=on/off
> >>
> >> We just have to make sure that when bluetoothd or bluetoothd-snoop die unexpectedly that these properties get set back to off.
> >
> > I'm not sure if we are able to set properties back to proper state if service
> > exited in such case (that would trigger loop in init).
>
> there is a service-exited-<name> type of action. I think we should look into using that one somehow.
I'll exercise that and send V2.
>
> > My idea was to use bluetooth.start/stop to start/stop (actually stop is needed
> > only for bluetoothd-snoop service) service by bluetooth user (sort of "sudo"
> > wrapper) but don't care about values in those props and if one want to use
> > props to track service lifetime then just use init.svc.bluetoothd{-snoop}
> > property.
>
> Fair enough.
>
> However then I would prefer if we do bluetooth.{start,stop}=daemon and bluetooth.{start,stop}=snoop to make this really tight into how our solution works.
OK.
>
> >>> +service bluetoothd /system/bin/logwrapper /system/bin/bluetoothd
> >>> + class main
> >>> + group bluetooth net_admin
> >>> + disabled
> >>> + oneshot
> >>> +
> >>> +service bluetoothd-snoop /system/bin/bluetoothd-snoop
> >>> + class main
> >>> + group bluetooth net_admin
> >>> + disabled
> >>> + oneshot
> >>
> >> Does net_admin include CAP_NET_RAW actually or are we using some sort of patched module for this?
> >
> > I'm not sure, this was probably just copied from <4.2 android. Since we
> > start as root and drop caps I think this can be removed.
>
> The snoop process just needs CAP_NET_RAW and write access to the file location. I would prefer to really make sure it runs with as less right as possible.
OK.
--
Best regards,
Szymon Janc
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC 1/4] android: Add sample init.bluetooth.rc file
2014-01-17 8:34 ` Szymon Janc
2014-01-17 8:53 ` Marcel Holtmann
@ 2014-01-17 8:56 ` Luiz Augusto von Dentz
2014-01-17 9:02 ` Szymon Janc
1 sibling, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2014-01-17 8:56 UTC (permalink / raw)
To: Szymon Janc; +Cc: Marcel Holtmann, linux-bluetooth@vger.kernel.org development
Hi Szymon,
On Fri, Jan 17, 2014 at 10:34 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Marcel,
>
> On Tuesday 14 of January 2014 13:20:32 Marcel Holtmann wrote:
>> Hi Szymon,
>>
>> > This file is intended to be included from device init.rc.
>> > ---
>> > android/Android.mk | 16 +++++++++++++++-
>> > android/Makefile.am | 1 +
>> > android/init.bluetooth.rc | 27 +++++++++++++++++++++++++++
>> > 3 files changed, 43 insertions(+), 1 deletion(-)
>> > create mode 100644 android/init.bluetooth.rc
>> >
>> > diff --git a/android/Android.mk b/android/Android.mk
>> > index ae52ab4..f13e703 100644
>> > --- a/android/Android.mk
>> > +++ b/android/Android.mk
>> > @@ -109,7 +109,7 @@ LOCAL_MODULE := bluetooth.default
>> > LOCAL_MODULE_PATH := $(TARGET_OUT_SHARED_LIBRARIES)/hw
>> > LOCAL_MODULE_TAGS := optional
>> > LOCAL_MODULE_CLASS := SHARED_LIBRARIES
>> > -LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop
>> > +LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop init.bluetooth.rc
>> >
>> > include $(BUILD_SHARED_LIBRARY)
>> >
>> > @@ -263,3 +263,17 @@ LOCAL_MODULE_TAGS := optional
>> > LOCAL_MODULE := bluetoothd-snoop
>> >
>> > include $(BUILD_EXECUTABLE)
>> > +
>> > +#
>> > +# init.bleutooth.rc
>> > +#
>> > +
>> > +include $(CLEAR_VARS)
>> > +
>> > +LOCAL_MODULE := init.bluetooth.rc
>> > +LOCAL_MODULE_CLASS := ETC
>> > +LOCAL_SRC_FILES := $(LOCAL_MODULE)
>> > +LOCAL_MODULE_TAGS := optional
>> > +LOCAL_MODULE_PATH := $(TARGET_ROOT_OUT)
>> > +
>> > +include $(BUILD_PREBUILT)
>> > diff --git a/android/Makefile.am b/android/Makefile.am
>> > index 7806f79..b205019 100644
>> > --- a/android/Makefile.am
>> > +++ b/android/Makefile.am
>> > @@ -134,6 +134,7 @@ android_audio_a2dp_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
>> > endif
>> >
>> > EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
>> > + android/init.bluetooth.rc \
>> > android/pics-gap.txt android/pics-hid.txt \
>> > android/pics-pan.txt android/pics-did.txt \
>> > android/pics-opp.txt android/pics-pbap.txt \
>> > diff --git a/android/init.bluetooth.rc b/android/init.bluetooth.rc
>> > new file mode 100644
>> > index 0000000..e2be9e8
>> > --- /dev/null
>> > +++ b/android/init.bluetooth.rc
>> > @@ -0,0 +1,27 @@
>> > +chown bluetooth bluetooth /data/misc/bluetooth
>> > +
>> > +chown bluetooth bluetooth /dev/uhid
>> > +
>> > +on property:bluetooth.start=bluetoothd
>> > + start bluetoothd
>> > +
>> > +on property:bluetooth.stop=bluetoothd
>> > + stop bluetoothd
>> > +
>> > +on property:bluetooth.start=bluetoothd-snoop
>> > + start bluetoothd-snoop
>> > +
>> > +on property:bluetooth.stop=bluetoothd-snoop
>> > + stop bluetoothd-snoop
>>
>> I think this is actually racy. The ctl.start and ctl.stop sound like they have special handling inside Android init. If you accidentally set bluetooth.start , then bad things happen.
>
> on property:foo=bar is also triggered if foo is already set to bar so I don't
> see what could happen wrong here.
>
>>
>> So I would propose doing bluetooth.daemon=on/off and bluetooth.snoop=on/off
>>
>> We just have to make sure that when bluetoothd or bluetoothd-snoop die unexpectedly that these properties get set back to off.
>
> I'm not sure if we are able to set properties back to proper state if service
> exited in such case (that would trigger loop in init).
>
> My idea was to use bluetooth.start/stop to start/stop (actually stop is needed
> only for bluetoothd-snoop service) service by bluetooth user (sort of "sudo"
> wrapper) but don't care about values in those props and if one want to use
> props to track service lifetime then just use init.svc.bluetoothd{-snoop}
> property.
>
>>
>> > +
>> > +service bluetoothd /system/bin/logwrapper /system/bin/bluetoothd
>> > + class main
>> > + group bluetooth net_admin
>> > + disabled
>> > + oneshot
>> > +
>> > +service bluetoothd-snoop /system/bin/bluetoothd-snoop
>> > + class main
>> > + group bluetooth net_admin
>> > + disabled
>> > + oneshot
>>
>> Does net_admin include CAP_NET_RAW actually or are we using some sort of patched module for this?
>
> I'm not sure, this was probably just copied from <4.2 android. Since we
> start as root and drop caps I think this can be removed.
Iirc CAP_NET_RAW is not required by mgmt nor any other part of
bluetoothd for Android make use of raw sockets so it was dropped at
some point.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC 1/4] android: Add sample init.bluetooth.rc file
2014-01-17 8:56 ` Luiz Augusto von Dentz
@ 2014-01-17 9:02 ` Szymon Janc
0 siblings, 0 replies; 11+ messages in thread
From: Szymon Janc @ 2014-01-17 9:02 UTC (permalink / raw)
To: Luiz Augusto von Dentz
Cc: Marcel Holtmann, linux-bluetooth@vger.kernel.org development
Hi Luiz,
On Friday 17 of January 2014 10:56:36 Luiz Augusto von Dentz wrote:
> Hi Szymon,
>
> On Fri, Jan 17, 2014 at 10:34 AM, Szymon Janc <szymon.janc@tieto.com> wrote:
> > Hi Marcel,
> >
> > On Tuesday 14 of January 2014 13:20:32 Marcel Holtmann wrote:
> >> Hi Szymon,
> >>
> >> > This file is intended to be included from device init.rc.
> >> > ---
> >> > android/Android.mk | 16 +++++++++++++++-
> >> > android/Makefile.am | 1 +
> >> > android/init.bluetooth.rc | 27 +++++++++++++++++++++++++++
> >> > 3 files changed, 43 insertions(+), 1 deletion(-)
> >> > create mode 100644 android/init.bluetooth.rc
> >> >
> >> > diff --git a/android/Android.mk b/android/Android.mk
> >> > index ae52ab4..f13e703 100644
> >> > --- a/android/Android.mk
> >> > +++ b/android/Android.mk
> >> > @@ -109,7 +109,7 @@ LOCAL_MODULE := bluetooth.default
> >> > LOCAL_MODULE_PATH := $(TARGET_OUT_SHARED_LIBRARIES)/hw
> >> > LOCAL_MODULE_TAGS := optional
> >> > LOCAL_MODULE_CLASS := SHARED_LIBRARIES
> >> > -LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop
> >> > +LOCAL_REQUIRED_MODULES := bluetoothd bluetoothd-snoop init.bluetooth.rc
> >> >
> >> > include $(BUILD_SHARED_LIBRARY)
> >> >
> >> > @@ -263,3 +263,17 @@ LOCAL_MODULE_TAGS := optional
> >> > LOCAL_MODULE := bluetoothd-snoop
> >> >
> >> > include $(BUILD_EXECUTABLE)
> >> > +
> >> > +#
> >> > +# init.bleutooth.rc
> >> > +#
> >> > +
> >> > +include $(CLEAR_VARS)
> >> > +
> >> > +LOCAL_MODULE := init.bluetooth.rc
> >> > +LOCAL_MODULE_CLASS := ETC
> >> > +LOCAL_SRC_FILES := $(LOCAL_MODULE)
> >> > +LOCAL_MODULE_TAGS := optional
> >> > +LOCAL_MODULE_PATH := $(TARGET_ROOT_OUT)
> >> > +
> >> > +include $(BUILD_PREBUILT)
> >> > diff --git a/android/Makefile.am b/android/Makefile.am
> >> > index 7806f79..b205019 100644
> >> > --- a/android/Makefile.am
> >> > +++ b/android/Makefile.am
> >> > @@ -134,6 +134,7 @@ android_audio_a2dp_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
> >> > endif
> >> >
> >> > EXTRA_DIST += android/Android.mk android/hal-ipc-api.txt android/README \
> >> > + android/init.bluetooth.rc \
> >> > android/pics-gap.txt android/pics-hid.txt \
> >> > android/pics-pan.txt android/pics-did.txt \
> >> > android/pics-opp.txt android/pics-pbap.txt \
> >> > diff --git a/android/init.bluetooth.rc b/android/init.bluetooth.rc
> >> > new file mode 100644
> >> > index 0000000..e2be9e8
> >> > --- /dev/null
> >> > +++ b/android/init.bluetooth.rc
> >> > @@ -0,0 +1,27 @@
> >> > +chown bluetooth bluetooth /data/misc/bluetooth
> >> > +
> >> > +chown bluetooth bluetooth /dev/uhid
> >> > +
> >> > +on property:bluetooth.start=bluetoothd
> >> > + start bluetoothd
> >> > +
> >> > +on property:bluetooth.stop=bluetoothd
> >> > + stop bluetoothd
> >> > +
> >> > +on property:bluetooth.start=bluetoothd-snoop
> >> > + start bluetoothd-snoop
> >> > +
> >> > +on property:bluetooth.stop=bluetoothd-snoop
> >> > + stop bluetoothd-snoop
> >>
> >> I think this is actually racy. The ctl.start and ctl.stop sound like they have special handling inside Android init. If you accidentally set bluetooth.start , then bad things happen.
> >
> > on property:foo=bar is also triggered if foo is already set to bar so I don't
> > see what could happen wrong here.
> >
> >>
> >> So I would propose doing bluetooth.daemon=on/off and bluetooth.snoop=on/off
> >>
> >> We just have to make sure that when bluetoothd or bluetoothd-snoop die unexpectedly that these properties get set back to off.
> >
> > I'm not sure if we are able to set properties back to proper state if service
> > exited in such case (that would trigger loop in init).
> >
> > My idea was to use bluetooth.start/stop to start/stop (actually stop is needed
> > only for bluetoothd-snoop service) service by bluetooth user (sort of "sudo"
> > wrapper) but don't care about values in those props and if one want to use
> > props to track service lifetime then just use init.svc.bluetoothd{-snoop}
> > property.
> >
> >>
> >> > +
> >> > +service bluetoothd /system/bin/logwrapper /system/bin/bluetoothd
> >> > + class main
> >> > + group bluetooth net_admin
> >> > + disabled
> >> > + oneshot
> >> > +
> >> > +service bluetoothd-snoop /system/bin/bluetoothd-snoop
> >> > + class main
> >> > + group bluetooth net_admin
> >> > + disabled
> >> > + oneshot
> >>
> >> Does net_admin include CAP_NET_RAW actually or are we using some sort of patched module for this?
> >
> > I'm not sure, this was probably just copied from <4.2 android. Since we
> > start as root and drop caps I think this can be removed.
>
> Iirc CAP_NET_RAW is not required by mgmt nor any other part of
> bluetoothd for Android make use of raw sockets so it was dropped at
> some point.
As per comment in main.c we need this for bnep ioctl.
--
Best regards,
Szymon Janc
^ permalink raw reply [flat|nested] 11+ messages in thread