[v6,2/8] lib: prepare xxhash for preboot environment
diff mbox series

Message ID 20200707034604.1539157-3-nickrterrell@gmail.com
State New
Headers show
Series
  • Add support for ZSTD-compressed kernel and initramfs
Related show

Commit Message

Nick Terrell July 7, 2020, 3:45 a.m. UTC
From: Nick Terrell <terrelln@fb.com>

Don't export symbols if XXH_PREBOOT is defined.

This change is necessary to get xxhash to work in a preboot environment,
which is needed to support zstd-compressed kernels.

Reviewed-by: Kees Cook <keescook@chromium.org>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
Signed-off-by: Nick Terrell <terrelln@fb.com>
---
 lib/xxhash.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Comments

Arvind Sankar July 7, 2020, 9:59 p.m. UTC | #1
On Mon, Jul 06, 2020 at 08:45:58PM -0700, Nick Terrell wrote:
> From: Nick Terrell <terrelln@fb.com>
> 
> Don't export symbols if XXH_PREBOOT is defined.
> 
> This change is necessary to get xxhash to work in a preboot environment,
> which is needed to support zstd-compressed kernels.

The usual way to do it is by adding -D__DISABLE_EXPORTS to the CFLAGS, which will
cause EXPORT_SYMBOL to be stubbed out. Doesn't that work here?
Nick Terrell July 8, 2020, 2:03 a.m. UTC | #2
> On Jul 7, 2020, at 5:59 PM, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> 
> On Mon, Jul 06, 2020 at 08:45:58PM -0700, Nick Terrell wrote:
>> From: Nick Terrell <terrelln@fb.com>
>> 
>> Don't export symbols if XXH_PREBOOT is defined.
>> 
>> This change is necessary to get xxhash to work in a preboot environment,
>> which is needed to support zstd-compressed kernels.
> 
> The usual way to do it is by adding -D__DISABLE_EXPORTS to the CFLAGS, which will
> cause EXPORT_SYMBOL to be stubbed out. Doesn't that work here?

I can do that. I did it this way because this was how other compressors did it.

Thanks,
Nick
Kees Cook July 8, 2020, 2:49 a.m. UTC | #3
On Tue, Jul 07, 2020 at 05:59:25PM -0400, Arvind Sankar wrote:
> On Mon, Jul 06, 2020 at 08:45:58PM -0700, Nick Terrell wrote:
> > From: Nick Terrell <terrelln@fb.com>
> > 
> > Don't export symbols if XXH_PREBOOT is defined.
> > 
> > This change is necessary to get xxhash to work in a preboot environment,
> > which is needed to support zstd-compressed kernels.
> 
> The usual way to do it is by adding -D__DISABLE_EXPORTS to the CFLAGS, which will
> cause EXPORT_SYMBOL to be stubbed out. Doesn't that work here?

This is quite rare, actually:

$ git grep DISABLE_EXPORTS
arch/s390/purgatory/Makefile:CFLAGS_sha256.o := -D__DISABLE_EXPORTS
arch/x86/boot/compressed/kaslr.c:#define __DISABLE_EXPORTS
arch/x86/purgatory/Makefile:CFLAGS_sha256.o := -D__DISABLE_EXPORTS
drivers/firmware/efi/libstub/Makefile: -D__DISABLE_EXPORTS
include/linux/export.h:#if !defined(CONFIG_MODULES) || defined(__DISABLE_EXPORTS)

But yes, it seems that would be the better approach.
Arvind Sankar July 8, 2020, 3:23 a.m. UTC | #4
On Tue, Jul 07, 2020 at 07:49:25PM -0700, Kees Cook wrote:
> On Tue, Jul 07, 2020 at 05:59:25PM -0400, Arvind Sankar wrote:
> > On Mon, Jul 06, 2020 at 08:45:58PM -0700, Nick Terrell wrote:
> > > From: Nick Terrell <terrelln@fb.com>
> > > 
> > > Don't export symbols if XXH_PREBOOT is defined.
> > > 
> > > This change is necessary to get xxhash to work in a preboot environment,
> > > which is needed to support zstd-compressed kernels.
> > 
> > The usual way to do it is by adding -D__DISABLE_EXPORTS to the CFLAGS, which will
> > cause EXPORT_SYMBOL to be stubbed out. Doesn't that work here?
> 
> This is quite rare, actually:
> 
> $ git grep DISABLE_EXPORTS
> arch/s390/purgatory/Makefile:CFLAGS_sha256.o := -D__DISABLE_EXPORTS
> arch/x86/boot/compressed/kaslr.c:#define __DISABLE_EXPORTS
> arch/x86/purgatory/Makefile:CFLAGS_sha256.o := -D__DISABLE_EXPORTS
> drivers/firmware/efi/libstub/Makefile: -D__DISABLE_EXPORTS
> include/linux/export.h:#if !defined(CONFIG_MODULES) || defined(__DISABLE_EXPORTS)
> 
> But yes, it seems that would be the better approach.
> 
> -- 
> Kees Cook

Looks like Ard added it a couple of years back [0] but it got used only
for the EFI stub and not the decompressors.

[0] http://lkml.kernel.org/r/20180704083651.24360-3-ard.biesheuvel@linaro.org

Patch
diff mbox series

diff --git a/lib/xxhash.c b/lib/xxhash.c
index aa61e2a3802f..b4364e011392 100644
--- a/lib/xxhash.c
+++ b/lib/xxhash.c
@@ -80,13 +80,11 @@  void xxh32_copy_state(struct xxh32_state *dst, const struct xxh32_state *src)
 {
 	memcpy(dst, src, sizeof(*dst));
 }
-EXPORT_SYMBOL(xxh32_copy_state);
 
 void xxh64_copy_state(struct xxh64_state *dst, const struct xxh64_state *src)
 {
 	memcpy(dst, src, sizeof(*dst));
 }
-EXPORT_SYMBOL(xxh64_copy_state);
 
 /*-***************************
  * Simple Hash Functions
@@ -151,7 +149,6 @@  uint32_t xxh32(const void *input, const size_t len, const uint32_t seed)
 
 	return h32;
 }
-EXPORT_SYMBOL(xxh32);
 
 static uint64_t xxh64_round(uint64_t acc, const uint64_t input)
 {
@@ -234,7 +231,6 @@  uint64_t xxh64(const void *input, const size_t len, const uint64_t seed)
 
 	return h64;
 }
-EXPORT_SYMBOL(xxh64);
 
 /*-**************************************************
  * Advanced Hash Functions
@@ -251,7 +247,6 @@  void xxh32_reset(struct xxh32_state *statePtr, const uint32_t seed)
 	state.v4 = seed - PRIME32_1;
 	memcpy(statePtr, &state, sizeof(state));
 }
-EXPORT_SYMBOL(xxh32_reset);
 
 void xxh64_reset(struct xxh64_state *statePtr, const uint64_t seed)
 {
@@ -265,7 +260,6 @@  void xxh64_reset(struct xxh64_state *statePtr, const uint64_t seed)
 	state.v4 = seed - PRIME64_1;
 	memcpy(statePtr, &state, sizeof(state));
 }
-EXPORT_SYMBOL(xxh64_reset);
 
 int xxh32_update(struct xxh32_state *state, const void *input, const size_t len)
 {
@@ -334,7 +328,6 @@  int xxh32_update(struct xxh32_state *state, const void *input, const size_t len)
 
 	return 0;
 }
-EXPORT_SYMBOL(xxh32_update);
 
 uint32_t xxh32_digest(const struct xxh32_state *state)
 {
@@ -372,7 +365,6 @@  uint32_t xxh32_digest(const struct xxh32_state *state)
 
 	return h32;
 }
-EXPORT_SYMBOL(xxh32_digest);
 
 int xxh64_update(struct xxh64_state *state, const void *input, const size_t len)
 {
@@ -439,7 +431,6 @@  int xxh64_update(struct xxh64_state *state, const void *input, const size_t len)
 
 	return 0;
 }
-EXPORT_SYMBOL(xxh64_update);
 
 uint64_t xxh64_digest(const struct xxh64_state *state)
 {
@@ -494,7 +485,19 @@  uint64_t xxh64_digest(const struct xxh64_state *state)
 
 	return h64;
 }
+
+#ifndef XXH_PREBOOT
+EXPORT_SYMBOL(xxh32_copy_state);
+EXPORT_SYMBOL(xxh64_copy_state);
+EXPORT_SYMBOL(xxh32);
+EXPORT_SYMBOL(xxh64);
+EXPORT_SYMBOL(xxh32_reset);
+EXPORT_SYMBOL(xxh64_reset);
+EXPORT_SYMBOL(xxh32_update);
+EXPORT_SYMBOL(xxh32_digest);
+EXPORT_SYMBOL(xxh64_update);
 EXPORT_SYMBOL(xxh64_digest);
 
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_DESCRIPTION("xxHash");
+#endif