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.