| 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
| ||
|
Message-ID: <CAGXu5jL+CWgMpBzhOwro7qo=a9kHhSGqp+EDMqKv23k7FOFUzQ@mail.gmail.com>
Date: Tue, 28 Nov 2017 17:44:52 -0800
From: Kees Cook <keescook@...omium.org>
To: Geliang Tang <geliangtang@...il.com>
Cc: Anton Vorontsov <anton@...msg.org>,
Colin Cross <ccross@...roid.com>,
Tony Luck <tony.luck@...el.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] pstore: add lz4hc and 842 compression support
Sorry for the delay on this review!
On Mon, Aug 28, 2017 at 6:56 AM, Geliang Tang <geliangtang@...il.com> wrote:
> Currently, pstore has supported three compression algorithms: zlib,
> lzo and lz4. This patch added two more compression algorithms: lz4hc
> and 842.
>
> Signed-off-by: Geliang Tang <geliangtang@...il.com>
> ---
> fs/pstore/Kconfig | 14 ++++++
> fs/pstore/platform.c | 128 +++++++++++++++++++++++++++++++++++++++++++++------
> 2 files changed, 127 insertions(+), 15 deletions(-)
>
> diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig
> index b42e5bd..e288596 100644
> --- a/fs/pstore/Kconfig
> +++ b/fs/pstore/Kconfig
> @@ -39,6 +39,20 @@ config PSTORE_LZ4_COMPRESS
> select LZ4_DECOMPRESS
> help
> This option enables LZ4 compression algorithm support.
> +
> +config PSTORE_LZ4HC_COMPRESS
> + bool "LZ4HC"
> + select LZ4HC_COMPRESS
> + select LZ4_DECOMPRESS
> + help
> + This option enables LZ4 high compression mode algorithm.
> +
> +config PSTORE_842_COMPRESS
> + bool "842"
> + select 842_COMPRESS
> + select 842_DECOMPRESS
> + help
> + This option enables 842 compression algorithm support.
> endchoice
I'm fine with these short descriptions. checkpatch's warning for
"choice" options doesn't always make sense. :)
>
> config PSTORE_CONSOLE
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index 2b21d18..252c320 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -34,9 +34,12 @@
> #ifdef CONFIG_PSTORE_LZO_COMPRESS
> #include <linux/lzo.h>
> #endif
> -#ifdef CONFIG_PSTORE_LZ4_COMPRESS
> +#if defined(CONFIG_PSTORE_LZ4_COMPRESS) || defined(CONFIG_PSTORE_LZ4HC_COMPRESS)
> #include <linux/lz4.h>
> #endif
> +#ifdef CONFIG_PSTORE_842_COMPRESS
> +#include <linux/sw842.h>
> +#endif
> #include <linux/string.h>
> #include <linux/timer.h>
> #include <linux/slab.h>
> @@ -337,20 +340,7 @@ static const struct pstore_zbackend backend_lzo = {
> };
> #endif
>
> -#ifdef CONFIG_PSTORE_LZ4_COMPRESS
> -static int compress_lz4(const void *in, void *out, size_t inlen, size_t outlen)
> -{
> - int ret;
> -
> - ret = LZ4_compress_default(in, out, inlen, outlen, workspace);
> - if (!ret) {
> - pr_err("LZ4_compress_default error; compression failed!\n");
> - return -EIO;
> - }
> -
> - return ret;
> -}
> -
> +#if defined(CONFIG_PSTORE_LZ4_COMPRESS) || defined(CONFIG_PSTORE_LZ4HC_COMPRESS)
> static int decompress_lz4(void *in, void *out, size_t inlen, size_t outlen)
> {
> int ret;
> @@ -392,6 +382,21 @@ static void free_lz4(void)
> big_oops_buf = NULL;
> big_oops_buf_sz = 0;
> }
> +#endif
> +
> +#ifdef CONFIG_PSTORE_LZ4_COMPRESS
> +static int compress_lz4(const void *in, void *out, size_t inlen, size_t outlen)
> +{
> + int ret;
> +
> + ret = LZ4_compress_default(in, out, inlen, outlen, workspace);
> + if (!ret) {
> + pr_err("LZ4_compress_default error; compression failed!\n");
> + return -EIO;
> + }
> +
> + return ret;
Other compress/decompress return outlen, rather than ret. Is "ret" the
outlen here?
> +}
>
> static const struct pstore_zbackend backend_lz4 = {
> .compress = compress_lz4,
> @@ -402,6 +407,95 @@ static const struct pstore_zbackend backend_lz4 = {
> };
> #endif
>
> +#ifdef CONFIG_PSTORE_LZ4HC_COMPRESS
> +static int compress_lz4hc(const void *in, void *out,
> + size_t inlen, size_t outlen)
> +{
> + int ret;
> +
> + ret = LZ4_compress_HC(in, out, inlen, outlen,
> + LZ4HC_DEFAULT_CLEVEL, workspace);
> + if (!ret) {
> + pr_err("LZ4_compress_HC error; compression failed!\n");
> + return -EIO;
> + }
> +
> + return ret;
And here?
> +}
> +
> +static const struct pstore_zbackend backend_lz4hc = {
> + .compress = compress_lz4hc,
> + .decompress = decompress_lz4,
> + .allocate = allocate_lz4,
> + .free = free_lz4,
> + .name = "lz4hc",
> +};
> +#endif
> +
> +#ifdef CONFIG_PSTORE_842_COMPRESS
> +static int compress_842(const void *in, void *out, size_t inlen, size_t outlen)
> +{
> + int ret;
> + unsigned int len;
> +
> + ret = sw842_compress(in, inlen, out, &len, workspace);
> + if (!ret) {
> + pr_err("sw842_compress error; compression failed!\n");
> + return -EIO;
> + }
> + outlen = len;
This has no effect. What was intended?
> +
> + return ret;
> +}
> +
> +static int decompress_842(void *in, void *out, size_t inlen, size_t outlen)
> +{
> + int ret;
> + unsigned int len;
> +
> + ret = sw842_decompress(in, inlen, out, &len);
> + if (ret < 0) {
> + pr_err("sw842_decompress error, ret = %d!\n", ret);
> + return -EIO;
> + }
> + outlen = len;
Same.
> +
> + return ret;
> +}
> +
> +static void allocate_842(void)
> +{
> + big_oops_buf_sz = psinfo->bufsize * 2;
> + big_oops_buf = kmalloc(big_oops_buf_sz, GFP_KERNEL);
> + if (big_oops_buf) {
> + workspace = kmalloc(SW842_MEM_COMPRESS, GFP_KERNEL);
> + if (!workspace) {
> + kfree(big_oops_buf);
> + big_oops_buf = NULL;
> + }
> + } else {
> + pr_err("No memory for uncompressed data; skipping compression\n");
> + workspace = NULL;
> + }
> +}
> +
> +static void free_842(void)
> +{
> + kfree(workspace);
> + kfree(big_oops_buf);
> + big_oops_buf = NULL;
> + big_oops_buf_sz = 0;
> +}
> +
> +static const struct pstore_zbackend backend_842 = {
> + .compress = compress_842,
> + .decompress = decompress_842,
> + .allocate = allocate_842,
> + .free = free_842,
> + .name = "842",
> +};
> +#endif
> +
> static const struct pstore_zbackend *zbackend =
> #if defined(CONFIG_PSTORE_ZLIB_COMPRESS)
> &backend_zlib;
> @@ -409,6 +503,10 @@ static const struct pstore_zbackend *zbackend =
> &backend_lzo;
> #elif defined(CONFIG_PSTORE_LZ4_COMPRESS)
> &backend_lz4;
> +#elif defined(CONFIG_PSTORE_LZ4HC_COMPRESS)
> + &backend_lz4hc;
> +#elif defined(CONFIG_PSTORE_842_COMPRESS)
> + &backend_842;
> #else
> NULL;
> #endif
> --
> 2.9.3
>
Otherwise this all looks good. Thanks!
-Kees
--
Kees Cook
Pixel Security
Powered by blists - more mailing lists