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
+
+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
--
1.8.3.2
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
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 <[email protected]> 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
Hi Szymon,
On Fri, Jan 17, 2014 at 10:34 AM, Szymon Janc <[email protected]> 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
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
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
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
Hi Szymon,
On 14 January 2014 16:28, Szymon Janc <[email protected]> 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
---
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
---
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
---
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