linux-kernel - Re: [PATCH] lib/lz4: smatch warning in LZ4_decompress

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 6 Jun 2020 18:16:23 +0300
From:   Vasily Averin <vvs@...tuozzo.com>
To:     Gao Xiang <hsiangkao@....com>, Yann Collet <cyan@...com>
Cc:     linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Gao Xiang <xiang@...nel.org>
Subject: Re: [PATCH] lib/lz4: smatch warning in LZ4_decompress_generic()

Dear Yann,
could you please consult us about your lz4 pacth
https://github.com/lz4/lz4/commit/1a191b3f8d26b50a7c1d41590b529ec308d768cd

Please see details below.

Thank you,
	Vasily Averin

On 6/6/20 5:36 PM, Gao Xiang wrote:
> On Sat, Jun 06, 2020 at 04:28:02PM +0300, Vasily Averin wrote:
>> Found by smatch:
>> lib/lz4/lz4_decompress.c:150 LZ4_decompress_generic() warn: maybe use && instead of &
>> It was realy incorrectly copied from
>> https://github.com/lz4/lz4/commit/45f8603aae389d34c689d3ff7427b314071ccd2c
>> line 1431
> 
> Simply no.
> 
>>
>> Fixes: 2209fda323e2 ("lib/lz4: update LZ4 decompressor module")
>> Signed-off-by: Vasily Averin <vvs@...tuozzo.com>
>> ---
>>  lib/lz4/lz4_decompress.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>commit 1a191b3f8d26b50a7c1d41590b529ec308d768cd
Author: Yann Collet <cyan@...com>
Date:   Wed May 2 10:33:12 2018 -0700

    simplify shortcut

diff --git a/lib/lz4.c b/lib/lz4.c
index c6f0426..b46910f 100644
--- a/lib/lz4.c
+++ b/lib/lz4.c
@@ -1429,62 +1429,29 @@ LZ4_FORCE_INLINE int LZ4_decompress_generic(
          */
         if ((endOnInput ? length != RUN_MASK : length <= 8) &&
             /* strictly "less than" on input, to re-enter the loop with at least one byte */
-            likely((endOnInput ? ip < shortiend : 1) && (op <= shortoend)))
+            likely((endOnInput ? ip < shortiend : 1) & (op <= shortoend)))
>
>> diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
>> index 0c9d3ad..f7f7dca 100644
>> --- a/lib/lz4/lz4_decompress.c
>> +++ b/lib/lz4/lz4_decompress.c
>> @@ -147,7 +147,7 @@ static FORCE_INLINE int LZ4_decompress_generic(
>>  		    * strictly "less than" on input, to re-enter
>>  		    * the loop with at least one byte
>>  		    */
>> -		   && likely((endOnInput ? ip < shortiend : 1) &
>> +		   && likely((endOnInput ? ip < shortiend : 1) &&
> 
> I'd like to say, this is not my mistake (even not an issue).
> If you notice the latest LZ4 upstream
> https://github.com/lz4/lz4/blob/dev/lib/lz4.c#L1865
> 
> Or some related change, the lz4 author Cyan did it on purpose.
> https://github.com/lz4/lz4/commit/1a191b3f8d26b50a7c1d41590b529ec308d768cd

For me it looks like typo in patch,
lets ask author of this commit.

> I think we could follow the latest LZ4 upstream in order to
> avoid further maintainence overhead. That's my own thought
> anyway.
> 
> Thanks,
> Gao Xiang
>