2021-02-18 13:03:14

by Jiri Bohac

[permalink] [raw]
Subject: pstore: fix compression

pstore_compress() and decompress_record() use a mistyped config option
name ("PSTORE_COMPRESSION" instead of "PSTORE_COMPRESS").
As a result compression and decompressionm of pstore records is always
disabled.

Use the correct config option name.

Signed-off-by: Jiri Bohac <[email protected]>
Fixes: fd49e03280e596e54edb93a91bc96170f8e97e4a ("pstore: Fix linking when crypto API disabled")

diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index 32f64abc277c..d963ae7902f9 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -269,7 +269,7 @@ static int pstore_compress(const void *in, void *out,
{
int ret;

- if (!IS_ENABLED(CONFIG_PSTORE_COMPRESSION))
+ if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS))
return -EINVAL;

ret = crypto_comp_compress(tfm, in, inlen, out, &outlen);
@@ -671,7 +671,7 @@ static void decompress_record(struct pstore_record *record)
int unzipped_len;
char *unzipped, *workspace;

- if (!IS_ENABLED(CONFIG_PSTORE_COMPRESSION) || !record->compressed)
+ if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS) || !record->compressed)
return;

/* Only PSTORE_TYPE_DMESG support compression. */

--
Jiri Bohac <[email protected]>
SUSE Labs, Prague, Czechia


2021-02-18 13:41:01

by Matteo Croce

[permalink] [raw]
Subject: Re: pstore: fix compression

On Thu, Feb 18, 2021 at 12:15 PM Jiri Bohac <[email protected]> wrote:
>
> pstore_compress() and decompress_record() use a mistyped config option
> name ("PSTORE_COMPRESSION" instead of "PSTORE_COMPRESS").
> As a result compression and decompressionm of pstore records is always
> disabled.
>
> Use the correct config option name.
>
> Signed-off-by: Jiri Bohac <[email protected]>
> Fixes: fd49e03280e596e54edb93a91bc96170f8e97e4a ("pstore: Fix linking when crypto API disabled")
>

Acked-by: Matteo Croce <[email protected]>

--
per aspera ad upstream

2021-02-18 14:06:09

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: pstore: fix compression

On Thu, Feb 18, 2021 at 12:15:47PM +0100, Jiri Bohac wrote:
> pstore_compress() and decompress_record() use a mistyped config option
> name ("PSTORE_COMPRESSION" instead of "PSTORE_COMPRESS").
> As a result compression and decompressionm of pstore records is always
> disabled.
>
> Use the correct config option name.
>
> Signed-off-by: Jiri Bohac <[email protected]>
> Fixes: fd49e03280e596e54edb93a91bc96170f8e97e4a ("pstore: Fix linking when crypto API disabled")
>
<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

2021-02-18 20:37:42

by Kees Cook

[permalink] [raw]
Subject: Re: pstore: fix compression

On Thu, 18 Feb 2021 12:15:47 +0100, Jiri Bohac wrote:
> pstore_compress() and decompress_record() use a mistyped config option
> name ("PSTORE_COMPRESSION" instead of "PSTORE_COMPRESS").
> As a result compression and decompressionm of pstore records is always
> disabled.
>
> Use the correct config option name.

Eek; thanks for the catch!

Applied to for-next/pstore, thanks!

[1/1] pstore: Fix typo in compression option name
https://git.kernel.org/kees/c/19d8e9149c27

--
Kees Cook

2021-02-19 00:58:13

by Jiri Bohac

[permalink] [raw]
Subject: Re: pstore: fix compression

On Thu, Feb 18, 2021 at 12:30:03PM -0800, Kees Cook wrote:
> Eek; thanks for the catch!

thanks for applying the fix;

BTW, with the compression broken, I was not getting any dmesg
stored in ERST at all, not even uncompressed. After instrumenting
the code with a lot of debug printks I found that writing
erst_erange.size worth of data into the ERST fails and the
maximum writeable size is 98 bytes smaler:

Details:

- erst_erange.size = 65536
- this results in erst_info.bufsize = 65336
- pstore_compress() returned -EINVAL (because of the
just-fixed typo), zipped_len = -EINVAL.
- pstore_dump calls copy_kmsg_to_buffer to only copy bufsize
bytes from big_oops_buf to psinfo->buf;
record.size = bufsize = 65336

psinfo->write(&record) then fails with -EINVAL;
by more tracing inside the ERST code I found the -EINVAL was
produced by __erst_write_to_storage()
after apei_exec_ctx_get_output() returned
val=ERST_STATUS_FAILED=3 and this got translated into -EINVAL by
erst_errno().

Once the compression was fixed everything started working because
the records are much smaller after the compression (~30kB).

My next thought was to find the largest possible record that
could be written successfully.
I modified the ERST init code to decrease erst_info.bufsize by a
value specified on the cmdline. The maximum writable record was
65238 bytes long (i.e. erst_erange.size - sizeof(struct
cper_pstore_record) - 98). With this hack I got
65238 bytes of uncompressed dmesg stored to ERST.

Any idea what might be causing this?
As far as I can tell, there are no other records in the ERST
(checked through the erst-dbg interface).
Tested on a HPE ProLiant DL120 Gen10 server.

Thanks!


--
Jiri Bohac <[email protected]>
SUSE Labs, Prague, Czechia