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

[Tom Rini]

On Fri, Oct 23, 2020 at 01:41:57AM +0000, Tan, Ley Foon wrote:
> 
> 
> > -----Original Message-----
> > From: Tom Rini <trini at konsulko.com>
> > Sent: Thursday, October 22, 2020 9:24 PM
> > To: Tan, Ley Foon <ley.foon.tan at intel.com>
> > Cc: Ley Foon Tan <lftan.linux at gmail.com>; ZY - u-boot <u-
> > boot at lists.denx.de>; See, Chin Liang <chin.liang.see at 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 at konsulko.com>
> > > > Sent: Friday, October 16, 2020 8:37 PM
> > > > To: Ley Foon Tan <lftan.linux at gmail.com>
> > > > Cc: Tan, Ley Foon <ley.foon.tan at intel.com>; ZY - u-boot <u-
> > > > boot at lists.denx.de>; See, Chin Liang <chin.liang.see at 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 at konsulko.com> wrote:
> > > > > >
> > > > > > On Wed, Jun 24, 2020 at 04:34:03PM +0800, Ley Foon Tan wrote:
> > > > > >
> > > > > > > From: Chin Liang See <chin.liang.see at 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 at alumni.caltech.edu>
> > > > > > > Signed-off-by: Chin Liang See <chin.liang.see at intel.com>
> > > > > > > Signed-off-by: Ley Foon Tan <ley.foon.tan at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20201022/43ee853c/attachment.sig>