[PATCH 1/3] utils/md5_file: don't iterate line-by-line

[PATCH 1/3] utils/md5_file: don't iterate line-by-line

[Ross Burton]

Opening a file in binary mode and iterating it seems like the simple solution
but will still break on newlines, which for binary files isn't really useful as
the size of the chunks could be huge or tiny.

Instead, let's be a bit more clever: we'll be MD5ing lots of files, but we don't
want to fill up memory: use mmap() to open the file and read the file in 8k
blocks.

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 bitbake/lib/bb/utils.py | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/bitbake/lib/bb/utils.py b/bitbake/lib/bb/utils.py
index 9903183213b..b20cdabcf01 100644
--- a/bitbake/lib/bb/utils.py
+++ b/bitbake/lib/bb/utils.py
@@ -524,12 +524,17 @@ def md5_file(filename):
     """
     Return the hex string representation of the MD5 checksum of filename.
     """
-    import hashlib
-    m = hashlib.md5()
+    import hashlib, mmap
 
     with open(filename, "rb") as f:
-        for line in f:
-            m.update(line)
+        m = hashlib.md5()
+        try:
+            with mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ) as mm:
+                for chunk in iter(lambda: mm.read(8192), b''):
+                    m.update(chunk)
+        except ValueError:
+            # You can't mmap() an empty file so silence this exception
+            pass
     return m.hexdigest()
 
 def sha256_file(filename):
-- 
2.11.0

[PATCH 2/3] checksum: sanity check path when recursively checksumming

[Ross Burton]

In case something goes tragically wrong, catch a request to checksum / and
refuse.

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 bitbake/lib/bb/checksum.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/bitbake/lib/bb/checksum.py b/bitbake/lib/bb/checksum.py
index 84289208f44..4e1598fe835 100644
--- a/bitbake/lib/bb/checksum.py
+++ b/bitbake/lib/bb/checksum.py
@@ -97,6 +97,8 @@ class FileChecksumCache(MultiProcessCache):
 
         def checksum_dir(pth):
             # Handle directories recursively
+            if pth == "/":
+                bb.fatal("Refusing to checksum /")
             dirchecksums = []
             for root, dirs, files in os.walk(pth):
                 for name in files:
-- 
2.11.0

[PATCH 3/3] classes: sanity-check LIC_FILES_CHKSUM

[Ross Burton]

We assume that LIC_FILES_CHKSUM is a file: URI but don't actually verify this,
which can lead to problems if you have a URI that resolves to a path of / as
Bitbake will then dutifully checksum / recursively.

[ YOCTO #12883 ]

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 meta/classes/base.bbclass    | 4 ++--
 meta/classes/license.bbclass | 4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 1a359a04b78..df11c8b270e 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -100,8 +100,8 @@ def get_lic_checksum_file_list(d):
         # We only care about items that are absolute paths since
         # any others should be covered by SRC_URI.
         try:
-            path = bb.fetch.decodeurl(url)[2]
-            if not path:
+            (method, host, path, user, pswd, parm) = bb.fetch.decodeurl(url)
+            if method != "file" or not path:
                 raise bb.fetch.MalformedUrl(url)
 
             if path[0] == '/':
diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
index 2ac15a1bc89..4cf7f074fc2 100644
--- a/meta/classes/license.bbclass
+++ b/meta/classes/license.bbclass
@@ -205,7 +205,9 @@ def find_license_files(d):
 
     for url in lic_files.split():
         try:
-            (type, host, path, user, pswd, parm) = bb.fetch.decodeurl(url)
+            (method, host, path, user, pswd, parm) = bb.fetch.decodeurl(url)
+            if method != "file" or not path:
+                raise bb.fetch.MalformedUrl()
         except bb.fetch.MalformedUrl:
             bb.fatal("%s: LIC_FILES_CHKSUM contains an invalid URL:  %s" % (d.getVar('PF'), url))
         # We want the license filename and path
-- 
2.11.0

✗ patchtest: failure for "utils/md5_file: don't iterate ..." and 2 more

[Patchwork]

== Series Details ==

Series: "utils/md5_file: don't iterate ..." and 2 more
Revision: 1
URL   : https://patchwork.openembedded.org/series/13489/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Series sent to the wrong mailing list or some patches from the series correspond to different mailing lists [test_target_mailing_list] 
  Suggested fix    Send the series again to the correct mailing list (ML)
  Suggested ML     bitbake-devel@lists.openembedded.org [http://git.openembedded.org/bitbake/]
  Patch's path:    bitbake/lib/bb/utils.py

* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at 3d493838f0)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [PATCH 1/3] utils/md5_file: don't iterate line-by-line

[akuster808]

On 08/13/2018 10:20 AM, Ross Burton wrote:
> Opening a file in binary mode and iterating it seems like the simple solution
> but will still break on newlines, which for binary files isn't really useful as
> the size of the chunks could be huge or tiny.
>
> Instead, let's be a bit more clever: we'll be MD5ing lots of files, but we don't
> want to fill up memory: use mmap() to open the file and read the file in 8k
> blocks.
>
> Signed-off-by: Ross Burton <ross.burton@intel.com>

shouldn't this go to the bitbake mailing list ?
> ---
>  bitbake/lib/bb/utils.py | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/bitbake/lib/bb/utils.py b/bitbake/lib/bb/utils.py
> index 9903183213b..b20cdabcf01 100644
> --- a/bitbake/lib/bb/utils.py
> +++ b/bitbake/lib/bb/utils.py
> @@ -524,12 +524,17 @@ def md5_file(filename):
>      """
>      Return the hex string representation of the MD5 checksum of filename.
>      """
> -    import hashlib
> -    m = hashlib.md5()
> +    import hashlib, mmap
>  
>      with open(filename, "rb") as f:
> -        for line in f:
> -            m.update(line)
> +        m = hashlib.md5()
> +        try:
> +            with mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ) as mm:
> +                for chunk in iter(lambda: mm.read(8192), b''):
> +                    m.update(chunk)
> +        except ValueError:
> +            # You can't mmap() an empty file so silence this exception
> +            pass
>      return m.hexdigest()
>  
>  def sha256_file(filename):

Re: [PATCH 1/3] utils/md5_file: don't iterate line-by-line

[Burton, Ross]

Yeah, just sent it there,  sorry

On 13 August 2018 at 19:03, akuster808 <akuster808@gmail.com> wrote:
>
>
> On 08/13/2018 10:20 AM, Ross Burton wrote:
>> Opening a file in binary mode and iterating it seems like the simple solution
>> but will still break on newlines, which for binary files isn't really useful as
>> the size of the chunks could be huge or tiny.
>>
>> Instead, let's be a bit more clever: we'll be MD5ing lots of files, but we don't
>> want to fill up memory: use mmap() to open the file and read the file in 8k
>> blocks.
>>
>> Signed-off-by: Ross Burton <ross.burton@intel.com>
>
> shouldn't this go to the bitbake mailing list ?
>> ---
>>  bitbake/lib/bb/utils.py | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/bitbake/lib/bb/utils.py b/bitbake/lib/bb/utils.py
>> index 9903183213b..b20cdabcf01 100644
>> --- a/bitbake/lib/bb/utils.py
>> +++ b/bitbake/lib/bb/utils.py
>> @@ -524,12 +524,17 @@ def md5_file(filename):
>>      """
>>      Return the hex string representation of the MD5 checksum of filename.
>>      """
>> -    import hashlib
>> -    m = hashlib.md5()
>> +    import hashlib, mmap
>>
>>      with open(filename, "rb") as f:
>> -        for line in f:
>> -            m.update(line)
>> +        m = hashlib.md5()
>> +        try:
>> +            with mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ) as mm:
>> +                for chunk in iter(lambda: mm.read(8192), b''):
>> +                    m.update(chunk)
>> +        except ValueError:
>> +            # You can't mmap() an empty file so silence this exception
>> +            pass
>>      return m.hexdigest()
>>
>>  def sha256_file(filename):
>