| Message ID | 20200624083403.99603-1-ley.foon.tan@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | lib: zlib: Use post-increment only in inffast.c | expand |
On Fri, Jul 17, 2020 at 9:29 PM Tom Rini <trini@konsulko.com> wrote: > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote: > > > From: Chin Liang See <chin.liang.see@intel.com> > > > > 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 <madler@alumni.caltech.edu> > > Signed-off-by: Chin Liang See <chin.liang.see@intel.com> > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > > 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) ================================
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 <trini@konsulko.com> wrote: > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote: > > > > > From: Chin Liang See <chin.liang.see@intel.com> > > > > > > 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 <madler@alumni.caltech.edu> > > > Signed-off-by: Chin Liang See <chin.liang.see@intel.com> > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > > > > 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
> -----Original Message----- > From: Tom Rini <trini@konsulko.com> > Sent: Friday, October 16, 2020 8:37 PM > To: Ley Foon Tan <lftan.linux@gmail.com> > Cc: Tan, Ley Foon <ley.foon.tan@intel.com>; ZY - u-boot <u- > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > 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 <trini@konsulko.com> wrote: > > > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote: > > > > > > > From: Chin Liang See <chin.liang.see@intel.com> > > > > > > > > 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 <madler@alumni.caltech.edu> > > > > Signed-off-by: Chin Liang See <chin.liang.see@intel.com> > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > > > > > > 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
On Wed, Oct 21, 2020 at 03:42:10AM +0000, Tan, Ley Foon wrote: > > > > -----Original Message----- > > From: Tom Rini <trini@konsulko.com> > > Sent: Friday, October 16, 2020 8:37 PM > > To: Ley Foon Tan <lftan.linux@gmail.com> > > Cc: Tan, Ley Foon <ley.foon.tan@intel.com>; ZY - u-boot <u- > > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > > 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 <trini@konsulko.com> wrote: > > > > > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote: > > > > > > > > > From: Chin Liang See <chin.liang.see@intel.com> > > > > > > > > > > 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 <madler@alumni.caltech.edu> > > > > > Signed-off-by: Chin Liang See <chin.liang.see@intel.com> > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > > > > > > > > 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
> -----Original Message----- > From: Tom Rini <trini@konsulko.com> > Sent: Thursday, October 22, 2020 9:24 PM > To: Tan, Ley Foon <ley.foon.tan@intel.com> > Cc: Ley Foon Tan <lftan.linux@gmail.com>; ZY - u-boot <u- > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > 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 <trini@konsulko.com> > > > Sent: Friday, October 16, 2020 8:37 PM > > > To: Ley Foon Tan <lftan.linux@gmail.com> > > > Cc: Tan, Ley Foon <ley.foon.tan@intel.com>; ZY - u-boot <u- > > > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > > > 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 <trini@konsulko.com> wrote: > > > > > > > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote: > > > > > > > > > > > From: Chin Liang See <chin.liang.see@intel.com> > > > > > > > > > > > > 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 <madler@alumni.caltech.edu> > > > > > > Signed-off-by: Chin Liang See <chin.liang.see@intel.com> > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > > > > > > > > > > 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
On Fri, Oct 23, 2020 at 01:41:57AM +0000, Tan, Ley Foon wrote: > > > > -----Original Message----- > > From: Tom Rini <trini@konsulko.com> > > Sent: Thursday, October 22, 2020 9:24 PM > > To: Tan, Ley Foon <ley.foon.tan@intel.com> > > Cc: Ley Foon Tan <lftan.linux@gmail.com>; ZY - u-boot <u- > > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > > 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 <trini@konsulko.com> > > > > Sent: Friday, October 16, 2020 8:37 PM > > > > To: Ley Foon Tan <lftan.linux@gmail.com> > > > > Cc: Tan, Ley Foon <ley.foon.tan@intel.com>; ZY - u-boot <u- > > > > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > > > > 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 <trini@konsulko.com> wrote: > > > > > > > > > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote: > > > > > > > > > > > > > From: Chin Liang See <chin.liang.see@intel.com> > > > > > > > > > > > > > > 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 <madler@alumni.caltech.edu> > > > > > > > Signed-off-by: Chin Liang See <chin.liang.see@intel.com> > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > > > > > > > > > > > > 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
> -----Original Message----- > From: Tom Rini <trini@konsulko.com> > Sent: Friday, October 23, 2020 9:52 AM > To: Tan, Ley Foon <ley.foon.tan@intel.com> > Cc: Ley Foon Tan <lftan.linux@gmail.com>; ZY - u-boot <u- > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > 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 <trini@konsulko.com> > > > Sent: Thursday, October 22, 2020 9:24 PM > > > To: Tan, Ley Foon <ley.foon.tan@intel.com> > > > Cc: Ley Foon Tan <lftan.linux@gmail.com>; ZY - u-boot <u- > > > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > > > 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 <trini@konsulko.com> > > > > > Sent: Friday, October 16, 2020 8:37 PM > > > > > To: Ley Foon Tan <lftan.linux@gmail.com> > > > > > Cc: Tan, Ley Foon <ley.foon.tan@intel.com>; ZY - u-boot <u- > > > > > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > > > > > 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 <trini@konsulko.com> > wrote: > > > > > > > > > > > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote: > > > > > > > > > > > > > > > From: Chin Liang See <chin.liang.see@intel.com> > > > > > > > > > > > > > > > > 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 <madler@alumni.caltech.edu> > > > > > > > > Signed-off-by: Chin Liang See <chin.liang.see@intel.com> > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > > > > > > > > > > > > > > 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
On Fri, Oct 23, 2020 at 01:59:29AM +0000, Tan, Ley Foon wrote: > > > > -----Original Message----- > > From: Tom Rini <trini@konsulko.com> > > Sent: Friday, October 23, 2020 9:52 AM > > To: Tan, Ley Foon <ley.foon.tan@intel.com> > > Cc: Ley Foon Tan <lftan.linux@gmail.com>; ZY - u-boot <u- > > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > > 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 <trini@konsulko.com> > > > > Sent: Thursday, October 22, 2020 9:24 PM > > > > To: Tan, Ley Foon <ley.foon.tan@intel.com> > > > > Cc: Ley Foon Tan <lftan.linux@gmail.com>; ZY - u-boot <u- > > > > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > > > > 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 <trini@konsulko.com> > > > > > > Sent: Friday, October 16, 2020 8:37 PM > > > > > > To: Ley Foon Tan <lftan.linux@gmail.com> > > > > > > Cc: Tan, Ley Foon <ley.foon.tan@intel.com>; ZY - u-boot <u- > > > > > > boot@lists.denx.de>; See, Chin Liang <chin.liang.see@intel.com> > > > > > > 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 <trini@konsulko.com> > > wrote: > > > > > > > > > > > > > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote: > > > > > > > > > > > > > > > > > From: Chin Liang See <chin.liang.see@intel.com> > > > > > > > > > > > > > > > > > > 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 <madler@alumni.caltech.edu> > > > > > > > > > Signed-off-by: Chin Liang See <chin.liang.see@intel.com> > > > > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com> > > > > > > > > > > > > > > > > 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
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));