falcosecurity / libs

libsinsp, libscap, the kernel module driver, and the eBPF driver sources
https://falcosecurity.github.io/libs/
Apache License 2.0
227 stars 162 forks source link

PATCH: fix missing DSO tags for libdriver-event-schema.so #1201

Closed dkogan closed 1 year ago

dkogan commented 1 year ago

Hello. There's a bug in the build system: DSO tags for libscap-event-schema.so are set twice, but never for libdriver-event-schema.so. There should be one for each. I'm the Debian maintainer, and I just fixed this for the distro:

https://salsa.debian.org/debian/falcosecurity-libs/-/blob/master/debian/patches/properties-for-driver-event-schema.patch

I think you should take the patch.

Thanks

FedeDP commented 1 year ago

Hi! Would you mind opening the PR? Thank you for reporting!

FedeDP commented 1 year ago

Also, as @geraldcombs pointed out (https://github.com/falcosecurity/libs/issues/762#issuecomment-1637189671) i see you have got multiple patches downstream in debian; would you mind upstreaming them? https://salsa.debian.org/debian/falcosecurity-libs/-/tree/master/debian/patches

dkogan commented 1 year ago

Hi. Would you mind doing the PR yourself? It's quite a bit of typing on my end.

Thanks

dkogan commented 1 year ago

Also, as @geraldcombs pointed out (#762 (comment)) i see you have got multiple patches downstream in debian; would you mind upstreaming them?

Absolutely. Please open PRs as needed.

We have

FedeDP commented 1 year ago
  • fix-library-install-path.patch

    Fixes library install paths. I don't know why it was set up the way it was set up. If you want to be more "normal", you should take the patch

Mmh you mean because we are using a subfolder for our libraries? It is not that uncommon as far as i know (eg: pipewire installs its .so under /usr/lib/pipewire-0.3/ ).

  • properties-for-driver-event-schema.patch

This is 100% an issue :) Thank you! I am going to open the PR!

FedeDP commented 1 year ago

Opened: https://github.com/falcosecurity/libs/pull/1215 Thank you (you got the co-authored-by of course :) )

dkogan commented 1 year ago

Federico Di Pierro @.***> writes:

  • fix-library-install-path.patch

    Fixes library install paths. I don't know why it was set up the way it was set up. If you want to be more "normal", you should take the patch

Mmh you mean because we are using a subfolder for our libraries? It is not that uncommon as far as i know (eg: pipewire installs its .so under /usr/lib/pipewire-0.3/ ).

By necessity libraries are placed in subdirectories only if they will never be loaded using the normal dynamic linking mechanism. Today we have this:

@.***:~$ objdump -p /usr/bin/sysdig | grep scap NEEDED libscap_event_schema.so.0

So if I run "sysdig" it will need to find "libscap_event_schema.so.0". The list of search paths includes "/usr/lib" and "/usr/lib/ARCH" but not "/usr/lib/falco". So by putting your libraries into a weird place, you either broke sysdig, or you had to do extra work for sysdig to find your libraries.

For Debian, this patch makes sense. I think it also makes sense for everyone else, but you can decide.