All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [RFC PATCH] package/libcamera: remove rpath and strip debug symbols before signing IPA libs
@ 2022-05-05 12:01 Quentin Schulz
  2022-05-05 20:57 ` Arnout Vandecappelle
  0 siblings, 1 reply; 6+ messages in thread
From: Quentin Schulz @ 2022-05-05 12:01 UTC (permalink / raw)
  To: buildroot
  Cc: Quentin Schulz, Quentin Schulz, Kieran Bingham, Laurent Pinchart

From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

Open-Source IPA shlibs need to be signed in order to be runnable within
the same process, otherwise they are deemed Closed-Source and run in
another process and communicate over IPC.
Buildroot strips debug symbols and sanitizes RPATH in a post build
process. We need to do the same before signing the IPA shlibs otherwise
the signature won't match the shlib on the rootfs.

meson gets rid of rpath while installing so we don't need to do it
manually.
However the signing process is also part of the meson install target, so
we have a chicken and the egg problem. Let's install the libs in the
target directory (and do a useless signing) to get rid of rpath, then
strip debug symbols the same way Buildroot does in post build step, then
re-sign shlibs directly in TARGET_DIR with signing script from
libcamera.

Cc: Quentin Schulz <foss+buildroot@0leil.net>
Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
 package/libcamera/libcamera.mk | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
index 77381ab3ca..d1303a2ff5 100644
--- a/package/libcamera/libcamera.mk
+++ b/package/libcamera/libcamera.mk
@@ -104,4 +104,25 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
 LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
 endif
 
+# Open-Source IPA shlibs need to be signed in order to be runnable within the
+# same process, otherwise they are deemed Closed-Source and run in another
+# process and communicate over IPC.
+# Buildroot strips debug symbols and sanitizes RPATH in a post build process. We
+# need to do the same before signing the IPA shlibs otherwise the signature
+# won't match the shlib on the rootfs.
+#
+# meson gets rid of rpath while installing so we don't need to do it manually.
+# However the signing process is also part of the meson install target, so we
+# have a chicken and the egg problem. Let's install the libs in the target
+# directory (and do a useless signing) to get rid of rpath, then strip debug
+# symbols the same way Buildroot does in post build step, then re-sign shlibs
+# directly in TARGET_DIR with signing script from libcamera.
+define LIBCAMERA_INSTALL_TARGET_CMDS
+	$(TARGET_MAKE_ENV) $(LIBCAMERA_NINJA_ENV) DESTDIR=$(TARGET_DIR) \
+		$(NINJA) $(NINJA_OPTS) -C $(LIBCAMERA_SRCDIR)/build install
+	find $(TARGET_DIR) -type f -name "ipa_*.so" -print0 | xargs -0 $(STRIPCMD) 2>/dev/null || true
+	MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib/libcamera/ $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
+	MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib64/libcamera/ $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
+endef
+
 $(eval $(meson-package))
-- 
2.35.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [RFC PATCH] package/libcamera: remove rpath and strip debug symbols before signing IPA libs
  2022-05-05 12:01 [Buildroot] [RFC PATCH] package/libcamera: remove rpath and strip debug symbols before signing IPA libs Quentin Schulz
@ 2022-05-05 20:57 ` Arnout Vandecappelle
  2022-05-06  8:56   ` Quentin Schulz
  0 siblings, 1 reply; 6+ messages in thread
From: Arnout Vandecappelle @ 2022-05-05 20:57 UTC (permalink / raw)
  To: Quentin Schulz, buildroot
  Cc: Quentin Schulz, Kieran Bingham, Laurent Pinchart

  Hi Quentin,

On 05/05/2022 14:01, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> Open-Source IPA shlibs need to be signed in order to be runnable within
> the same process, otherwise they are deemed Closed-Source and run in
> another process and communicate over IPC.
> Buildroot strips debug symbols and sanitizes RPATH in a post build
> process. We need to do the same before signing the IPA shlibs otherwise
> the signature won't match the shlib on the rootfs.
> 
> meson gets rid of rpath while installing so we don't need to do it
> manually.
> However the signing process is also part of the meson install target, so
> we have a chicken and the egg problem. Let's install the libs in the
> target directory (and do a useless signing) to get rid of rpath, then
> strip debug symbols the same way Buildroot does in post build step, then
> re-sign shlibs directly in TARGET_DIR with signing script from
> libcamera.
> 
> Cc: Quentin Schulz <foss+buildroot@0leil.net>
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
>   package/libcamera/libcamera.mk | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/package/libcamera/libcamera.mk b/package/libcamera/libcamera.mk
> index 77381ab3ca..d1303a2ff5 100644
> --- a/package/libcamera/libcamera.mk
> +++ b/package/libcamera/libcamera.mk
> @@ -104,4 +104,25 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
>   LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
>   endif
>   
> +# Open-Source IPA shlibs need to be signed in order to be runnable within the
> +# same process, otherwise they are deemed Closed-Source and run in another
> +# process and communicate over IPC.
> +# Buildroot strips debug symbols and sanitizes RPATH in a post build process. We
> +# need to do the same before signing the IPA shlibs otherwise the signature
> +# won't match the shlib on the rootfs.
> +#
> +# meson gets rid of rpath while installing so we don't need to do it manually.
> +# However the signing process is also part of the meson install target, so we
> +# have a chicken and the egg problem. Let's install the libs in the target
> +# directory (and do a useless signing) to get rid of rpath, then strip debug
> +# symbols the same way Buildroot does in post build step, then re-sign shlibs
> +# directly in TARGET_DIR with signing script from libcamera.
> +define LIBCAMERA_INSTALL_TARGET_CMDS
> +	$(TARGET_MAKE_ENV) $(LIBCAMERA_NINJA_ENV) DESTDIR=$(TARGET_DIR) \
> +		$(NINJA) $(NINJA_OPTS) -C $(LIBCAMERA_SRCDIR)/build install
> +	find $(TARGET_DIR) -type f -name "ipa_*.so" -print0 | xargs -0 $(STRIPCMD) 2>/dev/null || true

  You should add --no-run-if-empty to xargs, in case there is no library at all 
(unless you're very sure that doesn't happen). Also, why is the || true needed?

> +	MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib/libcamera/ $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
> +	MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib64/libcamera/ $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))

  Instead of overriding INSTALL_TARGET_CMDS, you could also do the additional 
steps in POST_INSTALL_TARGET_HOOKS.

  However, I think it may be simpler to do the stripping in a POST_BUILD_HOOK. 
That way, you only need to do the strip; the signing is done in the install 
step, so if we make sure the .so is stripped by then, all should be well, right?

  Finally, there is one little caveat: if the libraries are installed in 
BR2_STRIP_EXCLUDE_DIRS (i.e. if BR2_STRIP_EXCLUDE_DIRS is set to /usr/lib), or 
if they match BR2_STRIP_EXCLUDE_FILES (e.g. if it is set to "ipa_*.so"), then 
the libs would be stripped while they shouldn't be. However, I don't think 
that's a big deal.

  Regards,
  Arnout


> +endef
> +
>   $(eval $(meson-package))
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [RFC PATCH] package/libcamera: remove rpath and strip debug symbols before signing IPA libs
  2022-05-05 20:57 ` Arnout Vandecappelle
@ 2022-05-06  8:56   ` Quentin Schulz
  2022-05-06  9:04     ` Kieran Bingham
  0 siblings, 1 reply; 6+ messages in thread
From: Quentin Schulz @ 2022-05-06  8:56 UTC (permalink / raw)
  To: Arnout Vandecappelle, Quentin Schulz, buildroot
  Cc: Kieran Bingham, Laurent Pinchart

Hi Arnout,

On 5/5/22 22:57, Arnout Vandecappelle wrote:
>   Hi Quentin,
> 
> On 05/05/2022 14:01, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>
>> Open-Source IPA shlibs need to be signed in order to be runnable within
>> the same process, otherwise they are deemed Closed-Source and run in
>> another process and communicate over IPC.
>> Buildroot strips debug symbols and sanitizes RPATH in a post build
>> process. We need to do the same before signing the IPA shlibs otherwise
>> the signature won't match the shlib on the rootfs.
>>
>> meson gets rid of rpath while installing so we don't need to do it
>> manually.
>> However the signing process is also part of the meson install target, so
>> we have a chicken and the egg problem. Let's install the libs in the
>> target directory (and do a useless signing) to get rid of rpath, then
>> strip debug symbols the same way Buildroot does in post build step, then
>> re-sign shlibs directly in TARGET_DIR with signing script from
>> libcamera.
>>
>> Cc: Quentin Schulz <foss+buildroot@0leil.net>
>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>> ---
>>   package/libcamera/libcamera.mk | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/package/libcamera/libcamera.mk 
>> b/package/libcamera/libcamera.mk
>> index 77381ab3ca..d1303a2ff5 100644
>> --- a/package/libcamera/libcamera.mk
>> +++ b/package/libcamera/libcamera.mk
>> @@ -104,4 +104,25 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
>>   LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
>>   endif
>> +# Open-Source IPA shlibs need to be signed in order to be runnable 
>> within the
>> +# same process, otherwise they are deemed Closed-Source and run in 
>> another
>> +# process and communicate over IPC.
>> +# Buildroot strips debug symbols and sanitizes RPATH in a post build 
>> process. We
>> +# need to do the same before signing the IPA shlibs otherwise the 
>> signature
>> +# won't match the shlib on the rootfs.
>> +#
>> +# meson gets rid of rpath while installing so we don't need to do it 
>> manually.
>> +# However the signing process is also part of the meson install 
>> target, so we
>> +# have a chicken and the egg problem. Let's install the libs in the 
>> target
>> +# directory (and do a useless signing) to get rid of rpath, then 
>> strip debug
>> +# symbols the same way Buildroot does in post build step, then 
>> re-sign shlibs
>> +# directly in TARGET_DIR with signing script from libcamera.
>> +define LIBCAMERA_INSTALL_TARGET_CMDS
>> +    $(TARGET_MAKE_ENV) $(LIBCAMERA_NINJA_ENV) DESTDIR=$(TARGET_DIR) \
>> +        $(NINJA) $(NINJA_OPTS) -C $(LIBCAMERA_SRCDIR)/build install
>> +    find $(TARGET_DIR) -type f -name "ipa_*.so" -print0 | xargs -0 
>> $(STRIPCMD) 2>/dev/null || true
> 
>   You should add --no-run-if-empty to xargs, in case there is no library 
> at all (unless you're very sure that doesn't happen). Also, why is the 
> || true needed?
> 

Shamelessly stolen from main Makefile :) 
https://git.busybox.net/buildroot/tree/Makefile#n758

But yes, no guarantee today that an IPA shlib will be installed by 
libcamera package, so I'll add this option to xargs command. Thanks for 
the heads up.

Will remove the redirect and the || true as they are not really useful 
in that package.

>> +    MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib/libcamera/ 
>> $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem 
>> $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
>> +    MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib64/libcamera/ 
>> $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem 
>> $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
> 
>   Instead of overriding INSTALL_TARGET_CMDS, you could also do the 
> additional steps in POST_INSTALL_TARGET_HOOKS.
> 

Was not aware of those hooks, neat :)

>   However, I think it may be simpler to do the stripping in a 
> POST_BUILD_HOOK. That way, you only need to do the strip; the signing is 
> done in the install step, so if we make sure the .so is stripped by 
> then, all should be well, right?
> 

If having the staging shlibs stripped is fine, then this shouldn't be an 
issue indeed.

>   Finally, there is one little caveat: if the libraries are installed in 
> BR2_STRIP_EXCLUDE_DIRS (i.e. if BR2_STRIP_EXCLUDE_DIRS is set to 
> /usr/lib), or if they match BR2_STRIP_EXCLUDE_FILES (e.g. if it is set 
> to "ipa_*.so"), then the libs would be stripped while they shouldn't be. 
> However, I don't think that's a big deal.
> 

Does not hurt to use the same logic as in STRIP_FIND_CMD in main 
Makefile, so will do.

Will send a v2 soon, thanks!

Quentin
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [RFC PATCH] package/libcamera: remove rpath and strip debug symbols before signing IPA libs
  2022-05-06  8:56   ` Quentin Schulz
@ 2022-05-06  9:04     ` Kieran Bingham
  2022-05-06  9:24       ` Quentin Schulz
  0 siblings, 1 reply; 6+ messages in thread
From: Kieran Bingham @ 2022-05-06  9:04 UTC (permalink / raw)
  To: Arnout Vandecappelle, Quentin Schulz, Quentin Schulz, buildroot
  Cc: Laurent Pinchart

Quoting Quentin Schulz (2022-05-06 09:56:36)
> Hi Arnout,
> 
> On 5/5/22 22:57, Arnout Vandecappelle wrote:
> >   Hi Quentin,
> > 
> > On 05/05/2022 14:01, Quentin Schulz wrote:
> >> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >>
> >> Open-Source IPA shlibs need to be signed in order to be runnable within
> >> the same process, otherwise they are deemed Closed-Source and run in
> >> another process and communicate over IPC.
> >> Buildroot strips debug symbols and sanitizes RPATH in a post build
> >> process. We need to do the same before signing the IPA shlibs otherwise
> >> the signature won't match the shlib on the rootfs.
> >>
> >> meson gets rid of rpath while installing so we don't need to do it
> >> manually.
> >> However the signing process is also part of the meson install target, so
> >> we have a chicken and the egg problem. Let's install the libs in the
> >> target directory (and do a useless signing) to get rid of rpath, then
> >> strip debug symbols the same way Buildroot does in post build step, then
> >> re-sign shlibs directly in TARGET_DIR with signing script from
> >> libcamera.
> >>
> >> Cc: Quentin Schulz <foss+buildroot@0leil.net>
> >> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >> ---
> >>   package/libcamera/libcamera.mk | 21 +++++++++++++++++++++
> >>   1 file changed, 21 insertions(+)
> >>
> >> diff --git a/package/libcamera/libcamera.mk 
> >> b/package/libcamera/libcamera.mk
> >> index 77381ab3ca..d1303a2ff5 100644
> >> --- a/package/libcamera/libcamera.mk
> >> +++ b/package/libcamera/libcamera.mk
> >> @@ -104,4 +104,25 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
> >>   LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
> >>   endif
> >> +# Open-Source IPA shlibs need to be signed in order to be runnable 
> >> within the
> >> +# same process, otherwise they are deemed Closed-Source and run in 
> >> another
> >> +# process and communicate over IPC.
> >> +# Buildroot strips debug symbols and sanitizes RPATH in a post build 
> >> process. We
> >> +# need to do the same before signing the IPA shlibs otherwise the 
> >> signature
> >> +# won't match the shlib on the rootfs.
> >> +#
> >> +# meson gets rid of rpath while installing so we don't need to do it 
> >> manually.
> >> +# However the signing process is also part of the meson install 
> >> target, so we
> >> +# have a chicken and the egg problem. Let's install the libs in the 
> >> target
> >> +# directory (and do a useless signing) to get rid of rpath, then 
> >> strip debug
> >> +# symbols the same way Buildroot does in post build step, then 
> >> re-sign shlibs
> >> +# directly in TARGET_DIR with signing script from libcamera.
> >> +define LIBCAMERA_INSTALL_TARGET_CMDS
> >> +    $(TARGET_MAKE_ENV) $(LIBCAMERA_NINJA_ENV) DESTDIR=$(TARGET_DIR) \
> >> +        $(NINJA) $(NINJA_OPTS) -C $(LIBCAMERA_SRCDIR)/build install
> >> +    find $(TARGET_DIR) -type f -name "ipa_*.so" -print0 | xargs -0 
> >> $(STRIPCMD) 2>/dev/null || true
> > 
> >   You should add --no-run-if-empty to xargs, in case there is no library 
> > at all (unless you're very sure that doesn't happen). Also, why is the 
> > || true needed?
> > 
> 
> Shamelessly stolen from main Makefile :) 
> https://git.busybox.net/buildroot/tree/Makefile#n758
> 
> But yes, no guarantee today that an IPA shlib will be installed by 
> libcamera package, so I'll add this option to xargs command. Thanks for 
> the heads up.
> 
> Will remove the redirect and the || true as they are not really useful 
> in that package.
> 
> >> +    MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib/libcamera/ 
> >> $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem 
> >> $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
> >> +    MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib64/libcamera/ 
> >> $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem 
> >> $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
> > 
> >   Instead of overriding INSTALL_TARGET_CMDS, you could also do the 
> > additional steps in POST_INSTALL_TARGET_HOOKS.
> > 
> 
> Was not aware of those hooks, neat :)

Are these hooks called after any modifcations to the files by buildroot,
or before?

If it's after then resigning at that point would be reasonable I think.

> 
> >   However, I think it may be simpler to do the stripping in a 
> > POST_BUILD_HOOK. That way, you only need to do the strip; the signing is 
> > done in the install step, so if we make sure the .so is stripped by 
> > then, all should be well, right?
> > 
> 
> If having the staging shlibs stripped is fine, then this shouldn't be an 
> issue indeed.

This sounds fine to me too though.


> 
> >   Finally, there is one little caveat: if the libraries are installed in 
> > BR2_STRIP_EXCLUDE_DIRS (i.e. if BR2_STRIP_EXCLUDE_DIRS is set to 
> > /usr/lib), or if they match BR2_STRIP_EXCLUDE_FILES (e.g. if it is set 
> > to "ipa_*.so"), then the libs would be stripped while they shouldn't be. 
> > However, I don't think that's a big deal.
> > 
> 
> Does not hurt to use the same logic as in STRIP_FIND_CMD in main 
> Makefile, so will do.
> 
> Will send a v2 soon, thanks!
> 
> Quentin
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [RFC PATCH] package/libcamera: remove rpath and strip debug symbols before signing IPA libs
  2022-05-06  9:04     ` Kieran Bingham
@ 2022-05-06  9:24       ` Quentin Schulz
  2022-05-09 11:00         ` Arnout Vandecappelle
  0 siblings, 1 reply; 6+ messages in thread
From: Quentin Schulz @ 2022-05-06  9:24 UTC (permalink / raw)
  To: Kieran Bingham, Arnout Vandecappelle, Quentin Schulz, buildroot
  Cc: Laurent Pinchart

Hi Kieran,

On 5/6/22 11:04, Kieran Bingham wrote:
> Quoting Quentin Schulz (2022-05-06 09:56:36)
>> Hi Arnout,
>>
>> On 5/5/22 22:57, Arnout Vandecappelle wrote:
>>>    Hi Quentin,
>>>
>>> On 05/05/2022 14:01, Quentin Schulz wrote:
>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>>
>>>> Open-Source IPA shlibs need to be signed in order to be runnable within
>>>> the same process, otherwise they are deemed Closed-Source and run in
>>>> another process and communicate over IPC.
>>>> Buildroot strips debug symbols and sanitizes RPATH in a post build
>>>> process. We need to do the same before signing the IPA shlibs otherwise
>>>> the signature won't match the shlib on the rootfs.
>>>>
>>>> meson gets rid of rpath while installing so we don't need to do it
>>>> manually.
>>>> However the signing process is also part of the meson install target, so
>>>> we have a chicken and the egg problem. Let's install the libs in the
>>>> target directory (and do a useless signing) to get rid of rpath, then
>>>> strip debug symbols the same way Buildroot does in post build step, then
>>>> re-sign shlibs directly in TARGET_DIR with signing script from
>>>> libcamera.
>>>>
>>>> Cc: Quentin Schulz <foss+buildroot@0leil.net>
>>>> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
>>>> ---
>>>>    package/libcamera/libcamera.mk | 21 +++++++++++++++++++++
>>>>    1 file changed, 21 insertions(+)
>>>>
>>>> diff --git a/package/libcamera/libcamera.mk
>>>> b/package/libcamera/libcamera.mk
>>>> index 77381ab3ca..d1303a2ff5 100644
>>>> --- a/package/libcamera/libcamera.mk
>>>> +++ b/package/libcamera/libcamera.mk
>>>> @@ -104,4 +104,25 @@ LIBCAMERA_DEPENDENCIES += libexecinfo
>>>>    LIBCAMERA_LDFLAGS = $(TARGET_LDFLAGS) -lexecinfo
>>>>    endif
>>>> +# Open-Source IPA shlibs need to be signed in order to be runnable
>>>> within the
>>>> +# same process, otherwise they are deemed Closed-Source and run in
>>>> another
>>>> +# process and communicate over IPC.
>>>> +# Buildroot strips debug symbols and sanitizes RPATH in a post build
>>>> process. We
>>>> +# need to do the same before signing the IPA shlibs otherwise the
>>>> signature
>>>> +# won't match the shlib on the rootfs.
>>>> +#
>>>> +# meson gets rid of rpath while installing so we don't need to do it
>>>> manually.
>>>> +# However the signing process is also part of the meson install
>>>> target, so we
>>>> +# have a chicken and the egg problem. Let's install the libs in the
>>>> target
>>>> +# directory (and do a useless signing) to get rid of rpath, then
>>>> strip debug
>>>> +# symbols the same way Buildroot does in post build step, then
>>>> re-sign shlibs
>>>> +# directly in TARGET_DIR with signing script from libcamera.
>>>> +define LIBCAMERA_INSTALL_TARGET_CMDS
>>>> +    $(TARGET_MAKE_ENV) $(LIBCAMERA_NINJA_ENV) DESTDIR=$(TARGET_DIR) \
>>>> +        $(NINJA) $(NINJA_OPTS) -C $(LIBCAMERA_SRCDIR)/build install
>>>> +    find $(TARGET_DIR) -type f -name "ipa_*.so" -print0 | xargs -0
>>>> $(STRIPCMD) 2>/dev/null || true
>>>
>>>    You should add --no-run-if-empty to xargs, in case there is no library
>>> at all (unless you're very sure that doesn't happen). Also, why is the
>>> || true needed?
>>>
>>
>> Shamelessly stolen from main Makefile :)
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__git.busybox.net_buildroot_tree_Makefile-23n758&d=DwIFaQ&c=_sEr5x9kUWhuk4_nFwjJtA&r=LYjLexDn7rXIzVmkNPvw5ymA1XTSqHGq8yBP6m6qZZ4njZguQhZhkI_-172IIy1t&m=v7wKLvb2dSEca5qoEy27u1GlltwWpk-os-gDEP8xPgINiPx_Wi0sl33H9p0IWbhB&s=1cdGHSGxMYhIJBf276S9PZjGtNuSRmIceB_oL1bI4FY&e=
>>
>> But yes, no guarantee today that an IPA shlib will be installed by
>> libcamera package, so I'll add this option to xargs command. Thanks for
>> the heads up.
>>
>> Will remove the redirect and the || true as they are not really useful
>> in that package.
>>
>>>> +    MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib/libcamera/
>>>> $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem
>>>> $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
>>>> +    MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib64/libcamera/
>>>> $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem
>>>> $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
>>>
>>>    Instead of overriding INSTALL_TARGET_CMDS, you could also do the
>>> additional steps in POST_INSTALL_TARGET_HOOKS.
>>>
>>
>> Was not aware of those hooks, neat :)
> 
> Are these hooks called after any modifcations to the files by buildroot,
> or before?
>  > If it's after then resigning at that point would be reasonable I think.
> 

AFAIK, before.

Buildroot runs all requested tasks/steps for each package, then remove 
rpath and strips symbols in target/ directory.

POST_INSTALL_TARGET_HOOKS is run right after the install task/step for 
the package is run (basically after ninja install is executed).

POST_BUILD_HOOK is happening right after the build task/step (after 
ninja is executed) of the package is run.

We anyway need to strip the shlibs manually in the package makefile 
because nothing does it and I don't think there's a way to hook 
something from a package makefile to happen after Buildroot "main" post 
build script is run (not the one in package makefiles, the one run right 
before creating the rootfs).

>>
>>>    However, I think it may be simpler to do the stripping in a
>>> POST_BUILD_HOOK. That way, you only need to do the strip; the signing is
>>> done in the install step, so if we make sure the .so is stripped by
>>> then, all should be well, right?
>>>
>>
>> If having the staging shlibs stripped is fine, then this shouldn't be an
>> issue indeed.
> 
> This sounds fine to me too though.
> 
> 
>>
>>>    Finally, there is one little caveat: if the libraries are installed in
>>> BR2_STRIP_EXCLUDE_DIRS (i.e. if BR2_STRIP_EXCLUDE_DIRS is set to
>>> /usr/lib), or if they match BR2_STRIP_EXCLUDE_FILES (e.g. if it is set
>>> to "ipa_*.so"), then the libs would be stripped while they shouldn't be.
>>> However, I don't think that's a big deal.
>>>
>>
>> Does not hurt to use the same logic as in STRIP_FIND_CMD in main
>> Makefile, so will do.
>>

Can't use the logic for BR2_STRIP_EXCLUDE_DIRS because the dir in the 
build directory of libcamera is different from the one installed on the 
rootfs (i.e. build/src/ipa/*/ipa_*.so while on the target it'd be 
/usr/lib{64,}/libcamera/ipa_*.so).

For BR2_STRIP_EXCLUDE_FILES, if just a filename should be given, the 
find command will not care about intermediary directories so we're fine.

Meanwhile, I realised that STRIPCMD in package makefiles might not be 
the same as the one in the main Makefile. Namely, it's /bin/true if 
BR2_STRIP_strip is unset. So I'll need to use a hardcoded version 
instead of STRIPCMD variable unfortunately :/ Testing this to make sure 
I didn't misread the code.

Cheers,
Quentin
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Buildroot] [RFC PATCH] package/libcamera: remove rpath and strip debug symbols before signing IPA libs
  2022-05-06  9:24       ` Quentin Schulz
@ 2022-05-09 11:00         ` Arnout Vandecappelle
  0 siblings, 0 replies; 6+ messages in thread
From: Arnout Vandecappelle @ 2022-05-09 11:00 UTC (permalink / raw)
  To: Quentin Schulz, Kieran Bingham, Quentin Schulz, buildroot
  Cc: Laurent Pinchart



On 06/05/2022 11:24, Quentin Schulz wrote:
> Hi Kieran,
> 
> On 5/6/22 11:04, Kieran Bingham wrote:
>> Quoting Quentin Schulz (2022-05-06 09:56:36)
>>> Hi Arnout,
>>>
>>> On 5/5/22 22:57, Arnout Vandecappelle wrote:
>>>>    Hi Quentin,
>>>>
>>>> On 05/05/2022 14:01, Quentin Schulz wrote:
>>>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

[snip]
>>>>> +    MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib/libcamera/
>>>>> $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem
>>>>> $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
>>>>> +    MESON_INSTALL_DESTDIR_PREFIX=$(TARGET_DIR)/usr/lib64/libcamera/
>>>>> $(@D)/src/ipa/ipa-sign-install.sh $(@D)/build/src/ipa-priv-key.pem
>>>>> $(addprefix ipa_,$(addsuffix .so,$(LIBCAMERA_PIPELINES-y)))
>>>>
>>>>    Instead of overriding INSTALL_TARGET_CMDS, you could also do the
>>>> additional steps in POST_INSTALL_TARGET_HOOKS.
>>>>
>>>
>>> Was not aware of those hooks, neat :)
>>
>> Are these hooks called after any modifcations to the files by buildroot,
>> or before?
>>  > If it's after then resigning at that point would be reasonable I think.
>>
> 
> AFAIK, before.
> 
> Buildroot runs all requested tasks/steps for each package, then remove rpath and 
> strips symbols in target/ directory.
> 
> POST_INSTALL_TARGET_HOOKS is run right after the install task/step for the 
> package is run (basically after ninja install is executed).
> 
> POST_BUILD_HOOK is happening right after the build task/step (after ninja is 
> executed) of the package is run.
> 
> We anyway need to strip the shlibs manually in the package makefile because 
> nothing does it and I don't think there's a way to hook something from a package 
> makefile to happen after Buildroot "main" post build script is run (not the one 
> in package makefiles, the one run right before creating the rootfs).

  Actually, there is, I didn't think of that! $(PKG)_ROOTFS_PRE_CMD_HOOKS gets 
executed right before the rootfs is created. Note that this is during the rootfs 
creation step and is done in a temporary directory (TARGET_DIR is overridden 
with the name of that temporary directory, so you can still use TARGET_DIR).

  It's actually meant for stuff that needs to be run under fakeroot, but it can 
be used for this as well.

  With that approach, we still have the unstripped libraries in STAGING_DIR 
(which is useful for debugging), and we don't need to re-create the strip 
commands. The disadvantages are:
- contents of target are not usable as is (signature will be wrong);
- we still need to "manually" call ipa-sign-install.sh.

  So all in all, I'm not sure that that option is better than your v2.

  (There is also $(PKG)_TARGET_FINALIZE_HOOKS, but those get called before 
stripping, so not useful here.)


  Regards,
  Arnout


> 
>>>
>>>>    However, I think it may be simpler to do the stripping in a
>>>> POST_BUILD_HOOK. That way, you only need to do the strip; the signing is
>>>> done in the install step, so if we make sure the .so is stripped by
>>>> then, all should be well, right?
>>>>
>>>
>>> If having the staging shlibs stripped is fine, then this shouldn't be an
>>> issue indeed.
>>
>> This sounds fine to me too though.
>>
>>
>>>
>>>>    Finally, there is one little caveat: if the libraries are installed in
>>>> BR2_STRIP_EXCLUDE_DIRS (i.e. if BR2_STRIP_EXCLUDE_DIRS is set to
>>>> /usr/lib), or if they match BR2_STRIP_EXCLUDE_FILES (e.g. if it is set
>>>> to "ipa_*.so"), then the libs would be stripped while they shouldn't be.
>>>> However, I don't think that's a big deal.
>>>>
>>>
>>> Does not hurt to use the same logic as in STRIP_FIND_CMD in main
>>> Makefile, so will do.
>>>
> 
> Can't use the logic for BR2_STRIP_EXCLUDE_DIRS because the dir in the build 
> directory of libcamera is different from the one installed on the rootfs (i.e. 
> build/src/ipa/*/ipa_*.so while on the target it'd be 
> /usr/lib{64,}/libcamera/ipa_*.so).
> 
> For BR2_STRIP_EXCLUDE_FILES, if just a filename should be given, the find 
> command will not care about intermediary directories so we're fine.
> 
> Meanwhile, I realised that STRIPCMD in package makefiles might not be the same 
> as the one in the main Makefile. Namely, it's /bin/true if BR2_STRIP_strip is 
> unset. So I'll need to use a hardcoded version instead of STRIPCMD variable 
> unfortunately :/ Testing this to make sure I didn't misread the code.
> 
> Cheers,
> Quentin
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-05-09 11:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-05 12:01 [Buildroot] [RFC PATCH] package/libcamera: remove rpath and strip debug symbols before signing IPA libs Quentin Schulz
2022-05-05 20:57 ` Arnout Vandecappelle
2022-05-06  8:56   ` Quentin Schulz
2022-05-06  9:04     ` Kieran Bingham
2022-05-06  9:24       ` Quentin Schulz
2022-05-09 11:00         ` Arnout Vandecappelle

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.