Skip to content

Commit 438b805

Browse files
ardbiesheuvelkees
authored andcommitted
pstore: Replace crypto API compression with zlib_deflate library calls
Pstore supports compression using a variety of algorithms exposed by the crypto API. This uses the deprecated comp (as opposed to scomp/acomp) API, and so we should stop using that, and either move to the new API, or switch to a different approach entirely. Given that we only compress ASCII text in pstore, and considering that this happens when the system is likely to be in an unstable state, the flexibility that the complex crypto API provides does not outweigh its impact on the risk that we might encounter additional problems when trying to commit the kernel log contents to the pstore backend. So let's switch [back] to the zlib deflate library API, and remove all the complexity that really has no place in a low-level diagnostic facility. Note that, while more modern compression algorithms have been added to the kernel in recent years, the code size of zlib deflate is substantially smaller than, e.g., zstd, while its performance in terms of compression ratio is comparable for ASCII text, and speed is deemed irrelevant in this context. Note that this means that compressed pstore records may no longer be accessible after a kernel upgrade, but this has never been part of the contract. (The choice of compression algorithm is not stored in the pstore records either) Tested-by: "Guilherme G. Piccoli" <gpiccoli@igalia.com> # Steam Deck Signed-off-by: Ard Biesheuvel <ardb@kernel.org> Reviewed-by: Eric Biggers <ebiggers@google.com> Link: https://lore.kernel.org/r/20230712162332.2670437-3-ardb@kernel.org Signed-off-by: Kees Cook <keescook@chromium.org>
1 parent 1756dde commit 438b805

File tree

2 files changed

+97
-139
lines changed

2 files changed

+97
-139
lines changed

fs/pstore/Kconfig

Lines changed: 9 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# SPDX-License-Identifier: GPL-2.0-only
22
config PSTORE
33
tristate "Persistent store support"
4-
select CRYPTO if PSTORE_COMPRESS
54
default n
65
help
76
This option enables generic access to platform level
@@ -22,99 +21,18 @@ config PSTORE_DEFAULT_KMSG_BYTES
2221
Defines default size of pstore kernel log storage.
2322
Can be enlarged if needed, not recommended to shrink it.
2423

25-
config PSTORE_DEFLATE_COMPRESS
26-
tristate "DEFLATE (ZLIB) compression"
27-
default y
28-
depends on PSTORE
29-
select CRYPTO_DEFLATE
30-
help
31-
This option enables DEFLATE (also known as ZLIB) compression
32-
algorithm support.
33-
34-
config PSTORE_LZO_COMPRESS
35-
tristate "LZO compression"
36-
depends on PSTORE
37-
select CRYPTO_LZO
38-
help
39-
This option enables LZO compression algorithm support.
40-
41-
config PSTORE_LZ4_COMPRESS
42-
tristate "LZ4 compression"
43-
depends on PSTORE
44-
select CRYPTO_LZ4
45-
help
46-
This option enables LZ4 compression algorithm support.
47-
48-
config PSTORE_LZ4HC_COMPRESS
49-
tristate "LZ4HC compression"
50-
depends on PSTORE
51-
select CRYPTO_LZ4HC
52-
help
53-
This option enables LZ4HC (high compression) mode algorithm.
54-
55-
config PSTORE_842_COMPRESS
56-
bool "842 compression"
57-
depends on PSTORE
58-
select CRYPTO_842
59-
help
60-
This option enables 842 compression algorithm support.
61-
62-
config PSTORE_ZSTD_COMPRESS
63-
bool "zstd compression"
64-
depends on PSTORE
65-
select CRYPTO_ZSTD
66-
help
67-
This option enables zstd compression algorithm support.
68-
6924
config PSTORE_COMPRESS
70-
def_bool y
25+
bool "Pstore compression (deflate)"
7126
depends on PSTORE
72-
depends on PSTORE_DEFLATE_COMPRESS || PSTORE_LZO_COMPRESS || \
73-
PSTORE_LZ4_COMPRESS || PSTORE_LZ4HC_COMPRESS || \
74-
PSTORE_842_COMPRESS || PSTORE_ZSTD_COMPRESS
75-
76-
choice
77-
prompt "Default pstore compression algorithm"
78-
depends on PSTORE_COMPRESS
27+
select ZLIB_INFLATE
28+
select ZLIB_DEFLATE
29+
default y
7930
help
80-
This option chooses the default active compression algorithm.
81-
This change be changed at boot with "pstore.compress=..." on
82-
the kernel command line.
83-
84-
Currently, pstore has support for 6 compression algorithms:
85-
deflate, lzo, lz4, lz4hc, 842 and zstd.
86-
87-
The default compression algorithm is deflate.
88-
89-
config PSTORE_DEFLATE_COMPRESS_DEFAULT
90-
bool "deflate" if PSTORE_DEFLATE_COMPRESS
91-
92-
config PSTORE_LZO_COMPRESS_DEFAULT
93-
bool "lzo" if PSTORE_LZO_COMPRESS
94-
95-
config PSTORE_LZ4_COMPRESS_DEFAULT
96-
bool "lz4" if PSTORE_LZ4_COMPRESS
97-
98-
config PSTORE_LZ4HC_COMPRESS_DEFAULT
99-
bool "lz4hc" if PSTORE_LZ4HC_COMPRESS
100-
101-
config PSTORE_842_COMPRESS_DEFAULT
102-
bool "842" if PSTORE_842_COMPRESS
103-
104-
config PSTORE_ZSTD_COMPRESS_DEFAULT
105-
bool "zstd" if PSTORE_ZSTD_COMPRESS
106-
107-
endchoice
108-
109-
config PSTORE_COMPRESS_DEFAULT
110-
string
111-
depends on PSTORE_COMPRESS
112-
default "deflate" if PSTORE_DEFLATE_COMPRESS_DEFAULT
113-
default "lzo" if PSTORE_LZO_COMPRESS_DEFAULT
114-
default "lz4" if PSTORE_LZ4_COMPRESS_DEFAULT
115-
default "lz4hc" if PSTORE_LZ4HC_COMPRESS_DEFAULT
116-
default "842" if PSTORE_842_COMPRESS_DEFAULT
117-
default "zstd" if PSTORE_ZSTD_COMPRESS_DEFAULT
31+
Whether pstore records should be compressed before being written to
32+
the backing store. This is implemented using the zlib 'deflate'
33+
algorithm, using the library implementation instead of using the full
34+
blown crypto API. This reduces the risk of secondary oopses or other
35+
problems while pstore is recording panic metadata.
11836

11937
config PSTORE_CONSOLE
12038
bool "Log kernel console messages"

fs/pstore/platform.c

Lines changed: 88 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,14 @@
1616
#include <linux/console.h>
1717
#include <linux/module.h>
1818
#include <linux/pstore.h>
19-
#include <linux/crypto.h>
2019
#include <linux/string.h>
2120
#include <linux/timer.h>
2221
#include <linux/slab.h>
2322
#include <linux/uaccess.h>
2423
#include <linux/jiffies.h>
24+
#include <linux/vmalloc.h>
2525
#include <linux/workqueue.h>
26+
#include <linux/zlib.h>
2627

2728
#include "internal.h"
2829

@@ -71,12 +72,21 @@ static char *backend;
7172
module_param(backend, charp, 0444);
7273
MODULE_PARM_DESC(backend, "specific backend to use");
7374

74-
static char *compress =
75-
#ifdef CONFIG_PSTORE_COMPRESS_DEFAULT
76-
CONFIG_PSTORE_COMPRESS_DEFAULT;
77-
#else
78-
NULL;
79-
#endif
75+
/*
76+
* pstore no longer implements compression via the crypto API, and only
77+
* supports zlib deflate compression implemented using the zlib library
78+
* interface. This removes additional complexity which is hard to justify for a
79+
* diagnostic facility that has to operate in conditions where the system may
80+
* have become unstable. Zlib deflate is comparatively small in terms of code
81+
* size, and compresses ASCII text comparatively well. In terms of compression
82+
* speed, deflate is not the best performer but for recording the log output on
83+
* a kernel panic, this is not considered critical.
84+
*
85+
* The only remaining arguments supported by the compress= module parameter are
86+
* 'deflate' and 'none'. To retain compatibility with existing installations,
87+
* all other values are logged and replaced with 'deflate'.
88+
*/
89+
static char *compress = "deflate";
8090
module_param(compress, charp, 0444);
8191
MODULE_PARM_DESC(compress, "compression to use");
8292

@@ -85,8 +95,7 @@ unsigned long kmsg_bytes = CONFIG_PSTORE_DEFAULT_KMSG_BYTES;
8595
module_param(kmsg_bytes, ulong, 0444);
8696
MODULE_PARM_DESC(kmsg_bytes, "amount of kernel log to snapshot (in bytes)");
8797

88-
/* Compression parameters */
89-
static struct crypto_comp *tfm;
98+
static void *compress_workspace;
9099

91100
static char *big_oops_buf;
92101

@@ -156,36 +165,49 @@ static bool pstore_cannot_block_path(enum kmsg_dump_reason reason)
156165
static int pstore_compress(const void *in, void *out,
157166
unsigned int inlen, unsigned int outlen)
158167
{
168+
struct z_stream_s zstream = {
169+
.next_in = in,
170+
.avail_in = inlen,
171+
.next_out = out,
172+
.avail_out = outlen,
173+
.workspace = compress_workspace,
174+
};
159175
int ret;
160176

161177
if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS))
162178
return -EINVAL;
163179

164-
ret = crypto_comp_compress(tfm, in, inlen, out, &outlen);
165-
if (ret) {
166-
pr_err("crypto_comp_compress failed, ret = %d!\n", ret);
167-
return ret;
168-
}
180+
ret = zlib_deflateInit2(&zstream, Z_DEFAULT_COMPRESSION, Z_DEFLATED,
181+
-MAX_WBITS, DEF_MEM_LEVEL, Z_DEFAULT_STRATEGY);
182+
if (ret != Z_OK)
183+
return -EINVAL;
169184

170-
return outlen;
185+
ret = zlib_deflate(&zstream, Z_FINISH);
186+
if (ret != Z_STREAM_END)
187+
return -EINVAL;
188+
189+
ret = zlib_deflateEnd(&zstream);
190+
if (ret != Z_OK)
191+
pr_warn_once("zlib_deflateEnd() failed: %d\n", ret);
192+
193+
return zstream.total_out;
171194
}
172195

173196
static void allocate_buf_for_compression(void)
174197
{
175-
struct crypto_comp *ctx;
176198
char *buf;
177199

178-
/* Skip if not built-in or compression backend not selected yet. */
179-
if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS) || !compress)
180-
return;
181-
182-
/* Skip if no pstore backend yet or compression init already done. */
183-
if (!psinfo || tfm)
200+
/* Skip if not built-in or compression disabled. */
201+
if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS) || !compress ||
202+
!strcmp(compress, "none")) {
203+
compress = NULL;
184204
return;
205+
}
185206

186-
if (!crypto_has_comp(compress, 0, 0)) {
187-
pr_err("Unknown compression: %s\n", compress);
188-
return;
207+
if (strcmp(compress, "deflate")) {
208+
pr_err("Unsupported compression '%s', falling back to deflate\n",
209+
compress);
210+
compress = "deflate";
189211
}
190212

191213
/*
@@ -200,27 +222,27 @@ static void allocate_buf_for_compression(void)
200222
return;
201223
}
202224

203-
ctx = crypto_alloc_comp(compress, 0, 0);
204-
if (IS_ERR_OR_NULL(ctx)) {
225+
compress_workspace =
226+
vmalloc(zlib_deflate_workspacesize(MAX_WBITS, DEF_MEM_LEVEL));
227+
if (!compress_workspace) {
228+
pr_err("Failed to allocate zlib deflate workspace\n");
205229
kfree(buf);
206-
pr_err("crypto_alloc_comp('%s') failed: %ld\n", compress,
207-
PTR_ERR(ctx));
208230
return;
209231
}
210232

211233
/* A non-NULL big_oops_buf indicates compression is available. */
212-
tfm = ctx;
213234
big_oops_buf = buf;
214235

215236
pr_info("Using crash dump compression: %s\n", compress);
216237
}
217238

218239
static void free_buf_for_compression(void)
219240
{
220-
if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && tfm) {
221-
crypto_free_comp(tfm);
222-
tfm = NULL;
241+
if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && compress_workspace) {
242+
vfree(compress_workspace);
243+
compress_workspace = NULL;
223244
}
245+
224246
kfree(big_oops_buf);
225247
big_oops_buf = NULL;
226248
}
@@ -531,7 +553,8 @@ void pstore_unregister(struct pstore_info *psi)
531553
}
532554
EXPORT_SYMBOL_GPL(pstore_unregister);
533555

534-
static void decompress_record(struct pstore_record *record)
556+
static void decompress_record(struct pstore_record *record,
557+
struct z_stream_s *zstream)
535558
{
536559
int ret;
537560
int unzipped_len;
@@ -547,26 +570,37 @@ static void decompress_record(struct pstore_record *record)
547570
}
548571

549572
/* Missing compression buffer means compression was not initialized. */
550-
if (!big_oops_buf) {
573+
if (!zstream->workspace) {
551574
pr_warn("no decompression method initialized!\n");
552575
return;
553576
}
554577

578+
ret = zlib_inflateReset(zstream);
579+
if (ret != Z_OK) {
580+
pr_err("zlib_inflateReset() failed, ret = %d!\n", ret);
581+
return;
582+
}
583+
555584
/* Allocate enough space to hold max decompression and ECC. */
556585
workspace = kmalloc(psinfo->bufsize + record->ecc_notice_size,
557586
GFP_KERNEL);
558587
if (!workspace)
559588
return;
560589

561-
/* After decompression "unzipped_len" is almost certainly smaller. */
562-
ret = crypto_comp_decompress(tfm, record->buf, record->size,
563-
workspace, &unzipped_len);
564-
if (ret) {
565-
pr_err("crypto_comp_decompress failed, ret = %d!\n", ret);
590+
zstream->next_in = record->buf;
591+
zstream->avail_in = record->size;
592+
zstream->next_out = workspace;
593+
zstream->avail_out = psinfo->bufsize;
594+
595+
ret = zlib_inflate(zstream, Z_FINISH);
596+
if (ret != Z_STREAM_END) {
597+
pr_err("zlib_inflate() failed, ret = %d!\n", ret);
566598
kfree(workspace);
567599
return;
568600
}
569601

602+
unzipped_len = zstream->total_out;
603+
570604
/* Append ECC notice to decompressed buffer. */
571605
memcpy(workspace + unzipped_len, record->buf + record->size,
572606
record->ecc_notice_size);
@@ -596,10 +630,17 @@ void pstore_get_backend_records(struct pstore_info *psi,
596630
{
597631
int failed = 0;
598632
unsigned int stop_loop = 65536;
633+
struct z_stream_s zstream = {};
599634

600635
if (!psi || !root)
601636
return;
602637

638+
if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && compress) {
639+
zstream.workspace = kvmalloc(zlib_inflate_workspacesize(),
640+
GFP_KERNEL);
641+
zlib_inflateInit2(&zstream, -DEF_WBITS);
642+
}
643+
603644
mutex_lock(&psi->read_mutex);
604645
if (psi->open && psi->open(psi))
605646
goto out;
@@ -628,7 +669,7 @@ void pstore_get_backend_records(struct pstore_info *psi,
628669
break;
629670
}
630671

631-
decompress_record(record);
672+
decompress_record(record, &zstream);
632673
rc = pstore_mkfile(root, record);
633674
if (rc) {
634675
/* pstore_mkfile() did not take record, so free it. */
@@ -644,6 +685,12 @@ void pstore_get_backend_records(struct pstore_info *psi,
644685
out:
645686
mutex_unlock(&psi->read_mutex);
646687

688+
if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && compress) {
689+
if (zlib_inflateEnd(&zstream) != Z_OK)
690+
pr_warn("zlib_inflateEnd() failed\n");
691+
kvfree(zstream.workspace);
692+
}
693+
647694
if (failed)
648695
pr_warn("failed to create %d record(s) from '%s'\n",
649696
failed, psi->name);
@@ -671,13 +718,6 @@ static int __init pstore_init(void)
671718
{
672719
int ret;
673720

674-
/*
675-
* Check if any pstore backends registered earlier but did not
676-
* initialize compression because crypto was not ready. If so,
677-
* initialize compression now.
678-
*/
679-
allocate_buf_for_compression();
680-
681721
ret = pstore_init_fs();
682722
if (ret)
683723
free_buf_for_compression();

0 commit comments

Comments
 (0)