* [PATCH] pstore/platform: build fix when crypto API are disabled
@ 2020-07-06 23:40 Matteo Croce
2020-07-07 2:37 ` Kees Cook
0 siblings, 1 reply; 3+ messages in thread
From: Matteo Croce @ 2020-07-06 23:40 UTC (permalink / raw)
To: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck; +Cc: linux-kernel
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)
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] pstore/platform: build fix when crypto API are disabled
2020-07-06 23:40 [PATCH] pstore/platform: build fix when crypto API are disabled Matteo Croce
@ 2020-07-07 2:37 ` Kees Cook
2020-07-07 11:25 ` Matteo Croce
0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2020-07-07 2:37 UTC (permalink / raw)
To: Matteo Croce; +Cc: Anton Vorontsov, Colin Cross, Tony Luck, linux-kernel
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] pstore/platform: build fix when crypto API are disabled
2020-07-07 2:37 ` Kees Cook
@ 2020-07-07 11:25 ` Matteo Croce
0 siblings, 0 replies; 3+ messages in thread
From: Matteo Croce @ 2020-07-07 11:25 UTC (permalink / raw)
To: Kees Cook; +Cc: Anton Vorontsov, Colin Cross, Tony Luck, linux-kernel
On Tue, Jul 7, 2020 at 4:37 AM Kees Cook <keescook@chromium.org> wrote:
>
> 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).
>
Yes, it works for me!
--
per aspera ad upstream
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-07 11:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-06 23:40 [PATCH] pstore/platform: build fix when crypto API are disabled Matteo Croce
2020-07-07 2:37 ` Kees Cook
2020-07-07 11:25 ` Matteo Croce
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).