[PATCH 0/5] Improve isELF, gets triggered by ELF anywhere in pathname

[PATCH 0/5] Improve isELF, gets triggered by ELF anywhere in pathname

[Olof Johansson]

If the substring "ELF" is found anywhere in the pathname, the isELF function
would identify the file as an ELF file. The function could also be used to
execute arbitrary shell commands as the user running bitbake, since the file
execution allows processing of shell meta characters like variable expansion.

The isELF function has been copied and was until this patchset available from
two locations, one in lib/oe/package.py and one in package.bbclass. The two
functions had diverged. This is changed so that one common implementation is
used.

Olof Johansson (5):
  lib/oe/package.py: Expose is_elf
  package.bbclass: Make use of common is_elf function
  lib/oe/package.py: is_elf: Don't let filename influence filetype
  lib/oe/package.py: is_elf: Disallow shell specials to be expanded
  lib/oe/package.py: is_elf: Make it less prone to false positives

 meta/classes/package.bbclass | 40 +++++---------------
 meta/lib/oe/package.py       | 88 +++++++++++++++++++++++++++-----------------
 2 files changed, 63 insertions(+), 65 deletions(-)

-- 
2.11.0

[PATCH 1/5] lib/oe/package.py: Expose is_elf

[Olof Johansson]

is_elf/isELF had copies in both staging.bbclass and package.bbclass.
After recent refactoring in staging.bbclass (involving breaking out the
isELF function to is_elf in lib/oe/package.py), the implementions
diverged. It would be beneficial to make everybody use this one
implementation, so let's expose it here for others to use.

Signed-off-by: Olof Johansson <olofjn@axis.com>
---
 meta/lib/oe/package.py | 88 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 54 insertions(+), 34 deletions(-)

diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index 1e5c3aa8e1..f1f9333e0f 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -1,3 +1,6 @@
+import mmap
+import oe.utils
+
 def runstrip(arg):
     # Function to strip a single file, called from split_and_strip_files below
     # A working 'file' (one which works on the target architecture)
@@ -44,6 +47,56 @@ def runstrip(arg):
 
     return
 
+# Detect .ko module by searching for "vermagic=" string
+def is_kernel_module(path):
+    with open(path) as f:
+        return mmap.mmap(f.fileno(), 0, prot=mmap.PROT_READ).find(b"vermagic=") >= 0
+
+def _is_elf_error(msg):
+    bb.error('is_elf: %s' % msg)
+
+def is_elf(path, on_error=_is_elf_error):
+    """
+    Determine if a given file is an ELF archive (and other attributes),
+    using the file utility.
+
+    :param path: str, path of potential ELF file
+    :param on_error: callable, gets called when an error occurs.
+                     the callback takes a message parameter. A
+                     default error handler is provided that prints
+                     the message with 'bb.error'.
+
+    is_elf returns a bitstring of flags, corresponding to various
+    properties:
+
+    *  1: ELF
+    *  2: stripped
+    *  4: executable
+    *  8: shared library
+    * 16: kernel module
+
+    A return value of 0 means that the file is not an ELF file.
+    """
+    ret, result = oe.utils.getstatusoutput(
+        "file \"%s\"" % path.replace("\"", "\\\""))
+
+    if ret:
+        error_cb('"file %s" failed')
+        return
+
+    if not "ELF" in result:
+        return 0
+
+    exec_type = 1
+    if "not stripped" not in result:
+        exec_type |= 2
+    if "executable" in result:
+        exec_type |= 4
+    if "shared" in result:
+        exec_type |= 8
+    if "relocatable" in result and is_kernel_module(path):
+        exec_type |= 16
+    return exec_type
 
 def strip_execs(pn, dstdir, strip_cmd, libdir, base_libdir, qa_already_stripped=False):
     """
@@ -56,40 +109,7 @@ def strip_execs(pn, dstdir, strip_cmd, libdir, base_libdir, qa_already_stripped=
     :param qa_already_stripped: Set to True if already-stripped' in ${INSANE_SKIP}
     This is for proper logging and messages only.
     """
-    import stat, errno, oe.path, oe.utils, mmap
-
-    # Detect .ko module by searching for "vermagic=" string
-    def is_kernel_module(path):
-        with open(path) as f:
-            return mmap.mmap(f.fileno(), 0, prot=mmap.PROT_READ).find(b"vermagic=") >= 0
-
-    # Return type (bits):
-    # 0 - not elf
-    # 1 - ELF
-    # 2 - stripped
-    # 4 - executable
-    # 8 - shared library
-    # 16 - kernel module
-    def is_elf(path):
-        exec_type = 0
-        ret, result = oe.utils.getstatusoutput(
-            "file \"%s\"" % path.replace("\"", "\\\""))
-
-        if ret:
-            bb.error("split_and_strip_files: 'file %s' failed" % path)
-            return exec_type
-
-        if "ELF" in result:
-            exec_type |= 1
-            if "not stripped" not in result:
-                exec_type |= 2
-            if "executable" in result:
-                exec_type |= 4
-            if "shared" in result:
-                exec_type |= 8
-            if "relocatable" in result and is_kernel_module(path):
-                exec_type |= 16
-        return exec_type
+    import stat, errno, oe.path, oe.utils
 
     elffiles = {}
     inodes = {}
-- 
2.11.0

Re: [PATCH 1/5] lib/oe/package.py: Expose is_elf

[Olof Johansson]

On 17-12-01 16:50 +0100, Olof Johansson wrote:
> is_elf/isELF had copies in both staging.bbclass and package.bbclass.
> After recent refactoring in staging.bbclass (involving breaking out the
> isELF function to is_elf in lib/oe/package.py), the implementions
> diverged. It would be beneficial to make everybody use this one
> implementation, so let's expose it here for others to use.
...
> +def is_elf(path, on_error=_is_elf_error):
...
> +    :param on_error: callable, gets called when an error occurs.
> +                     the callback takes a message parameter. A
> +                     default error handler is provided that prints
> +                     the message with 'bb.error'.
> +
...
> +    """
...
> +    if ret:
> +        error_cb('"file %s" failed')

This should be on_error, not error_cb, sorry! Will fix in v2,
together with not assuming "," in file output, as suggested by
Mark.

-- 
olofjn

[PATCH 2/5] package.bbclass: Make use of common is_elf function

[Olof Johansson]

The isELF and is_elf function share a common ancestry, but have
diverged. Let's use the implementation from oe.package.

Signed-off-by: Olof Johansson <olofjn@axis.com>
---
 meta/classes/package.bbclass | 40 +++++++++-------------------------------
 1 file changed, 9 insertions(+), 31 deletions(-)

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index 2053d46395..f65596126d 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -857,6 +857,12 @@ python fixup_perms () {
 
 python split_and_strip_files () {
     import stat, errno
+    import oe.package
+
+    def is_elf(path):
+        return oe.package.is_elf(
+            path, lambda msg: package_qa_handle_error("split-strip", msg, d)
+        )
 
     dvar = d.getVar('PKGD')
     pn = d.getVar('PN')
@@ -892,34 +898,6 @@ python split_and_strip_files () {
     sourcefile = d.expand("${WORKDIR}/debugsources.list")
     bb.utils.remove(sourcefile)
 
-    # Return type (bits):
-    # 0 - not elf
-    # 1 - ELF
-    # 2 - stripped
-    # 4 - executable
-    # 8 - shared library
-    # 16 - kernel module
-    def isELF(path):
-        type = 0
-        ret, result = oe.utils.getstatusoutput("file \"%s\"" % path.replace("\"", "\\\""))
-
-        if ret:
-            msg = "split_and_strip_files: 'file %s' failed" % path
-            package_qa_handle_error("split-strip", msg, d)
-            return type
-
-        # Not stripped
-        if "ELF" in result:
-            type |= 1
-            if "not stripped" not in result:
-                type |= 2
-            if "executable" in result:
-                type |= 4
-            if "shared" in result:
-                type |= 8
-        return type
-
-
     #
     # First lets figure out all of the files we may have to process ... do this only once!
     #
@@ -961,14 +939,14 @@ python split_and_strip_files () {
                     # If it's a symlink, and points to an ELF file, we capture the readlink target
                     if cpath.islink(file):
                         target = os.readlink(file)
-                        if isELF(ltarget):
-                            #bb.note("Sym: %s (%d)" % (ltarget, isELF(ltarget)))
+                        if is_elf(ltarget):
+                            #bb.note("Sym: %s (%d)" % (ltarget, is_elf(ltarget)))
                             symlinks[file] = target
                         continue
 
                     # It's a file (or hardlink), not a link
                     # ...but is it ELF, and is it already stripped?
-                    elf_file = isELF(file)
+                    elf_file = is_elf(file)
                     if elf_file & 1:
                         if elf_file & 2:
                             if 'already-stripped' in (d.getVar('INSANE_SKIP_' + pn) or "").split():
-- 
2.11.0

[PATCH 3/5] lib/oe/package.py: is_elf: Don't let filename influence filetype

[Olof Johansson]

The is_elf function is simply looking for the substring ELF in the
output from file. But file, by default, prefixes the outut with the
filename. If the filename containts the substring ELF anywhere, is_elf
would identify it as a ELF.

The --brief (or -b) flag of GNU's file utility makes file not prepend
filename to the output.

Signed-off-by: Olof Johansson <olofjn@axis.com>
---
 meta/lib/oe/package.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index f1f9333e0f..eab94feb91 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -78,7 +78,7 @@ def is_elf(path, on_error=_is_elf_error):
     A return value of 0 means that the file is not an ELF file.
     """
     ret, result = oe.utils.getstatusoutput(
-        "file \"%s\"" % path.replace("\"", "\\\""))
+        "file -b \"%s\"" % path.replace("\"", "\\\""))
 
     if ret:
         error_cb('"file %s" failed')
-- 
2.11.0

[PATCH 4/5] lib/oe/package.py: is_elf: Disallow shell specials to be expanded

[Olof Johansson]

By using single quotes instead of double quotes, we don't have to worry
about escaping dangerous characters, other than ' itself.

Signed-off-by: Olof Johansson <olofjn@axis.com>
---
 meta/lib/oe/package.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index eab94feb91..976d2ef36c 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -78,7 +78,7 @@ def is_elf(path, on_error=_is_elf_error):
     A return value of 0 means that the file is not an ELF file.
     """
     ret, result = oe.utils.getstatusoutput(
-        "file -b \"%s\"" % path.replace("\"", "\\\""))
+        "file -b '%s'" % path.replace("'", "\\'"))
 
     if ret:
         error_cb('"file %s" failed')
-- 
2.11.0

[PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Olof Johansson]

Avoid matching substrings that are picked up from paths, for instance.
Do this by anchoring the tokens we look for (e.g "executable" or "not
stripped") with whitespace and punctuation.

Submitted with this patch series is a change that adds the use of
--brief to file. This removes the path prefix to the output, but the
path can still be included in shebang lines (which file will report as
something like "a /foo/bar/baz.py script").

Signed-off-by: Olof Johansson <olofjn@axis.com>
---
 meta/lib/oe/package.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
index 976d2ef36c..2bd771cfc5 100644
--- a/meta/lib/oe/package.py
+++ b/meta/lib/oe/package.py
@@ -84,17 +84,17 @@ def is_elf(path, on_error=_is_elf_error):
         error_cb('"file %s" failed')
         return
 
-    if not "ELF" in result:
+    if not result.startswith("ELF "):
         return 0
 
     exec_type = 1
-    if "not stripped" not in result:
+    if ", not stripped" not in result:
         exec_type |= 2
-    if "executable" in result:
+    if " executable, " in result:
         exec_type |= 4
-    if "shared" in result:
+    if " shared object, " in result:
         exec_type |= 8
-    if "relocatable" in result and is_kernel_module(path):
+    if "relocatable, " in result and is_kernel_module(path):
         exec_type |= 16
     return exec_type
 
-- 
2.11.0

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Mark Hatle]

On 12/1/17 9:50 AM, Olof Johansson wrote:
> Avoid matching substrings that are picked up from paths, for instance.
> Do this by anchoring the tokens we look for (e.g "executable" or "not
> stripped") with whitespace and punctuation.
> 
> Submitted with this patch series is a change that adds the use of
> --brief to file. This removes the path prefix to the output, but the
> path can still be included in shebang lines (which file will report as
> something like "a /foo/bar/baz.py script").
> 
> Signed-off-by: Olof Johansson <olofjn@axis.com>
> ---
>  meta/lib/oe/package.py | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
> index 976d2ef36c..2bd771cfc5 100644
> --- a/meta/lib/oe/package.py
> +++ b/meta/lib/oe/package.py
> @@ -84,17 +84,17 @@ def is_elf(path, on_error=_is_elf_error):
>          error_cb('"file %s" failed')
>          return
>  
> -    if not "ELF" in result:
> +    if not result.startswith("ELF "):
>          return 0
>  
>      exec_type = 1
> -    if "not stripped" not in result:
> +    if ", not stripped" not in result:

The original implementation did not include the ',' options because different
versions of file (as well as different architectures) would return different
strings.

They all contains the same information, but order and inclusion of ',' changed
regularly.

So I would caution that for this to check out a wide variety of host systems and
architectures would need to be verified.  (It's very possible that all modern
systems now conform to a single standard...)

(The rest of the serious looks like a very good improvement, and I've got no
further comments on that.)

--Mark

>          exec_type |= 2
> -    if "executable" in result:
> +    if " executable, " in result:
>          exec_type |= 4
> -    if "shared" in result:
> +    if " shared object, " in result:
>          exec_type |= 8
> -    if "relocatable" in result and is_kernel_module(path):
> +    if "relocatable, " in result and is_kernel_module(path):
>          exec_type |= 16
>      return exec_type
>  
> 

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Olof Johansson]

On 17-12-01 11:43 -0600, Mark Hatle wrote:
> The original implementation did not include the ',' options because different
> versions of file (as well as different architectures) would return different
> strings.
> 
> They all contains the same information, but order and inclusion of ',' changed
> regularly.
> 
> So I would caution that for this to check out a wide variety of host systems and
> architectures would need to be verified.  (It's very possible that all modern
> systems now conform to a single standard...)
> 
> (The rest of the serious looks like a very good improvement, and I've got no
> further comments on that.)

I was a little bit worried about this, thanks, this is useful
feedback. Perhaps dropping the "," while still retaining the
whitespace delimitation could be enough, and still work with a
variety of file implementations?

And while we're talking portability, is the use of GNU file's
--brief option a problem (as introduced by another patch in this
series)? It isn't specified in posix, I don't know how widespread
it is.

-- 
olofjn

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Olof Johansson]

On 17-12-01 22:13 +0100, Olof Johansson wrote:
> On 17-12-01 11:43 -0600, Mark Hatle wrote:
> > The original implementation did not include the ',' options because different
> > versions of file (as well as different architectures) would return different
> > strings.
> > 
> > They all contains the same information, but order and inclusion of ',' changed
> > regularly.
> > 
> > So I would caution that for this to check out a wide variety of host systems and
> > architectures would need to be verified.  (It's very possible that all modern
> > systems now conform to a single standard...)
> 
> I was a little bit worried about this, thanks, this is useful
> feedback. Perhaps dropping the "," while still retaining the
> whitespace delimitation could be enough, and still work with a
> variety of file implementations?

Reading more about this, I was wrong in calling it GNU file, I
don't know where I got that from. It actually seems to be only
one prevalent implementation, one written by Ian Darwin [0]:

> Who's using it?
>
>    Most known BSD distributions (FreeBSD, NetBSD, Darwin/Mac OS X, etc)
>    Every known Linux distribution

Could you elaborate on the portability issues you've seen?


0: http://www.darwinsys.com/file/

-- 
olofjn

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Mark Hatle]

On 12/4/17 4:00 AM, Olof Johansson wrote:
> On 17-12-01 22:13 +0100, Olof Johansson wrote:
>> On 17-12-01 11:43 -0600, Mark Hatle wrote:
>>> The original implementation did not include the ',' options because different
>>> versions of file (as well as different architectures) would return different
>>> strings.
>>>
>>> They all contains the same information, but order and inclusion of ',' changed
>>> regularly.
>>>
>>> So I would caution that for this to check out a wide variety of host systems and
>>> architectures would need to be verified.  (It's very possible that all modern
>>> systems now conform to a single standard...)
>>
>> I was a little bit worried about this, thanks, this is useful
>> feedback. Perhaps dropping the "," while still retaining the
>> whitespace delimitation could be enough, and still work with a
>> variety of file implementations?
> 
> Reading more about this, I was wrong in calling it GNU file, I
> don't know where I got that from. It actually seems to be only
> one prevalent implementation, one written by Ian Darwin [0]:

I'm thinking back to RHEL 4-ish era, Ubuntu, Debian, Arch, Gentoo, etc.  We were
getting a mix/match of different results depending on the age of the 'file'
application being used, and even the magic file associated with it.

All of the information was present, but the order of certain elements changed
from arch to arch, and as I mentioned before the ',' bit was not always present.

It's possible that all modern file based systems are now using the same sources
and magic, but it was definitely not true in the past.

--Mark

>> Who's using it?
>>
>>    Most known BSD distributions (FreeBSD, NetBSD, Darwin/Mac OS X, etc)
>>    Every known Linux distribution
> 
> Could you elaborate on the portability issues you've seen?
> 
> 
> 0: http://www.darwinsys.com/file/
> 

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Burton, Ross]

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

You might be interested in some of the patches I've got sitting in
http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=ross/mutb
specifically around "Add new ELF parser".  This adds a fully-featured
Python ELF parser to lib/oe which could be used to inspect the binaries the
way file does, but without having to call and parse the output of file.

Ross

On 1 December 2017 at 15:50, Olof Johansson <olof.johansson@axis.com> wrote:

> Avoid matching substrings that are picked up from paths, for instance.
> Do this by anchoring the tokens we look for (e.g "executable" or "not
> stripped") with whitespace and punctuation.
>
> Submitted with this patch series is a change that adds the use of
> --brief to file. This removes the path prefix to the output, but the
> path can still be included in shebang lines (which file will report as
> something like "a /foo/bar/baz.py script").
>
> Signed-off-by: Olof Johansson <olofjn@axis.com>
> ---
>  meta/lib/oe/package.py | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py
> index 976d2ef36c..2bd771cfc5 100644
> --- a/meta/lib/oe/package.py
> +++ b/meta/lib/oe/package.py
> @@ -84,17 +84,17 @@ def is_elf(path, on_error=_is_elf_error):
>          error_cb('"file %s" failed')
>          return
>
> -    if not "ELF" in result:
> +    if not result.startswith("ELF "):
>          return 0
>
>      exec_type = 1
> -    if "not stripped" not in result:
> +    if ", not stripped" not in result:
>          exec_type |= 2
> -    if "executable" in result:
> +    if " executable, " in result:
>          exec_type |= 4
> -    if "shared" in result:
> +    if " shared object, " in result:
>          exec_type |= 8
> -    if "relocatable" in result and is_kernel_module(path):
> +    if "relocatable, " in result and is_kernel_module(path):
>          exec_type |= 16
>      return exec_type
>
> --
> 2.11.0
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>

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

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Olof Johansson]

On 17-12-04 12:36 +0000, Burton, Ross wrote:
> You might be interested in some of the patches I've got sitting in
> http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=ross/mutb
> specifically around "Add new ELF parser".  This adds a fully-featured
> Python ELF parser to lib/oe which could be used to inspect the binaries the
> way file does, but without having to call and parse the output of file.

Oh, that's awesome. Please disregard v1 of my patch series, and
I'll try to adapt package.bbclass and lib/oe/package.py to make
use of your changes and see what happens. Thanks!

-- 
olofjn

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Burton, Ross]

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

On 4 December 2017 at 15:30, Olof Johansson <olof.johansson@axis.com> wrote:

> On 17-12-04 12:36 +0000, Burton, Ross wrote:
> > You might be interested in some of the patches I've got sitting in
> > http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=ross/mutb
> > specifically around "Add new ELF parser".  This adds a fully-featured
> > Python ELF parser to lib/oe which could be used to inspect the binaries
> the
> > way file does, but without having to call and parse the output of file.
>
> Oh, that's awesome. Please disregard v1 of my patch series, and
> I'll try to adapt package.bbclass and lib/oe/package.py to make
> use of your changes and see what happens. Thanks!
>

Hopefully to get what you want out of it doesn't involve too much pain...

I still need to finish the series of to get it ready for merging, but I've
just pushed a new branch ross/elf to poky-contrib which has just the
patches.  As I clean it up I'll force push there, so feel free to
track/rebase/send improvements.

Ross

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

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Burton, Ross]

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

If you are looking at this then you'll want to pull my branch again as I
just pushed a few fixes to make ARM binaries actually work.

Also, a prototype "is this binary stripped" function would be:

elf = oe.elf.Elf.from_file(sys.argv[1])
for h in elf.header.section_headers:
    if h.type == oe.elf.Elf.ShType.progbits and h.name == ".debug_info":
        return False
return True

If you're not working on this yet, please say, as I'll probably have a look
this week.

Ross


On 4 December 2017 at 15:33, Burton, Ross <ross.burton@intel.com> wrote:

> On 4 December 2017 at 15:30, Olof Johansson <olof.johansson@axis.com>
> wrote:
>
>> On 17-12-04 12:36 +0000, Burton, Ross wrote:
>> > You might be interested in some of the patches I've got sitting in
>> > http://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=ross/mutb
>> > specifically around "Add new ELF parser".  This adds a fully-featured
>> > Python ELF parser to lib/oe which could be used to inspect the binaries
>> the
>> > way file does, but without having to call and parse the output of file.
>>
>> Oh, that's awesome. Please disregard v1 of my patch series, and
>> I'll try to adapt package.bbclass and lib/oe/package.py to make
>> use of your changes and see what happens. Thanks!
>>
>
> Hopefully to get what you want out of it doesn't involve too much pain...
>
> I still need to finish the series of to get it ready for merging, but I've
> just pushed a new branch ross/elf to poky-contrib which has just the
> patches.  As I clean it up I'll force push there, so feel free to
> track/rebase/send improvements.
>
> Ross
>

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

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Olof Johansson]

On 17-12-06 21:38 +0000, Burton, Ross wrote:
> If you are looking at this then you'll want to pull my branch again as I
> just pushed a few fixes to make ARM binaries actually work.
> 
> Also, a prototype "is this binary stripped" function would be:
> 
> elf = oe.elf.Elf.from_file(sys.argv[1])
> for h in elf.header.section_headers:
>     if h.type == oe.elf.Elf.ShType.progbits and h.name == ".debug_info":
>         return False
> return True
> 
> If you're not working on this yet, please say, as I'll probably have a look
> this week.

I'm so sorry, I missed your email; I am, but maybe not as quickly
as you are able to(?). My is_stripped looks kind of like yours,
but without the ShType.progbits. Thanks for the suggestion! I
were able to hackishly integrate the Elf class it with
staging.bbclass and package.bbclass.

I did have a couple of questions; if I were to add an is_stripped
method, would that fit best in the Elf class itself or in some
wrapper?  Would renaming the elf class to "ElfParser" class make
more sense, and have an ELF class that wraps it? I can publish a
draft somewhere later today.

Thanks your work!
-- 
olofjn

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Burton, Ross]

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

On 18 December 2017 at 11:06, Olof Johansson <olof.johansson@axis.com>
wrote:

> I'm so sorry, I missed your email; I am, but maybe not as quickly
> as you are able to(?). My is_stripped looks kind of like yours,
> but without the ShType.progbits. Thanks for the suggestion! I
> were able to hackishly integrate the Elf class it with
> staging.bbclass and package.bbclass.
>
> I did have a couple of questions; if I were to add an is_stripped
> method, would that fit best in the Elf class itself or in some
> wrapper?  Would renaming the elf class to "ElfParser" class make
> more sense, and have an ELF class that wraps it? I can publish a
> draft somewhere later today.
>

The Elf class is generated with kaitai struct so I'm trying to reduce the
number of changes to it, but we can definitely move it into a subpackage
like oe.parsers.elf and then have oe.elf for just the utility method.

Ross

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

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Burton, Ross]

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

On 18 December 2017 at 11:28, Burton, Ross <ross.burton@intel.com> wrote:

> On 18 December 2017 at 11:06, Olof Johansson <olof.johansson@axis.com>
> wrote:
>
>> I'm so sorry, I missed your email; I am, but maybe not as quickly
>> as you are able to(?). My is_stripped looks kind of like yours,
>> but without the ShType.progbits. Thanks for the suggestion! I
>> were able to hackishly integrate the Elf class it with
>> staging.bbclass and package.bbclass.
>>
>> I did have a couple of questions; if I were to add an is_stripped
>> method, would that fit best in the Elf class itself or in some
>> wrapper?  Would renaming the elf class to "ElfParser" class make
>> more sense, and have an ELF class that wraps it? I can publish a
>> draft somewhere later today.
>>
>
> The Elf class is generated with kaitai struct so I'm trying to reduce the
> number of changes to it, but we can definitely move it into a subpackage
> like oe.parsers.elf and then have oe.elf for just the utility method.
>

Just pushed ross/elf with this change.  The next obvious step is to turn
lib/oe/qa.py into a new lib/oe/elf.py which has a better API and includes
the stripped function.

Ross

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

Re: [PATCH 5/5] lib/oe/package.py: is_elf: Make it less prone to false positives

[Olof Johansson]

On 17-12-18 12:00 +0000, Burton, Ross wrote:
> Just pushed ross/elf with this change.  The next obvious step is to turn
> lib/oe/qa.py into a new lib/oe/elf.py which has a better API and includes
> the stripped function.

Great; I've adapted my changes on top of yours and tested it a
bit. I've published what I have on github for now, if you want to
take a look. Is this along the lines of what you want?

 https://github.com/olof/poky-contrib/compare/olof/elf
 https://github.com/olof/poky-contrib/compare/ross/elf...olof/elf

(Let me know if you prefer me to send it as draft patches to the
list instead.)

(Btw, I didn't know about Kaitai before, but I like the idea of
declarative binary parsing a lot :))

-- 
olofjn