[PATCH] lib: zlib: Use post-increment only in inffast.c

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] lib: zlib: Use post-increment only in inffast.c

Tan, Ley Foon
From: Chin Liang See <[hidden email]>

This fixes CVE-2016-9841. Changes integrated from [1], with changes
make for Uboot code base.

An old inffast.c optimization turns out to not be optimal anymore
with modern compilers, and furthermore was not compliant with the
C standard, for which decrementing a pointer before its allocated
memory is undefined. Per the recommendation of a security audit of
the zlib code by Trail of Bits and TrustInSoft, in support of the
Mozilla Foundation, this "optimization" was removed, in order to
avoid the possibility of undefined behavior.

[1]: https://github.com/madler/zlib/commit/9aaec95e82117c1cb0f9624264c3618fc380cecb

Signed-off-by: Mark Adler <[hidden email]>
Signed-off-by: Chin Liang See <[hidden email]>
Signed-off-by: Ley Foon Tan <[hidden email]>
---
 lib/zlib/inffast.c | 87 ++++++++++++++++++----------------------------
 1 file changed, 34 insertions(+), 53 deletions(-)

diff --git a/lib/zlib/inffast.c b/lib/zlib/inffast.c
index e3c7f3b892bb..cdc778eb1d6f 100644
--- a/lib/zlib/inffast.c
+++ b/lib/zlib/inffast.c
@@ -12,25 +12,6 @@
 
 #ifndef ASMINF
 
-/* Allow machine dependent optimization for post-increment or pre-increment.
-   Based on testing to date,
-   Pre-increment preferred for:
-   - PowerPC G3 (Adler)
-   - MIPS R5000 (Randers-Pehrson)
-   Post-increment preferred for:
-   - none
-   No measurable difference:
-   - Pentium III (Anderson)
-   - M68060 (Nikl)
- */
-#ifdef POSTINC
-#  define OFF 0
-#  define PUP(a) *(a)++
-#else
-#  define OFF 1
-#  define PUP(a) *++(a)
-#endif
-
 /*
    Decode literal, length, and distance codes and write out the resulting
    literal and match bytes until either not enough input or output is
@@ -97,7 +78,7 @@ void inflate_fast(z_streamp strm, unsigned start)
 
     /* copy state to local variables */
     state = (struct inflate_state FAR *)strm->state;
-    in = strm->next_in - OFF;
+    in = strm->next_in;
     last = in + (strm->avail_in - 5);
     if (in > last && strm->avail_in > 5) {
         /*
@@ -107,7 +88,7 @@ void inflate_fast(z_streamp strm, unsigned start)
  strm->avail_in = 0xffffffff - (uintptr_t)in;
         last = in + (strm->avail_in - 5);
     }
-    out = strm->next_out - OFF;
+    out = strm->next_out;
     beg = out - (start - strm->avail_out);
     end = out + (strm->avail_out - 257);
 #ifdef INFLATE_STRICT
@@ -128,9 +109,9 @@ void inflate_fast(z_streamp strm, unsigned start)
        input data or output space */
     do {
         if (bits < 15) {
-            hold += (unsigned long)(PUP(in)) << bits;
+            hold += (unsigned long)(*in++) << bits;
             bits += 8;
-            hold += (unsigned long)(PUP(in)) << bits;
+            hold += (unsigned long)(*in++) << bits;
             bits += 8;
         }
         this = lcode[hold & lmask];
@@ -143,14 +124,14 @@ void inflate_fast(z_streamp strm, unsigned start)
             Tracevv((stderr, this.val >= 0x20 && this.val < 0x7f ?
                     "inflate:         literal '%c'\n" :
                     "inflate:         literal 0x%02x\n", this.val));
-            PUP(out) = (unsigned char)(this.val);
+            *out++ = (unsigned char)(this.val);
         }
         else if (op & 16) {                     /* length base */
             len = (unsigned)(this.val);
             op &= 15;                           /* number of extra bits */
             if (op) {
                 if (bits < op) {
-                    hold += (unsigned long)(PUP(in)) << bits;
+                    hold += (unsigned long)(*in++) << bits;
                     bits += 8;
                 }
                 len += (unsigned)hold & ((1U << op) - 1);
@@ -159,9 +140,9 @@ void inflate_fast(z_streamp strm, unsigned start)
             }
             Tracevv((stderr, "inflate:         length %u\n", len));
             if (bits < 15) {
-                hold += (unsigned long)(PUP(in)) << bits;
+                hold += (unsigned long)(*in++) << bits;
                 bits += 8;
-                hold += (unsigned long)(PUP(in)) << bits;
+                hold += (unsigned long)(*in++) << bits;
                 bits += 8;
             }
             this = dcode[hold & dmask];
@@ -174,10 +155,10 @@ void inflate_fast(z_streamp strm, unsigned start)
                 dist = (unsigned)(this.val);
                 op &= 15;                       /* number of extra bits */
                 if (bits < op) {
-                    hold += (unsigned long)(PUP(in)) << bits;
+                    hold += (unsigned long)(*in++) << bits;
                     bits += 8;
                     if (bits < op) {
-                        hold += (unsigned long)(PUP(in)) << bits;
+                        hold += (unsigned long)(*in++) << bits;
                         bits += 8;
                     }
                 }
@@ -200,13 +181,13 @@ void inflate_fast(z_streamp strm, unsigned start)
                         state->mode = BAD;
                         break;
                     }
-                    from = window - OFF;
+                    from = window;
                     if (write == 0) {           /* very common case */
                         from += wsize - op;
                         if (op < len) {         /* some from window */
                             len -= op;
                             do {
-                                PUP(out) = PUP(from);
+                                *out++ = *from++;
                             } while (--op);
                             from = out - dist;  /* rest from output */
                         }
@@ -217,14 +198,14 @@ void inflate_fast(z_streamp strm, unsigned start)
                         if (op < len) {         /* some from end of window */
                             len -= op;
                             do {
-                                PUP(out) = PUP(from);
+                                *out++ = *from++;
                             } while (--op);
-                            from = window - OFF;
+                            from = window;
                             if (write < len) {  /* some from start of window */
                                 op = write;
                                 len -= op;
                                 do {
-                                    PUP(out) = PUP(from);
+                                    *out++ = *from++;
                                 } while (--op);
                                 from = out - dist;      /* rest from output */
                             }
@@ -235,21 +216,21 @@ void inflate_fast(z_streamp strm, unsigned start)
                         if (op < len) {         /* some from window */
                             len -= op;
                             do {
-                                PUP(out) = PUP(from);
+                                *out++ = *from++;
                             } while (--op);
                             from = out - dist;  /* rest from output */
                         }
                     }
                     while (len > 2) {
-                        PUP(out) = PUP(from);
-                        PUP(out) = PUP(from);
-                        PUP(out) = PUP(from);
+                        *out++ = *from++;
+                        *out++ = *from++;
+                        *out++ = *from++;
                         len -= 3;
                     }
                     if (len) {
-                        PUP(out) = PUP(from);
+                        *out++ = *from++;
                         if (len > 1)
-                            PUP(out) = PUP(from);
+                            *out++ = *from++;
                     }
                 }
                 else {
@@ -259,25 +240,25 @@ void inflate_fast(z_streamp strm, unsigned start)
                     from = out - dist;          /* copy direct from output */
                     /* minimum length is three */
     /* Align out addr */
-    if (!((long)(out - 1 + OFF) & 1)) {
- PUP(out) = PUP(from);
+    if (!((long)(out - 1) & 1)) {
+ *out++ = *from++;
  len--;
     }
-    sout = (unsigned short *)(out - OFF);
+    sout = (unsigned short *)(out);
     if (dist > 2 ) {
  unsigned short *sfrom;
 
- sfrom = (unsigned short *)(from - OFF);
+ sfrom = (unsigned short *)(from);
  loops = len >> 1;
  do
-    PUP(sout) = get_unaligned(++sfrom);
+    *sout++ = get_unaligned(++sfrom);
  while (--loops);
- out = (unsigned char *)sout + OFF;
- from = (unsigned char *)sfrom + OFF;
+ out = (unsigned char *)sout;
+ from = (unsigned char *)sfrom;
     } else { /* dist == 1 or dist == 2 */
  unsigned short pat16;
 
- pat16 = *(sout-2+2*OFF);
+ pat16 = *(sout-2);
  if (dist == 1)
 #if defined(__BIG_ENDIAN)
     pat16 = (pat16 & 0xff) | ((pat16 & 0xff ) << 8);
@@ -288,12 +269,12 @@ void inflate_fast(z_streamp strm, unsigned start)
 #endif
  loops = len >> 1;
  do
-    PUP(sout) = pat16;
+    *sout++ = pat16;
  while (--loops);
- out = (unsigned char *)sout + OFF;
+ out = (unsigned char *)sout;
     }
     if (len & 1)
- PUP(out) = PUP(from);
+ *out++ = *from++;
                 }
             }
             else if ((op & 64) == 0) {          /* 2nd level distance code */
@@ -329,8 +310,8 @@ void inflate_fast(z_streamp strm, unsigned start)
     hold &= (1U << bits) - 1;
 
     /* update state and return */
-    strm->next_in = in + OFF;
-    strm->next_out = out + OFF;
+    strm->next_in = in;
+    strm->next_out = out;
     strm->avail_in = (unsigned)(in < last ? 5 + (last - in) : 5 - (in - last));
     strm->avail_out = (unsigned)(out < end ?
                                  257 + (end - out) : 257 - (out - end));
--
2.19.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] lib: zlib: Use post-increment only in inffast.c

Tom Rini-4
On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote:

> From: Chin Liang See <[hidden email]>
>
> This fixes CVE-2016-9841. Changes integrated from [1], with changes
> make for Uboot code base.
>
> An old inffast.c optimization turns out to not be optimal anymore
> with modern compilers, and furthermore was not compliant with the
> C standard, for which decrementing a pointer before its allocated
> memory is undefined. Per the recommendation of a security audit of
> the zlib code by Trail of Bits and TrustInSoft, in support of the
> Mozilla Foundation, this "optimization" was removed, in order to
> avoid the possibility of undefined behavior.
>
> [1]: https://github.com/madler/zlib/commit/9aaec95e82117c1cb0f9624264c3618fc380cecb
>
> Signed-off-by: Mark Adler <[hidden email]>
> Signed-off-by: Chin Liang See <[hidden email]>
> Signed-off-by: Ley Foon Tan <[hidden email]>
This breaks the following tests on sandbox:
FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch - u_boot_spawn.Timeout
FAILED test/py/tests/test_fit.py::test_fit - OSError: [Errno 5] Input/output error

--
Tom

signature.asc (673 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] lib: zlib: Use post-increment only in inffast.c

Ley Foon Tan-2
On Fri, Jul 17, 2020 at 9:29 PM Tom Rini <[hidden email]> wrote:

>
> On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote:
>
> > From: Chin Liang See <[hidden email]>
> >
> > This fixes CVE-2016-9841. Changes integrated from [1], with changes
> > make for Uboot code base.
> >
> > An old inffast.c optimization turns out to not be optimal anymore
> > with modern compilers, and furthermore was not compliant with the
> > C standard, for which decrementing a pointer before its allocated
> > memory is undefined. Per the recommendation of a security audit of
> > the zlib code by Trail of Bits and TrustInSoft, in support of the
> > Mozilla Foundation, this "optimization" was removed, in order to
> > avoid the possibility of undefined behavior.
> >
> > [1]: https://github.com/madler/zlib/commit/9aaec95e82117c1cb0f9624264c3618fc380cecb
> >
> > Signed-off-by: Mark Adler <[hidden email]>
> > Signed-off-by: Chin Liang See <[hidden email]>
> > Signed-off-by: Ley Foon Tan <[hidden email]>
>
> This breaks the following tests on sandbox:
> FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch - u_boot_spawn.Timeout
> FAILED test/py/tests/test_fit.py::test_fit - OSError: [Errno 5] Input/output error
>
Hi Tom

I have tried to run the sandtest, but it failed in different test
cases. I am run this command "./test/py/test.py --bd sandbox --build".
Error log at bottom of email.

Found that https://gitlab.denx.de/u-boot/u-boot/-/blob/master/lib/zlib/zlib.h
always "#undef POSTINC", it is mean that U-boot can only support
pre-increment? I have tried changing "#undef POSTINC" to "define
POSTINC" and without this patch, the test failed at the same location.
So, the failure is not caused by this patch.
Note, this patch mainly changes to support post-increment only.

Any suggestion to fix this?

Regards
Ley Foon

-----------------
test/py/tests/test_fs/test_squashfs/test_sqfs_load.py F
                                        [  0%]
test/py/tests/test_fs/test_squashfs/test_sqfs_ls.py F

====================================================== FAILURES
======================================================
___________________________________________________ test_sqfs_load
___________________________________________________

u_boot_console = <u_boot_console_sandbox.ConsoleSandbox object at
0x7f788515dd60>

    @pytest.mark.boardspec('sandbox')
    @pytest.mark.buildconfigspec('cmd_fs_generic')
    @pytest.mark.buildconfigspec('cmd_squashfs')
    @pytest.mark.buildconfigspec('fs_squashfs')
    @pytest.mark.requiredtool('mksquashfs')
    def test_sqfs_load(u_boot_console):
        build_dir = u_boot_console.config.build_dir
        command = "sqfsload host 0 $kernel_addr_r "

        for opt in comp_opts:
            # generate and load the squashfs image
            try:
                opt.gen_image(build_dir)
            except RuntimeError:
                opt.clean_source(build_dir)
                # skip unsupported compression types
                continue

            path = os.path.join(build_dir, "sqfs-" + opt.name)
            output = u_boot_console.run_command("host bind 0 " + path)

>           output = u_boot_console.run_command(command + "xxx")

test/py/tests/test_fs/test_squashfs/test_sqfs_load.py:30:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
test/py/u_boot_console_base.py:205: in run_command
    m = self.p.expect([self.prompt_compiled] + self.bad_patterns)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <u_boot_spawn.Spawn object at 0x7f7884ba9d00>
patterns = [re.compile('^=>\\ ', re.MULTILINE), re.compile('(U-Boot
SPL \\d{4}\\.\\d{2}[^\r\n]*\\))'), re.compile('(U-Boot SPL
\\...d{2}[^\r\n]*\\))'), re.compile('Hit any key to stop autoboot: '),
re.compile("Unknown command '.*' - try 'help'"), ...]

    def expect(self, patterns):
        """Wait for the sub-process to emit specific data.

        This function waits for the process to emit one pattern from the
        supplied list of patterns, or for a timeout to occur.

        Args:
            patterns: A list of strings or regex objects that we expect to
                see in the sub-process' stdout.

        Returns:
            The index within the patterns array of the pattern the process
            emitted.

        Notable exceptions:
            Timeout, if the process did not emit any of the patterns within
            the expected time.
        """

        for pi in range(len(patterns)):
            if type(patterns[pi]) == type(''):
                patterns[pi] = re.compile(patterns[pi])

        tstart_s = time.time()
        try:
            while True:
                earliest_m = None
                earliest_pi = None
                for pi in range(len(patterns)):
                    pattern = patterns[pi]
                    m = pattern.search(self.buf)
                    if not m:
                        continue
                    if earliest_m and m.start() >= earliest_m.start():
                        continue
                    earliest_m = m
                    earliest_pi = pi
                if earliest_m:
                    pos = earliest_m.start()
                    posafter = earliest_m.end()
                    self.before = self.buf[:pos]
                    self.after = self.buf[pos:posafter]
                    self.output += self.buf[:posafter]
                    self.buf = self.buf[posafter:]
                    return earliest_pi
                tnow_s = time.time()
                if self.timeout:
                    tdelta_ms = (tnow_s - tstart_s) * 1000
                    poll_maxwait = self.timeout - tdelta_ms
                    if tdelta_ms > self.timeout:
                        raise Timeout()
                else:
                    poll_maxwait = None
                events = self.poll.poll(poll_maxwait)
                if not events:
                    raise Timeout()
>               c = os.read(self.fd, 1024).decode(errors='replace')
E               OSError: [Errno 5] Input/output error

test/py/u_boot_spawn.py:171: OSError
------------------------------------------------ Captured stdout call
------------------------------------------------
Parallel mksquashfs: Using 1 processor
Creating 4.0 filesystem on
/home/lftan/git/u-boot/build-sandbox/sqfs-gzip, block size 4096.
[===================================================================|] 4/4 100%

Exportable Squashfs 4.0 filesystem, gzip compressed, data block size 4096
    compressed data, compressed metadata, compressed fragments,
    compressed xattrs, compressed ids
    duplicates are removed
Filesystem size 6.97 Kbytes (0.01 Mbytes)
    73.37% of uncompressed filesystem size (9.50 Kbytes)
Inode table size 98 bytes (0.10 Kbytes)
    57.31% of uncompressed inode table size (171 bytes)
Directory table size 55 bytes (0.05 Kbytes)
    72.37% of uncompressed directory table size (76 bytes)
Number of duplicate files found 0
Number of inodes 5
Number of files 3
Number of fragments 1
Number of symbolic links  1
Number of device nodes 0
Number of fifo nodes 0
Number of socket nodes 0
Number of directories 1
Number of ids (unique uids + gids) 1
Number of uids 1
    lftan (1000)
Number of gids 1
    lftan (1000)
=> host bind 0 /home/lftan/git/u-boot/build-sandbox/sqfs-gzip
=> => sqfsload host 0 $kernel_addr_r xxx
Error: corrupted compressed data.
____________________________________________________ test_sqfs_ls
____________________________________________________

u_boot_console = <u_boot_console_sandbox.ConsoleSandbox object at
0x7f788515dd60>

    @pytest.mark.boardspec('sandbox')
    @pytest.mark.buildconfigspec('cmd_fs_generic')
    @pytest.mark.buildconfigspec('cmd_squashfs')
    @pytest.mark.buildconfigspec('fs_squashfs')
    @pytest.mark.requiredtool('mksquashfs')
    def test_sqfs_ls(u_boot_console):
        build_dir = u_boot_console.config.build_dir
        for opt in comp_opts:
            try:
>               opt.gen_image(build_dir)

test/py/tests/test_fs/test_squashfs/test_sqfs_ls.py:18:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <sqfs_common.Compression object at 0x7f7884c986a0>, build_dir =
'/home/lftan/git/u-boot/build-sandbox'

    def gen_image(self, build_dir):
        src = os.path.join(build_dir, "sqfs_src/")
>       os.mkdir(src)
E       FileExistsError: [Errno 17] File exists:
'/home/lftan/git/u-boot/build-sandbox/sqfs_src/'

test/py/tests/test_fs/test_squashfs/sqfs_common.py:35: FileExistsError
----------------------------------------------- Captured stdout setup
------------------------------------------------
/u-boot


U-Boot 2020.10-00718-g0f35d96bfd-dirty (Oct 16 2020 - 04:30:45 +0800)

Model: sandbox
DRAM:  128 MiB
WDT:   Started with servicing (60s timeout)
MMC:   mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD)
In:    serial
Out:   vidconsole
Err:   vidconsole
Model: sandbox
SCSI:
Net:   eth0: eth@10002000, eth5: eth@10003000, eth3: sbe5, eth6: eth@10004000
Hit any key to stop autoboot:  0
=>
=============================== 2 failed, 641 passed, 88 skipped in
113.21s (0:01:53) ================================
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] lib: zlib: Use post-increment only in inffast.c

Tom Rini-4
On Fri, Oct 16, 2020 at 04:40:00AM +0800, Ley Foon Tan wrote:

> On Fri, Jul 17, 2020 at 9:29 PM Tom Rini <[hidden email]> wrote:
> >
> > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote:
> >
> > > From: Chin Liang See <[hidden email]>
> > >
> > > This fixes CVE-2016-9841. Changes integrated from [1], with changes
> > > make for Uboot code base.
> > >
> > > An old inffast.c optimization turns out to not be optimal anymore
> > > with modern compilers, and furthermore was not compliant with the
> > > C standard, for which decrementing a pointer before its allocated
> > > memory is undefined. Per the recommendation of a security audit of
> > > the zlib code by Trail of Bits and TrustInSoft, in support of the
> > > Mozilla Foundation, this "optimization" was removed, in order to
> > > avoid the possibility of undefined behavior.
> > >
> > > [1]: https://github.com/madler/zlib/commit/9aaec95e82117c1cb0f9624264c3618fc380cecb
> > >
> > > Signed-off-by: Mark Adler <[hidden email]>
> > > Signed-off-by: Chin Liang See <[hidden email]>
> > > Signed-off-by: Ley Foon Tan <[hidden email]>
> >
> > This breaks the following tests on sandbox:
> > FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch - u_boot_spawn.Timeout
> > FAILED test/py/tests/test_fit.py::test_fit - OSError: [Errno 5] Input/output error
> >
> Hi Tom
>
> I have tried to run the sandtest, but it failed in different test
> cases. I am run this command "./test/py/test.py --bd sandbox --build".
> Error log at bottom of email.
>
> Found that https://gitlab.denx.de/u-boot/u-boot/-/blob/master/lib/zlib/zlib.h
> always "#undef POSTINC", it is mean that U-boot can only support
> pre-increment? I have tried changing "#undef POSTINC" to "define
> POSTINC" and without this patch, the test failed at the same location.
> So, the failure is not caused by this patch.
> Note, this patch mainly changes to support post-increment only.
>
> Any suggestion to fix this?
I'm not sure why the tests fail for you to start with.  They all pass
inn the CI environment as well as locally.  I would start by seeing how
your environment differs from those.

--
Tom

signature.asc (673 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] lib: zlib: Use post-increment only in inffast.c

Tan, Ley Foon


> -----Original Message-----
> From: Tom Rini <[hidden email]>
> Sent: Friday, October 16, 2020 8:37 PM
> To: Ley Foon Tan <[hidden email]>
> Cc: Tan, Ley Foon <[hidden email]>; ZY - u-boot <u-
> [hidden email]>; See, Chin Liang <[hidden email]>
> Subject: Re: [PATCH] lib: zlib: Use post-increment only in inffast.c
>
> On Fri, Oct 16, 2020 at 04:40:00AM +0800, Ley Foon Tan wrote:
> > On Fri, Jul 17, 2020 at 9:29 PM Tom Rini <[hidden email]> wrote:
> > >
> > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote:
> > >
> > > > From: Chin Liang See <[hidden email]>
> > > >
> > > > This fixes CVE-2016-9841. Changes integrated from [1], with
> > > > changes make for Uboot code base.
> > > >
> > > > An old inffast.c optimization turns out to not be optimal anymore
> > > > with modern compilers, and furthermore was not compliant with the
> > > > C standard, for which decrementing a pointer before its allocated
> > > > memory is undefined. Per the recommendation of a security audit of
> > > > the zlib code by Trail of Bits and TrustInSoft, in support of the
> > > > Mozilla Foundation, this "optimization" was removed, in order to
> > > > avoid the possibility of undefined behavior.
> > > >
> > > > [1]:
> > > >
> https://github.com/madler/zlib/commit/9aaec95e82117c1cb0f9624264c3
> > > > 618fc380cecb
> > > >
> > > > Signed-off-by: Mark Adler <[hidden email]>
> > > > Signed-off-by: Chin Liang See <[hidden email]>
> > > > Signed-off-by: Ley Foon Tan <[hidden email]>
> > >
> > > This breaks the following tests on sandbox:
> > > FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch -
> > > u_boot_spawn.Timeout FAILED test/py/tests/test_fit.py::test_fit -
> > > OSError: [Errno 5] Input/output error
> > >
> > Hi Tom
> >
> > I have tried to run the sandtest, but it failed in different test
> > cases. I am run this command "./test/py/test.py --bd sandbox --build".
> > Error log at bottom of email.
> >
> > Found that
> > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/lib/zlib/zlib.h
> > always "#undef POSTINC", it is mean that U-boot can only support
> > pre-increment? I have tried changing "#undef POSTINC" to "define
> > POSTINC" and without this patch, the test failed at the same location.
> > So, the failure is not caused by this patch.
> > Note, this patch mainly changes to support post-increment only.
> >
> > Any suggestion to fix this?
>
> I'm not sure why the tests fail for you to start with.  They all pass inn the CI
> environment as well as locally.  I would start by seeing how your
> environment differs from those.

Hi Tom

My test is running on Ubuntu 20.04 Linux.

Can you try change "#undef POSTINC" to "#define POSTINC" in lib/zlib/zlib.h if you can see the error?

Regards
Ley Foon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] lib: zlib: Use post-increment only in inffast.c

Tom Rini-4
On Wed, Oct 21, 2020 at 03:42:10AM +0000, Tan, Ley Foon wrote:

>
>
> > -----Original Message-----
> > From: Tom Rini <[hidden email]>
> > Sent: Friday, October 16, 2020 8:37 PM
> > To: Ley Foon Tan <[hidden email]>
> > Cc: Tan, Ley Foon <[hidden email]>; ZY - u-boot <u-
> > [hidden email]>; See, Chin Liang <[hidden email]>
> > Subject: Re: [PATCH] lib: zlib: Use post-increment only in inffast.c
> >
> > On Fri, Oct 16, 2020 at 04:40:00AM +0800, Ley Foon Tan wrote:
> > > On Fri, Jul 17, 2020 at 9:29 PM Tom Rini <[hidden email]> wrote:
> > > >
> > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote:
> > > >
> > > > > From: Chin Liang See <[hidden email]>
> > > > >
> > > > > This fixes CVE-2016-9841. Changes integrated from [1], with
> > > > > changes make for Uboot code base.
> > > > >
> > > > > An old inffast.c optimization turns out to not be optimal anymore
> > > > > with modern compilers, and furthermore was not compliant with the
> > > > > C standard, for which decrementing a pointer before its allocated
> > > > > memory is undefined. Per the recommendation of a security audit of
> > > > > the zlib code by Trail of Bits and TrustInSoft, in support of the
> > > > > Mozilla Foundation, this "optimization" was removed, in order to
> > > > > avoid the possibility of undefined behavior.
> > > > >
> > > > > [1]:
> > > > >
> > https://github.com/madler/zlib/commit/9aaec95e82117c1cb0f9624264c3
> > > > > 618fc380cecb
> > > > >
> > > > > Signed-off-by: Mark Adler <[hidden email]>
> > > > > Signed-off-by: Chin Liang See <[hidden email]>
> > > > > Signed-off-by: Ley Foon Tan <[hidden email]>
> > > >
> > > > This breaks the following tests on sandbox:
> > > > FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch -
> > > > u_boot_spawn.Timeout FAILED test/py/tests/test_fit.py::test_fit -
> > > > OSError: [Errno 5] Input/output error
> > > >
> > > Hi Tom
> > >
> > > I have tried to run the sandtest, but it failed in different test
> > > cases. I am run this command "./test/py/test.py --bd sandbox --build".
> > > Error log at bottom of email.
> > >
> > > Found that
> > > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/lib/zlib/zlib.h
> > > always "#undef POSTINC", it is mean that U-boot can only support
> > > pre-increment? I have tried changing "#undef POSTINC" to "define
> > > POSTINC" and without this patch, the test failed at the same location.
> > > So, the failure is not caused by this patch.
> > > Note, this patch mainly changes to support post-increment only.
> > >
> > > Any suggestion to fix this?
> >
> > I'm not sure why the tests fail for you to start with.  They all pass inn the CI
> > environment as well as locally.  I would start by seeing how your
> > environment differs from those.
>
> Hi Tom
>
> My test is running on Ubuntu 20.04 Linux.
>
> Can you try change "#undef POSTINC" to "#define POSTINC" in lib/zlib/zlib.h if you can see the error?
I see:
FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch - assert
'Hello, world' in "## Loa...
FAILED test/py/tests/test_fit.py::test_fit - OSError: [Errno 5]
Input/output error

as new problems with that change.

--
Tom

signature.asc (673 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] lib: zlib: Use post-increment only in inffast.c

Tan, Ley Foon


> -----Original Message-----
> From: Tom Rini <[hidden email]>
> Sent: Thursday, October 22, 2020 9:24 PM
> To: Tan, Ley Foon <[hidden email]>
> Cc: Ley Foon Tan <[hidden email]>; ZY - u-boot <u-
> [hidden email]>; See, Chin Liang <[hidden email]>
> Subject: Re: [PATCH] lib: zlib: Use post-increment only in inffast.c
>
> On Wed, Oct 21, 2020 at 03:42:10AM +0000, Tan, Ley Foon wrote:
> >
> >
> > > -----Original Message-----
> > > From: Tom Rini <[hidden email]>
> > > Sent: Friday, October 16, 2020 8:37 PM
> > > To: Ley Foon Tan <[hidden email]>
> > > Cc: Tan, Ley Foon <[hidden email]>; ZY - u-boot <u-
> > > [hidden email]>; See, Chin Liang <[hidden email]>
> > > Subject: Re: [PATCH] lib: zlib: Use post-increment only in inffast.c
> > >
> > > On Fri, Oct 16, 2020 at 04:40:00AM +0800, Ley Foon Tan wrote:
> > > > On Fri, Jul 17, 2020 at 9:29 PM Tom Rini <[hidden email]> wrote:
> > > > >
> > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote:
> > > > >
> > > > > > From: Chin Liang See <[hidden email]>
> > > > > >
> > > > > > This fixes CVE-2016-9841. Changes integrated from [1], with
> > > > > > changes make for Uboot code base.
> > > > > >
> > > > > > An old inffast.c optimization turns out to not be optimal
> > > > > > anymore with modern compilers, and furthermore was not
> > > > > > compliant with the C standard, for which decrementing a
> > > > > > pointer before its allocated memory is undefined. Per the
> > > > > > recommendation of a security audit of the zlib code by Trail
> > > > > > of Bits and TrustInSoft, in support of the Mozilla Foundation,
> > > > > > this "optimization" was removed, in order to avoid the possibility of
> undefined behavior.
> > > > > >
> > > > > > [1]:
> > > > > >
> > >
> https://github.com/madler/zlib/commit/9aaec95e82117c1cb0f9624264c3
> > > > > > 618fc380cecb
> > > > > >
> > > > > > Signed-off-by: Mark Adler <[hidden email]>
> > > > > > Signed-off-by: Chin Liang See <[hidden email]>
> > > > > > Signed-off-by: Ley Foon Tan <[hidden email]>
> > > > >
> > > > > This breaks the following tests on sandbox:
> > > > > FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch -
> > > > > u_boot_spawn.Timeout FAILED test/py/tests/test_fit.py::test_fit
> > > > > -
> > > > > OSError: [Errno 5] Input/output error
> > > > >
> > > > Hi Tom
> > > >
> > > > I have tried to run the sandtest, but it failed in different test
> > > > cases. I am run this command "./test/py/test.py --bd sandbox --build".
> > > > Error log at bottom of email.
> > > >
> > > > Found that
> > > > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/lib/zlib/zlib.h
> > > > always "#undef POSTINC", it is mean that U-boot can only support
> > > > pre-increment? I have tried changing "#undef POSTINC" to "define
> > > > POSTINC" and without this patch, the test failed at the same location.
> > > > So, the failure is not caused by this patch.
> > > > Note, this patch mainly changes to support post-increment only.
> > > >
> > > > Any suggestion to fix this?
> > >
> > > I'm not sure why the tests fail for you to start with.  They all
> > > pass inn the CI environment as well as locally.  I would start by
> > > seeing how your environment differs from those.
> >
> > Hi Tom
> >
> > My test is running on Ubuntu 20.04 Linux.
> >
> > Can you try change "#undef POSTINC" to "#define POSTINC" in
> lib/zlib/zlib.h if you can see the error?
>
> I see:
> FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch - assert 'Hello,
> world' in "## Loa...
> FAILED test/py/tests/test_fit.py::test_fit - OSError: [Errno 5] Input/output
> error
>
> as new problems with that change.
Just to confirm, this error is with this patch or change to "#define POSTINC"?

Yours errors are different from my run.

My run failed in these 2 testcases:
test/py/tests/test_fs/test_squashfs/test_sqfs_load.py F
test/py/tests/test_fs/test_squashfs/test_sqfs_ls.py F

Regards
Ley Foon

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] lib: zlib: Use post-increment only in inffast.c

Tom Rini-4
On Fri, Oct 23, 2020 at 01:41:57AM +0000, Tan, Ley Foon wrote:

>
>
> > -----Original Message-----
> > From: Tom Rini <[hidden email]>
> > Sent: Thursday, October 22, 2020 9:24 PM
> > To: Tan, Ley Foon <[hidden email]>
> > Cc: Ley Foon Tan <[hidden email]>; ZY - u-boot <u-
> > [hidden email]>; See, Chin Liang <[hidden email]>
> > Subject: Re: [PATCH] lib: zlib: Use post-increment only in inffast.c
> >
> > On Wed, Oct 21, 2020 at 03:42:10AM +0000, Tan, Ley Foon wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Tom Rini <[hidden email]>
> > > > Sent: Friday, October 16, 2020 8:37 PM
> > > > To: Ley Foon Tan <[hidden email]>
> > > > Cc: Tan, Ley Foon <[hidden email]>; ZY - u-boot <u-
> > > > [hidden email]>; See, Chin Liang <[hidden email]>
> > > > Subject: Re: [PATCH] lib: zlib: Use post-increment only in inffast.c
> > > >
> > > > On Fri, Oct 16, 2020 at 04:40:00AM +0800, Ley Foon Tan wrote:
> > > > > On Fri, Jul 17, 2020 at 9:29 PM Tom Rini <[hidden email]> wrote:
> > > > > >
> > > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote:
> > > > > >
> > > > > > > From: Chin Liang See <[hidden email]>
> > > > > > >
> > > > > > > This fixes CVE-2016-9841. Changes integrated from [1], with
> > > > > > > changes make for Uboot code base.
> > > > > > >
> > > > > > > An old inffast.c optimization turns out to not be optimal
> > > > > > > anymore with modern compilers, and furthermore was not
> > > > > > > compliant with the C standard, for which decrementing a
> > > > > > > pointer before its allocated memory is undefined. Per the
> > > > > > > recommendation of a security audit of the zlib code by Trail
> > > > > > > of Bits and TrustInSoft, in support of the Mozilla Foundation,
> > > > > > > this "optimization" was removed, in order to avoid the possibility of
> > undefined behavior.
> > > > > > >
> > > > > > > [1]:
> > > > > > >
> > > >
> > https://github.com/madler/zlib/commit/9aaec95e82117c1cb0f9624264c3
> > > > > > > 618fc380cecb
> > > > > > >
> > > > > > > Signed-off-by: Mark Adler <[hidden email]>
> > > > > > > Signed-off-by: Chin Liang See <[hidden email]>
> > > > > > > Signed-off-by: Ley Foon Tan <[hidden email]>
> > > > > >
> > > > > > This breaks the following tests on sandbox:
> > > > > > FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch -
> > > > > > u_boot_spawn.Timeout FAILED test/py/tests/test_fit.py::test_fit
> > > > > > -
> > > > > > OSError: [Errno 5] Input/output error
> > > > > >
> > > > > Hi Tom
> > > > >
> > > > > I have tried to run the sandtest, but it failed in different test
> > > > > cases. I am run this command "./test/py/test.py --bd sandbox --build".
> > > > > Error log at bottom of email.
> > > > >
> > > > > Found that
> > > > > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/lib/zlib/zlib.h
> > > > > always "#undef POSTINC", it is mean that U-boot can only support
> > > > > pre-increment? I have tried changing "#undef POSTINC" to "define
> > > > > POSTINC" and without this patch, the test failed at the same location.
> > > > > So, the failure is not caused by this patch.
> > > > > Note, this patch mainly changes to support post-increment only.
> > > > >
> > > > > Any suggestion to fix this?
> > > >
> > > > I'm not sure why the tests fail for you to start with.  They all
> > > > pass inn the CI environment as well as locally.  I would start by
> > > > seeing how your environment differs from those.
> > >
> > > Hi Tom
> > >
> > > My test is running on Ubuntu 20.04 Linux.
> > >
> > > Can you try change "#undef POSTINC" to "#define POSTINC" in
> > lib/zlib/zlib.h if you can see the error?
> >
> > I see:
> > FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch - assert 'Hello,
> > world' in "## Loa...
> > FAILED test/py/tests/test_fit.py::test_fit - OSError: [Errno 5] Input/output
> > error
> >
> > as new problems with that change.
> Just to confirm, this error is with this patch or change to "#define POSTINC"?
>
> Yours errors are different from my run.
>
> My run failed in these 2 testcases:
> test/py/tests/test_fs/test_squashfs/test_sqfs_load.py F
> test/py/tests/test_fs/test_squashfs/test_sqfs_ls.py F
The squashfs tests can fail if they ran before, sometimes and I think
that means we need a clean-up action in them.  Delete the .bm-work
directory and re-run.  I see the errors I reported when I changed the
define, as you asked.  Originally, I saw the errors I reported in the CI
systems, with the patch in question.

--
Tom

signature.asc (673 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RE: [PATCH] lib: zlib: Use post-increment only in inffast.c

Tan, Ley Foon


> -----Original Message-----
> From: Tom Rini <[hidden email]>
> Sent: Friday, October 23, 2020 9:52 AM
> To: Tan, Ley Foon <[hidden email]>
> Cc: Ley Foon Tan <[hidden email]>; ZY - u-boot <u-
> [hidden email]>; See, Chin Liang <[hidden email]>
> Subject: Re: [PATCH] lib: zlib: Use post-increment only in inffast.c
>
> On Fri, Oct 23, 2020 at 01:41:57AM +0000, Tan, Ley Foon wrote:
> >
> >
> > > -----Original Message-----
> > > From: Tom Rini <[hidden email]>
> > > Sent: Thursday, October 22, 2020 9:24 PM
> > > To: Tan, Ley Foon <[hidden email]>
> > > Cc: Ley Foon Tan <[hidden email]>; ZY - u-boot <u-
> > > [hidden email]>; See, Chin Liang <[hidden email]>
> > > Subject: Re: [PATCH] lib: zlib: Use post-increment only in inffast.c
> > >
> > > On Wed, Oct 21, 2020 at 03:42:10AM +0000, Tan, Ley Foon wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Tom Rini <[hidden email]>
> > > > > Sent: Friday, October 16, 2020 8:37 PM
> > > > > To: Ley Foon Tan <[hidden email]>
> > > > > Cc: Tan, Ley Foon <[hidden email]>; ZY - u-boot <u-
> > > > > [hidden email]>; See, Chin Liang <[hidden email]>
> > > > > Subject: Re: [PATCH] lib: zlib: Use post-increment only in
> > > > > inffast.c
> > > > >
> > > > > On Fri, Oct 16, 2020 at 04:40:00AM +0800, Ley Foon Tan wrote:
> > > > > > On Fri, Jul 17, 2020 at 9:29 PM Tom Rini <[hidden email]>
> wrote:
> > > > > > >
> > > > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote:
> > > > > > >
> > > > > > > > From: Chin Liang See <[hidden email]>
> > > > > > > >
> > > > > > > > This fixes CVE-2016-9841. Changes integrated from [1],
> > > > > > > > with changes make for Uboot code base.
> > > > > > > >
> > > > > > > > An old inffast.c optimization turns out to not be optimal
> > > > > > > > anymore with modern compilers, and furthermore was not
> > > > > > > > compliant with the C standard, for which decrementing a
> > > > > > > > pointer before its allocated memory is undefined. Per the
> > > > > > > > recommendation of a security audit of the zlib code by
> > > > > > > > Trail of Bits and TrustInSoft, in support of the Mozilla
> > > > > > > > Foundation, this "optimization" was removed, in order to
> > > > > > > > avoid the possibility of
> > > undefined behavior.
> > > > > > > >
> > > > > > > > [1]:
> > > > > > > >
> > > > >
> > >
> https://github.com/madler/zlib/commit/9aaec95e82117c1cb0f9624264c3
> > > > > > > > 618fc380cecb
> > > > > > > >
> > > > > > > > Signed-off-by: Mark Adler <[hidden email]>
> > > > > > > > Signed-off-by: Chin Liang See <[hidden email]>
> > > > > > > > Signed-off-by: Ley Foon Tan <[hidden email]>
> > > > > > >
> > > > > > > This breaks the following tests on sandbox:
> > > > > > > FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch -
> > > > > > > u_boot_spawn.Timeout FAILED
> > > > > > > test/py/tests/test_fit.py::test_fit
> > > > > > > -
> > > > > > > OSError: [Errno 5] Input/output error
> > > > > > >
> > > > > > Hi Tom
> > > > > >
> > > > > > I have tried to run the sandtest, but it failed in different
> > > > > > test cases. I am run this command "./test/py/test.py --bd sandbox --
> build".
> > > > > > Error log at bottom of email.
> > > > > >
> > > > > > Found that
> > > > > > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/lib/zlib/zl
> > > > > > ib.h always "#undef POSTINC", it is mean that U-boot can only
> > > > > > support pre-increment? I have tried changing "#undef POSTINC"
> > > > > > to "define POSTINC" and without this patch, the test failed at
> > > > > > the same location.
> > > > > > So, the failure is not caused by this patch.
> > > > > > Note, this patch mainly changes to support post-increment only.
> > > > > >
> > > > > > Any suggestion to fix this?
> > > > >
> > > > > I'm not sure why the tests fail for you to start with.  They all
> > > > > pass inn the CI environment as well as locally.  I would start
> > > > > by seeing how your environment differs from those.
> > > >
> > > > Hi Tom
> > > >
> > > > My test is running on Ubuntu 20.04 Linux.
> > > >
> > > > Can you try change "#undef POSTINC" to "#define POSTINC" in
> > > lib/zlib/zlib.h if you can see the error?
> > >
> > > I see:
> > > FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch - assert
> > > 'Hello, world' in "## Loa...
> > > FAILED test/py/tests/test_fit.py::test_fit - OSError: [Errno 5]
> > > Input/output error
> > >
> > > as new problems with that change.
> > Just to confirm, this error is with this patch or change to "#define POSTINC"?
> >
> > Yours errors are different from my run.
> >
> > My run failed in these 2 testcases:
> > test/py/tests/test_fs/test_squashfs/test_sqfs_load.py F
> > test/py/tests/test_fs/test_squashfs/test_sqfs_ls.py F
>
> The squashfs tests can fail if they ran before, sometimes and I think that
> means we need a clean-up action in them.  Delete the .bm-work directory
> and re-run.  I see the errors I reported when I changed the define, as you
> asked.  Originally, I saw the errors I reported in the CI systems, with the
> patch in question.

Yes, I also found delete .bm-work directory can solve squashfs test error. But, enable POSTINC still can see error even  clean the -work directory.

So, this can confirm the test error not caused by this patch.

The root cause is in test code or U-boot code?

Regards
Ley Foon
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] lib: zlib: Use post-increment only in inffast.c

Tom Rini-4
On Fri, Oct 23, 2020 at 01:59:29AM +0000, Tan, Ley Foon wrote:

>
>
> > -----Original Message-----
> > From: Tom Rini <[hidden email]>
> > Sent: Friday, October 23, 2020 9:52 AM
> > To: Tan, Ley Foon <[hidden email]>
> > Cc: Ley Foon Tan <[hidden email]>; ZY - u-boot <u-
> > [hidden email]>; See, Chin Liang <[hidden email]>
> > Subject: Re: [PATCH] lib: zlib: Use post-increment only in inffast.c
> >
> > On Fri, Oct 23, 2020 at 01:41:57AM +0000, Tan, Ley Foon wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Tom Rini <[hidden email]>
> > > > Sent: Thursday, October 22, 2020 9:24 PM
> > > > To: Tan, Ley Foon <[hidden email]>
> > > > Cc: Ley Foon Tan <[hidden email]>; ZY - u-boot <u-
> > > > [hidden email]>; See, Chin Liang <[hidden email]>
> > > > Subject: Re: [PATCH] lib: zlib: Use post-increment only in inffast.c
> > > >
> > > > On Wed, Oct 21, 2020 at 03:42:10AM +0000, Tan, Ley Foon wrote:
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Tom Rini <[hidden email]>
> > > > > > Sent: Friday, October 16, 2020 8:37 PM
> > > > > > To: Ley Foon Tan <[hidden email]>
> > > > > > Cc: Tan, Ley Foon <[hidden email]>; ZY - u-boot <u-
> > > > > > [hidden email]>; See, Chin Liang <[hidden email]>
> > > > > > Subject: Re: [PATCH] lib: zlib: Use post-increment only in
> > > > > > inffast.c
> > > > > >
> > > > > > On Fri, Oct 16, 2020 at 04:40:00AM +0800, Ley Foon Tan wrote:
> > > > > > > On Fri, Jul 17, 2020 at 9:29 PM Tom Rini <[hidden email]>
> > wrote:
> > > > > > > >
> > > > > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote:
> > > > > > > >
> > > > > > > > > From: Chin Liang See <[hidden email]>
> > > > > > > > >
> > > > > > > > > This fixes CVE-2016-9841. Changes integrated from [1],
> > > > > > > > > with changes make for Uboot code base.
> > > > > > > > >
> > > > > > > > > An old inffast.c optimization turns out to not be optimal
> > > > > > > > > anymore with modern compilers, and furthermore was not
> > > > > > > > > compliant with the C standard, for which decrementing a
> > > > > > > > > pointer before its allocated memory is undefined. Per the
> > > > > > > > > recommendation of a security audit of the zlib code by
> > > > > > > > > Trail of Bits and TrustInSoft, in support of the Mozilla
> > > > > > > > > Foundation, this "optimization" was removed, in order to
> > > > > > > > > avoid the possibility of
> > > > undefined behavior.
> > > > > > > > >
> > > > > > > > > [1]:
> > > > > > > > >
> > > > > >
> > > >
> > https://github.com/madler/zlib/commit/9aaec95e82117c1cb0f9624264c3
> > > > > > > > > 618fc380cecb
> > > > > > > > >
> > > > > > > > > Signed-off-by: Mark Adler <[hidden email]>
> > > > > > > > > Signed-off-by: Chin Liang See <[hidden email]>
> > > > > > > > > Signed-off-by: Ley Foon Tan <[hidden email]>
> > > > > > > >
> > > > > > > > This breaks the following tests on sandbox:
> > > > > > > > FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch -
> > > > > > > > u_boot_spawn.Timeout FAILED
> > > > > > > > test/py/tests/test_fit.py::test_fit
> > > > > > > > -
> > > > > > > > OSError: [Errno 5] Input/output error
> > > > > > > >
> > > > > > > Hi Tom
> > > > > > >
> > > > > > > I have tried to run the sandtest, but it failed in different
> > > > > > > test cases. I am run this command "./test/py/test.py --bd sandbox --
> > build".
> > > > > > > Error log at bottom of email.
> > > > > > >
> > > > > > > Found that
> > > > > > > https://gitlab.denx.de/u-boot/u-boot/-/blob/master/lib/zlib/zl
> > > > > > > ib.h always "#undef POSTINC", it is mean that U-boot can only
> > > > > > > support pre-increment? I have tried changing "#undef POSTINC"
> > > > > > > to "define POSTINC" and without this patch, the test failed at
> > > > > > > the same location.
> > > > > > > So, the failure is not caused by this patch.
> > > > > > > Note, this patch mainly changes to support post-increment only.
> > > > > > >
> > > > > > > Any suggestion to fix this?
> > > > > >
> > > > > > I'm not sure why the tests fail for you to start with.  They all
> > > > > > pass inn the CI environment as well as locally.  I would start
> > > > > > by seeing how your environment differs from those.
> > > > >
> > > > > Hi Tom
> > > > >
> > > > > My test is running on Ubuntu 20.04 Linux.
> > > > >
> > > > > Can you try change "#undef POSTINC" to "#define POSTINC" in
> > > > lib/zlib/zlib.h if you can see the error?
> > > >
> > > > I see:
> > > > FAILED test/py/tests/test_efi_fit.py::test_efi_fit_launch - assert
> > > > 'Hello, world' in "## Loa...
> > > > FAILED test/py/tests/test_fit.py::test_fit - OSError: [Errno 5]
> > > > Input/output error
> > > >
> > > > as new problems with that change.
> > > Just to confirm, this error is with this patch or change to "#define POSTINC"?
> > >
> > > Yours errors are different from my run.
> > >
> > > My run failed in these 2 testcases:
> > > test/py/tests/test_fs/test_squashfs/test_sqfs_load.py F
> > > test/py/tests/test_fs/test_squashfs/test_sqfs_ls.py F
> >
> > The squashfs tests can fail if they ran before, sometimes and I think that
> > means we need a clean-up action in them.  Delete the .bm-work directory
> > and re-run.  I see the errors I reported when I changed the define, as you
> > asked.  Originally, I saw the errors I reported in the CI systems, with the
> > patch in question.
>
> Yes, I also found delete .bm-work directory can solve squashfs test error. But, enable POSTINC still can see error even  clean the -work directory.
>
> So, this can confirm the test error not caused by this patch.
>
> The root cause is in test code or U-boot code?
I don't know where exactly what the problem is where, only that the
original patch in question introduces a regression.

--
Tom

signature.asc (673 bytes) Download Attachment