[PATCH] insane.bbclass: tighten lib_re and exec_re patterns to avoid false positive

[PATCH] insane.bbclass: tighten lib_re and exec_re patterns to avoid false positive

[Bill Randle]

lib_re would match files like "/libsoletta.so.0.0.1-gdb.py" which are
not valid library filenames. Tighten up the re for lib_re and exec_re
so they match only valid sonames (.so or .so followed by major version
and optional minor version and release).

[YOCTO #9215]

Signed-off-by: Bill Randle <william.c.randle@intel.com>

---
 meta/classes/insane.bbclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
index 7ac945d..e60c358 100644
--- a/meta/classes/insane.bbclass
+++ b/meta/classes/insane.bbclass
@@ -318,8 +318,8 @@ def package_qa_check_libdir(d):
 
     messages = []
 
-    lib_re = re.compile("^/lib.+\.so(\..+)?$")
-    exec_re = re.compile("^%s.*/lib.+\.so(\..+)?$" % exec_prefix)
+    lib_re = re.compile("^/lib.+\.so(\.\d+){0,3}$")
+    exec_re = re.compile("^%s.*/lib.+\.so(\.\d+){0,3}$" % exec_prefix)
 
     for root, dirs, files in os.walk(pkgdest):
         if root == pkgdest:
-- 
2.5.0

Re: [PATCH] insane.bbclass: tighten lib_re and exec_re patterns to avoid false positive

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

On 23 March 2016 at 15:54, Bill Randle <william.c.randle@intel.com> wrote:

> +    lib_re = re.compile("^/lib.+\.so(\.\d+){0,3}$")
> +    exec_re = re.compile("^%s.*/lib.+\.so(\.\d+){0,3}$" % exec_prefix)
>

I worry that this goes too far in the other way, my Debian host has a
library called /usr/lib/libblas.so.3gf.

Maybe we should add a "is this an ELF" test into the libdir check instead?
That might kill performance though, and this is just a sanity check.

Any other thoughts?

Ross

[-- Attachment #2: Type: text/html, Size: 1137 bytes --]

Re: [PATCH] insane.bbclass: tighten lib_re and exec_re patterns to avoid false positive

[Randle, William C]

On Wed, 2016-03-23 at 16:23 +0000, Burton, Ross wrote:

On 23 March 2016 at 15:54, Bill Randle <william.c.randle@intel.com<mailto:william.c.randle@intel.com>> wrote:
+    lib_re = re.compile("^/lib.+\.so(\.\d+){0,3}$")
+    exec_re = re.compile("^%s.*/lib.+\.so(\.\d+){0,3}$" % exec_prefix)


I worry that this goes too far in the other way, my Debian host has a library called /usr/lib/libblas.so.3gf.

Maybe we should add a "is this an ELF" test into the libdir check instead?  That might kill performance though, and this is just a sanity check.

Any other thoughts?

Ross

Ok, I found a library on my FC23 system named "libdmraid.so.1.0.0.rc16" in /usr/lib64. The available documentation on sonames I found states:


"Every shared library has a special name called the ``soname''. The soname has the prefix ``lib'', the name of the library, the phrase ``.so'', followed by a period and a version number that is incremented whenever the interface changes (as a special exception, the lowest-level C libraries don't start with ``lib''). A fully-qualified soname includes as a prefix the directory it's in; on a working system a fully-qualified soname is simply a symbolic link to the shared library's ``real name''.


"Every shared library also has a ``real name'', which is the filename containing the actual library code. The real name adds to the soname a period, a minor number, another period, and the release number. The last period and release number are optional. The minor number and release number support configuration control by letting you know exactly what version(s) of the library are installed. Note that these numbers might not be the same as the numbers used to describe the library in documentation, although that does make things easier."


That implies there should only be upto three numbers after the .so, to the ".rc16" on the end violates that rule, and the implication is these should be version *numbers*, not version strings, which would mean your library ending in .3gf violates that rule.


I guess some rules are just meant to be broken. Sigh.


    -Bill

Re: [PATCH] insane.bbclass: tighten lib_re and exec_re patterns to avoid false positive

[Phil Blundell]

On Wed, 2016-03-23 at 08:54 -0700, Bill Randle wrote:
> lib_re would match files like "/libsoletta.so.0.0.1-gdb.py" which are
> not valid library filenames.

What are the consequences of it matching things that are not libraries?

> Tighten up the re for lib_re and exec_re so they match only valid
> sonames (.so or .so followed by major version and optional minor
> version and release).

Where does this definition of "valid sonames" come from?

p.

Re: [PATCH] insane.bbclass: tighten lib_re and exec_re patterns to avoid false positive

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 478 bytes --]

On 23 March 2016 at 17:56, Phil Blundell <pb@pbcl.net> wrote:

> > lib_re would match files like "/libsoletta.so.0.0.1-gdb.py" which are
> > not valid library filenames.
>
> What are the consequences of it matching things that are not libraries?


See #9215, the libdir sanity test fires when it shouldn't.

WARNING: QA Issue: soletta-dbg: found library in wrong location:
/usr/share/gdb/auto-load/libsoletta.so.0.0.1-gdb.py [libdir]

That's no library...

Ross

[-- Attachment #2: Type: text/html, Size: 1033 bytes --]

Re: [PATCH] insane.bbclass: tighten lib_re and exec_re patterns to avoid false positive

[Phil Blundell]

[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]

On Wed, 2016-03-23 at 17:59 +0000, Burton, Ross wrote:
> 
> On 23 March 2016 at 17:56, Phil Blundell <pb@pbcl.net> wrote:
> > > lib_re would match files like "/libsoletta.so.0.0.1-gdb.py" which
> > are
> > > not valid library filenames.
> > 
> > What are the consequences of it matching things that are not
> > libraries?
> See #9215, the libdir sanity test fires when it shouldn't.
> 
> WARNING: QA Issue: soletta-dbg: found library in wrong location:
> /usr/share/gdb/auto-load/libsoletta.so.0.0.1-gdb.py [libdir]
> 
> That's no library...
Ah, right. I see.
I wonder if we should just give insane.bbclass a whitelist of
directories in which it should ignore apparently misplaced libraries.
 There aren't likely to be many cases like that one.
Or alternatively, add an extra check so that when it thinks it's found
a misplaced library, it checks the ELF header to make sure that it
really is one and suppresses the diagnostic if not.  If you only do
this in cases where it was about to print a warning anyway then the
impact on performance shouldn't be too bad.
p.

[-- Attachment #2: Type: text/html, Size: 1642 bytes --]

Re: [PATCH] insane.bbclass: tighten lib_re and exec_re patterns to avoid false positive

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 583 bytes --]

On 23 March 2016 at 18:03, Phil Blundell <pb@pbcl.net> wrote:

> Or alternatively, add an extra check so that when it thinks it's found a
> misplaced library, it checks the ELF header to make sure that it really is
> one and suppresses the diagnostic if not.  If you only do this in cases
> where it was about to print a warning anyway then the impact on performance
> shouldn't be too bad.
>

Yeah, best of both worlds.  A flexible regex to catch potential offenders,
and then use oe.qa.ELFFile to verify it's actually a library.  Can you
update the patch Bill?

Ross

[-- Attachment #2: Type: text/html, Size: 920 bytes --]

Re: [PATCH] insane.bbclass: tighten lib_re and exec_re patterns to avoid false positive

[Randle, William C]

On Wed, 2016-03-23 at 21:17 +0000, Burton, Ross wrote:
> 
> On 23 March 2016 at 18:03, Phil Blundell <pb@pbcl.net> wrote:
> > Or alternatively, add an extra check so that when it thinks it's found a
> > misplaced library, it checks the ELF header to make sure that it really is
> > one and suppresses the diagnostic if not.  If you only do this in cases
> > where it was about to print a warning anyway then the impact on performance
> > shouldn't be too bad.
> > 
> Yeah, best of both worlds.  A flexible regex to catch potential offenders, and
> then use oe.qa.ELFFile to verify it's actually a library.  Can you update the
> patch Bill?
> 
Yes, I'll update the patch and repost in a bit.

    -Bill