Re: [PATCH] pstore/platform: build fix when crypto API are disabled

[Kees Cook <keescook@chromium.org>]

On Tue, Jul 07, 2020 at 01:40:45AM +0200, Matteo Croce wrote:
> From: Matteo Croce <mcroce@microsoft.com>
> 
> When building a kernel with CONFIG_PSTORE=y and CONFIG_CRYPTO not set,
> a build error happens:
> 
>     ld: fs/pstore/platform.o: in function `pstore_dump':
>     platform.c:(.text+0x3f9): undefined reference to `crypto_comp_compress'
>     ld: fs/pstore/platform.o: in function `pstore_get_backend_records':
>     platform.c:(.text+0x784): undefined reference to `crypto_comp_decompress'
> 
> This because some pstore code uses crypto_comp_(de)compress
> regardless of the CONFIG_CRYPTO status.
> Fix it by wrapping the (de)compress usage by IS_ENABLED(CONFIG_PSTORE_COMPRESS)

I'm surprised this hasn't come up before in a randconfig! But I guess
it'd require a very lucky config: picking CONFIG_PSTORE but not
CONFIG_CRYPTO _and_ 0 of the many compression options in pstore. :P

But yes, I can reproduce this with:

# CONFIG_CRYPTO is not set
CONFIG_PSTORE=y
# CONFIG_PSTORE_DEFLATE_COMPRESS is not set
# CONFIG_PSTORE_LZO_COMPRESS is not set
# CONFIG_PSTORE_LZ4_COMPRESS is not set
# CONFIG_PSTORE_LZ4HC_COMPRESS is not set
# CONFIG_PSTORE_842_COMPRESS is not set
# CONFIG_PSTORE_ZSTD_COMPRESS is not set

> 
> Signed-off-by: Matteo Croce <mcroce@microsoft.com>
> ---
>  fs/pstore/platform.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
> index a9e297eefdff..6022d8359f96 100644
> --- a/fs/pstore/platform.c
> +++ b/fs/pstore/platform.c
> @@ -436,7 +436,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
>  					  dst_size, &dump_size))
>  			break;
>  
> -		if (big_oops_buf) {
> +		if (IS_ENABLED(CONFIG_PSTORE_COMPRESS) && big_oops_buf) {
>  			zipped_len = pstore_compress(dst, psinfo->buf,
>  						header_size + dump_size,
>  						psinfo->bufsize);
> @@ -668,7 +668,7 @@ static void decompress_record(struct pstore_record *record)
>  	int unzipped_len;
>  	char *unzipped, *workspace;
>  
> -	if (!record->compressed)
> +	if (!IS_ENABLED(CONFIG_PSTORE_COMPRESS) || !record->compressed)
>  		return;
>  
>  	/* Only PSTORE_TYPE_DMESG support compression. */
> -- 
> 2.26.2

This report also reminds me that I want to stop hard-coding the possible
compressors[1].

Regardless, for now, I'd like a slightly different patch, which pokes
pstore_compress() instead of doing it inline in pstore_dump():


diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index a9e297eefdff..36714df37d5d 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -269,6 +269,9 @@ static int pstore_compress(const void *in, void *out,
 {
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_PSTORE_COMPRESSION))
+		return -EINVAL;
+
 	ret = crypto_comp_compress(tfm, in, inlen, out, &outlen);
 	if (ret) {
 		pr_err("crypto_comp_compress failed, ret = %d!\n", ret);
@@ -668,7 +671,7 @@ static void decompress_record(struct pstore_record *record)
 	int unzipped_len;
 	char *unzipped, *workspace;
 
-	if (!record->compressed)
+	if (!IS_ENABLED(CONFIG_PSTORE_COMPRESSION) || !record->compressed)
 		return;
 
 	/* Only PSTORE_TYPE_DMESG support compression. */



Let me know if that works for you (it fixes it on my end).

Thanks!

-Kees

[1] https://lore.kernel.org/lkml/20180802215118.17752-1-keescook@chromium.org/

-- 
Kees Cook