linux-kernel - Re: [PATCH v2 4/4] fs/pstore: fs/squashfs: Change usage of LZ4 to comply with new LZ4 module version

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:   Tue, 10 Jan 2017 10:45:00 +0100
From:   Sven Schmidt <4sschmid@...ormatik.uni-hamburg.de>
To:     keescook@...omium.org
Cc:     gregkh@...uxfoundation.org, akpm@...ux-foundation.org,
        bongkyu.kim@....com, rsalvaterra@...il.com,
        sergey.senozhatsky@...il.com, linux-kernel@...r.kernel.org,
        herbert@...dor.apana.org.au, davem@...emloft.net,
        linux-crypto@...r.kernel.org, anton@...msg.org, ccross@...roid.com,
        tony.luck@...el.com, phillip@...ashfs.org.uk,
        Sven Schmidt <4sschmid@...ormatik.uni-hamburg.de>
Subject: Re: [PATCH v2 4/4] fs/pstore: fs/squashfs: Change usage of LZ4 to comply with new LZ4 module version

Hi Kees,

On 01/07/2017 10:33 PM, Kees Cook wrote:
 >On Sat, Jan 7, 2017 at 8:55 AM, Sven Schmidt
 ><4sschmid@...ormatik.uni-hamburg.de> wrote:
 >> This patch updates fs/pstore and fs/squashfs to use the updated functions from
 >> the new LZ4 module.
 >>
 >> Signed-off-by: Sven Schmidt <4sschmid@...ormatik.uni-hamburg.de>
 >> ---
 >>  fs/pstore/platform.c      | 14 ++++++++------
 >>  fs/squashfs/lz4_wrapper.c | 12 ++++++------
 >>  2 files changed, 14 insertions(+), 12 deletions(-)
 >>
 >> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
 >> index 729677e..a0d8ca8 100644
 >> --- a/fs/pstore/platform.c
 >> +++ b/fs/pstore/platform.c
 >> @@ -342,31 +342,33 @@ static int compress_lz4(const void *in, void *out, size_t inlen, size_t outlen)
 >>  {
 >>         int ret;
 >>
 >> -       ret = lz4_compress(in, inlen, out, &outlen, workspace);
 >> +       ret = LZ4_compress_default(in, out, inlen, outlen, workspace);
 >>         if (ret) {
 >> +               // ret is 0 means an error occured
 >
 >If that's true, then shouldn't the "if" logic be changed? Also, here
 >and in all following comments are C++ style instead of kernel C-style.
 >This should be "/* ret == 0 means an error occured */", though really,
 >that should be obvious from the code and the comment isn't really
 >needed.

indeed, it should. I fixed that one.

 >>                 pr_err("lz4_compress error, ret = %d!\n", ret);
 >If it's always going to be zero here, is there a better place to get
 >details on why it failed?

 It is always going to be zero. Honestly, after looking at the current LZ4 in kernel again
 I don't get why there actually was a need to print the return value since lz4_compress
 will (as far as I see) always return -1 in case of an error while the new lz4_compress_fast/default
 will return 0 in such case. Maybe I should just stick with the error?

 Thanks,

 Sven