linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] module: Add LZ4 module compression
@ 2026-11-15 12:04 Paul Menzel
  2026-11-15 17:35 ` Bradley Morgan
  0 siblings, 1 reply; 4+ messages in thread
From: Paul Menzel @ 2026-11-15 12:04 UTC (permalink / raw)
  To: Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	Aaron Tomlin, Nathan Chancellor, Nicolas Schier, Nick Terrell,
	David Sterba
  Cc: Paul Menzel, linux-modules, linux-kernel, linux-kbuild

XZ modules are small but slow to decompress in the kernel. Therefore,
support modules compressed in the LZ4 frame format (.ko.lz4), as written
by lz4(1). Decode block by block into a scratch buffer and copy into the
module pages. Linked blocks keep the last 64 KiB of output in front of
the scratch buffer as the dictionary. Verify the header checksum, block
checksums, content checksum and content size when the frame has them.
Frames with a dictionary ID are rejected.

modules_install uses `lz4 -12 -B4 -BD`: 64 KiB linked blocks are as
small as 4 MiB blocks (i915.ko: 3587140 vs 3585364 bytes) and keep the
scratch buffer at 128 KiB.

Userspace needs kmod with LZ4 support for depmod and modprobe [1].

On a Dell XPS 13 9310 with an Intel Core i7-1165G7 (Tiger Lake) and an
NVMe SSD, decompressing and checking the signatures of the 237 modules
loaded during boot takes 624 ms of CPU time with XZ (i915: 91 ms) and
131 ms with LZ4 (i915: 12 ms), measured by calling finit_module() again
for each loaded module. The LZ4 modules take 20.3 MB instead of 11.0 MB.
Both kernels are v7.4-rc1 with the same patches, including pending
lib/xz decoder speed-ups. Ten alternating boots of each kernel:

  | ms (median)                               |  XZ  | LZ4  | change |
  +-------------------------------------------+------+------+--------+
  | early-i915 (modprobe i915 + dependencies) |  295 |   90 |   -205 |
  | first i915 message                        |  738 |  547 |   -192 |
  | GDM autologin                             |  973 |  876 |    -96 |
  | GRUB start to autologin                   | 1292 | 1197 |    -94 |

QEMU 11.1.2 (KVM, AMD EPYC 7763, 8 vCPUs, module.sig_enforce=1, 10
rounds), loading the same 237 modules in dependency order:

  | median                   |   XZ    |   LZ4   | uncompressed |
  +--------------------------+---------+---------+--------------+
  | one at a time            | 2082 ms | 1499 ms |      1461 ms |
  | 8 in parallel, like udev |  619 ms |  573 ms |       593 ms |

Link: https://github.com/kmod-project/kmod/pull/505 [1]
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
---

Notes:
    Changes in v2:
    - Declare retval as ssize_t, so output of 2 GiB or more is not
      truncated (Sashiko). The zstd decoder has the same bug, fixed by
      the separate patch "module/decompress: Use ssize_t for the zstd
      decompressed size".
    - Reject empty frames with -EINVAL; v1 failed vmap() of 0 pages with
      -ENOMEM (Sashiko).
    - Describe the decoder and link the frame format specification, group
      the definitions by FLG byte, BD byte and block size field, compare the
      version with FIELD_GET(), rename LZ4F_DICT_SIZE to LZ4F_WINDOW_SIZE,
      look up the block maximum size in a table, shorten the dictionary
      comment (Bradley Morgan).
    
    v1: https://lore.kernel.org/all/20261113144233.193782-1-pmenzel@molgen.mpg.de/
    
    Claude Opus 5.5 (Claude Code) wrote the decoder, the v2
    changes and this message. Tests: W=1 compile (GCC 16.2, Debian config
    for x86_64, tinyconfig for i386); QEMU with module.sig_enforce=1: 11 LZ4
    frame variants (64 KiB-4 MiB, linked/independent, block/content
    checksums, content size, stored blocks) load, 9 corrupt, truncated or
    empty files are rejected with EINVAL or EKEYREJECTED (v1: ENOMEM for the
    empty ones); the 237-module boot set loads in QEMU. v1 also loaded on a
    Dell XPS 13 9310 (20 boots); v2 was not booted on hardware.

 kernel/module/Kconfig      |  13 +-
 kernel/module/decompress.c | 243 +++++++++++++++++++++++++++++++++++++
 scripts/Makefile.modinst   |   6 +
 3 files changed, 261 insertions(+), 1 deletion(-)

diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 43b1bb01fd273..36f12612c74dd 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -344,7 +344,7 @@ config MODULE_COMPRESS
 	  This is fully compatible with signed modules.
 
 	  The tool used to work with modules needs to support the selected
-	  compression type. kmod MAY support gzip, xz and zstd. Other tools
+	  compression type. kmod MAY support gzip, xz, zstd and lz4. Other tools
 	  might have a limited selection of the supported types.
 
 	  Note that for modules inside an initrd or initramfs, it's more
@@ -377,6 +377,15 @@ config MODULE_COMPRESS_ZSTD
 	  Support modules compressed with ZSTD. The installed modules are
 	  suffixed with .ko.zst.
 
+config MODULE_COMPRESS_LZ4
+	bool "LZ4"
+	help
+	  Support modules compressed with LZ4. The installed modules are
+	  suffixed with .ko.lz4.
+
+	  LZ4 modules are larger than XZ or ZSTD ones but decompress
+	  several times faster.
+
 endchoice
 
 config MODULE_COMPRESS_ALL
@@ -394,6 +403,8 @@ config MODULE_DECOMPRESS
 	select ZLIB_INFLATE if MODULE_COMPRESS_GZIP
 	select XZ_DEC if MODULE_COMPRESS_XZ
 	select ZSTD_DECOMPRESS if MODULE_COMPRESS_ZSTD
+	select LZ4_DECOMPRESS if MODULE_COMPRESS_LZ4
+	select XXHASH if MODULE_COMPRESS_LZ4
 	help
 	  Support for decompressing kernel modules by the kernel itself
 	  instead of relying on userspace to perform this task. Useful when
diff --git a/kernel/module/decompress.c b/kernel/module/decompress.c
index cce098671be97..028b411015f21 100644
--- a/kernel/module/decompress.c
+++ b/kernel/module/decompress.c
@@ -287,6 +287,249 @@ static ssize_t module_zstd_decompress(struct load_info *info,
 	kvfree(wksp);
 	return retval;
 }
+#elif defined(CONFIG_MODULE_COMPRESS_LZ4)
+#include <linux/bitfield.h>
+#include <linux/lz4.h>
+#include <linux/sizes.h>
+#include <linux/unaligned.h>
+#include <linux/xxhash.h>
+#define MODULE_COMPRESSION	lz4
+#define MODULE_DECOMPRESS_FN	module_lz4_decompress
+
+/*
+ * Decoder for the LZ4 frame format written by lz4(1), see
+ * https://github.com/lz4/lz4/blob/dev/doc/lz4_Frame_format.md
+ *
+ * Each block is decoded into a scratch buffer and copied into the module
+ * pages. A linked block may reference up to 64 KiB of the output before
+ * it, so that much output is kept in front of the scratch buffer.
+ */
+#define LZ4F_MAGIC		0x184d2204
+
+/* Frame descriptor, FLG byte */
+#define LZ4F_FLG_VERSION	GENMASK(7, 6)
+#define LZ4F_FLG_BLOCK_INDEP	BIT(5)
+#define LZ4F_FLG_BLOCK_CSUM	BIT(4)
+#define LZ4F_FLG_CONTENT_SIZE	BIT(3)
+#define LZ4F_FLG_CONTENT_CSUM	BIT(2)
+#define LZ4F_FLG_RESERVED	BIT(1)
+#define LZ4F_FLG_DICT_ID	BIT(0)
+
+/* Frame descriptor, BD byte */
+#define LZ4F_BD_BLOCK_MAXSIZE	GENMASK(6, 4)
+#define LZ4F_BD_RESERVED	(BIT(7) | GENMASK(3, 0))
+
+/* Block size field: the top bit marks a block stored uncompressed */
+#define LZ4F_BLOCK_UNCOMPRESSED	BIT(31)
+
+#define LZ4F_WINDOW_SIZE	SZ_64K
+
+/* Block maximum sizes for the BD byte's Block MaxSize values 4 to 7 */
+static const u32 lz4f_block_max[] = { SZ_64K, SZ_256K, SZ_1M, SZ_4M };
+
+static int module_lz4_copy_out(struct load_info *info, const u8 *src,
+			       size_t len, size_t *total)
+{
+	while (len) {
+		size_t offset = *total % PAGE_SIZE;
+		size_t chunk = min_t(size_t, len, PAGE_SIZE - offset);
+		struct page *page;
+
+		if (offset)
+			page = info->pages[info->used_pages - 1];
+		else
+			page = module_get_next_page(info);
+		if (IS_ERR(page))
+			return PTR_ERR(page);
+
+		memcpy_to_page(page, offset, src, chunk);
+		src += chunk;
+		len -= chunk;
+		*total += chunk;
+	}
+
+	return 0;
+}
+
+static int module_lz4_verify_content(struct load_info *info, size_t total,
+				     u32 expected)
+{
+	void *data;
+	u32 csum;
+
+	data = vmap(info->pages, info->used_pages, VM_MAP, PAGE_KERNEL_RO);
+	if (!data)
+		return -ENOMEM;
+	csum = xxh32(data, total, 0);
+	vunmap(data);
+
+	if (csum != expected) {
+		pr_err("LZ4-compressed data is corrupt (content checksum)\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static ssize_t module_lz4_decompress(struct load_info *info,
+				     const void *buf, size_t size)
+{
+	const u8 *in = buf, *end = in + size, *desc;
+	size_t block_max, total = 0, dict_len = 0;
+	u64 content_size = 0;
+	u8 *scratch, *out;
+	bool linked;
+	u8 flg, bd;
+	ssize_t retval;
+
+	if (size < 7 || get_unaligned_le32(in) != LZ4F_MAGIC) {
+		pr_err("not an lz4 compressed module\n");
+		return -EINVAL;
+	}
+
+	desc = in + 4;
+	flg = desc[0];
+	bd = desc[1];
+	in = desc + 2;
+	if (FIELD_GET(LZ4F_FLG_VERSION, flg) != 1 ||
+	    (flg & LZ4F_FLG_RESERVED) || (bd & LZ4F_BD_RESERVED) ||
+	    FIELD_GET(LZ4F_BD_BLOCK_MAXSIZE, bd) < 4) {
+		pr_err("LZ4-compressed data has an unsupported frame descriptor\n");
+		return -EINVAL;
+	}
+	if (flg & LZ4F_FLG_DICT_ID) {
+		pr_err("LZ4-compressed data needs a dictionary\n");
+		return -EINVAL;
+	}
+	if (flg & LZ4F_FLG_CONTENT_SIZE) {
+		if (end - in < 8)
+			goto truncated_header;
+		content_size = get_unaligned_le64(in);
+		in += 8;
+	}
+	if (end - in < 1)
+		goto truncated_header;
+	if (*in != ((xxh32(desc, in - desc, 0) >> 8) & 0xff)) {
+		pr_err("LZ4-compressed data has a corrupt frame descriptor\n");
+		return -EINVAL;
+	}
+	in++;
+
+	block_max = lz4f_block_max[FIELD_GET(LZ4F_BD_BLOCK_MAXSIZE, bd) - 4];
+	linked = !(flg & LZ4F_FLG_BLOCK_INDEP);
+
+	scratch = kvmalloc((linked ? LZ4F_WINDOW_SIZE : 0) + block_max,
+			   GFP_KERNEL);
+	if (!scratch)
+		return -ENOMEM;
+	out = scratch + (linked ? LZ4F_WINDOW_SIZE : 0);
+
+	for (;;) {
+		u32 block_size;
+		size_t len;
+		int ret;
+
+		if (end - in < 4)
+			goto truncated;
+		block_size = get_unaligned_le32(in);
+		in += 4;
+		if (!block_size)
+			break;
+
+		len = block_size & ~LZ4F_BLOCK_UNCOMPRESSED;
+		if (len > block_max) {
+			pr_err("LZ4-compressed data has an oversized block\n");
+			retval = -EINVAL;
+			goto out;
+		}
+		if ((size_t)(end - in) < len +
+		    (flg & LZ4F_FLG_BLOCK_CSUM ? 4 : 0))
+			goto truncated;
+
+		if ((flg & LZ4F_FLG_BLOCK_CSUM) &&
+		    xxh32(in, len, 0) != get_unaligned_le32(in + len)) {
+			pr_err("LZ4-compressed data is corrupt (block checksum)\n");
+			retval = -EINVAL;
+			goto out;
+		}
+
+		if (block_size & LZ4F_BLOCK_UNCOMPRESSED) {
+			memcpy(out, in, len);
+			ret = len;
+		} else if (linked) {
+			ret = LZ4_decompress_safe_usingDict(in, out, len,
+							    block_max,
+							    out - dict_len,
+							    dict_len);
+		} else {
+			ret = LZ4_decompress_safe(in, out, len, block_max);
+		}
+		if (ret < 0) {
+			pr_err("LZ4-decompression failed\n");
+			retval = -EINVAL;
+			goto out;
+		}
+
+		retval = module_lz4_copy_out(info, out, ret, &total);
+		if (retval)
+			goto out;
+
+		if (linked) {
+			/* Keep the last 64 KiB as the next block's dictionary. */
+			size_t keep = min_t(size_t, dict_len + ret,
+					    LZ4F_WINDOW_SIZE);
+
+			memmove(out - keep, out + ret - keep, keep);
+			dict_len = keep;
+		}
+
+		in += len;
+		if (flg & LZ4F_FLG_BLOCK_CSUM)
+			in += 4;
+	}
+
+	if (!total) {
+		pr_err("LZ4-compressed module is empty\n");
+		retval = -EINVAL;
+		goto out;
+	}
+
+	if (flg & LZ4F_FLG_CONTENT_CSUM) {
+		if (end - in < 4)
+			goto truncated;
+		retval = module_lz4_verify_content(info, total,
+						   get_unaligned_le32(in));
+		if (retval)
+			goto out;
+		in += 4;
+	}
+
+	if (in != end) {
+		pr_err("LZ4-compressed module has trailing data\n");
+		retval = -EINVAL;
+		goto out;
+	}
+
+	if ((flg & LZ4F_FLG_CONTENT_SIZE) && content_size != total) {
+		pr_err("LZ4-compressed data has a wrong content size\n");
+		retval = -EINVAL;
+		goto out;
+	}
+
+	retval = total;
+	goto out;
+
+truncated:
+	pr_err("LZ4-compressed data is truncated\n");
+	retval = -EINVAL;
+out:
+	kvfree(scratch);
+	return retval;
+
+truncated_header:
+	pr_err("LZ4-compressed data is truncated\n");
+	return -EINVAL;
+}
 #else
 #error "Unexpected configuration for CONFIG_MODULE_DECOMPRESS"
 #endif
diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index 9ba45e5b32b18..aebc5e89d0e75 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -57,6 +57,7 @@ ifdef CONFIG_MODULE_COMPRESS_ALL
 suffix-$(CONFIG_MODULE_COMPRESS_GZIP)	:= .gz
 suffix-$(CONFIG_MODULE_COMPRESS_XZ)	:= .xz
 suffix-$(CONFIG_MODULE_COMPRESS_ZSTD)	:= .zst
+suffix-$(CONFIG_MODULE_COMPRESS_LZ4)	:= .lz4
 endif
 
 modules := $(patsubst %.o, $(dst)/%.ko$(suffix-y), $(modules))
@@ -151,6 +152,8 @@ quiet_cmd_xz = XZ      $@
       cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
 quiet_cmd_zstd = ZSTD    $@
       cmd_zstd = $(ZSTD) --rm -f -q $<
+quiet_cmd_lz4 = LZ4     $@
+      cmd_lz4 = $(LZ4) -12 -B4 -BD --rm -f -q $< $@
 
 $(dst)/%.ko.gz: $(dst)/%.ko FORCE
 	$(call cmd,gzip)
@@ -161,6 +164,9 @@ $(dst)/%.ko.xz: $(dst)/%.ko FORCE
 $(dst)/%.ko.zst: $(dst)/%.ko FORCE
 	$(call cmd,zstd)
 
+$(dst)/%.ko.lz4: $(dst)/%.ko FORCE
+	$(call cmd,lz4)
+
 PHONY += FORCE
 FORCE:
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] module: Add LZ4 module compression
  2026-11-15 12:04 [PATCH v2] module: Add LZ4 module compression Paul Menzel
@ 2026-11-15 17:35 ` Bradley Morgan
  2026-11-15 21:09   ` Paul Menzel
  0 siblings, 1 reply; 4+ messages in thread
From: Bradley Morgan @ 2026-11-15 17:35 UTC (permalink / raw)
  To: pmenzel
  Cc: atomlin, da.gomez, dsterba, linux-kbuild, linux-kernel,
	linux-modules, mcgrof, nathan, nsc, petr.pavlu, samitolvanen,
	terrelln

On 15 November 2026 12:04:13 GMT, Paul Menzel <pmenzel@molgen.mpg.de>
wrote:
>XZ modules are small but slow to decompress in the kernel. Therefore,
>support modules compressed in the LZ4 frame format (.ko.lz4), as written
>by lz4(1). Decode block by block into a scratch buffer and copy into the
>module pages. Linked blocks keep the last 64 KiB of output in front of
>the scratch buffer as the dictionary. Verify the header checksum, block
>checksums, content checksum and content size when the frame has them.
>Frames with a dictionary ID are rejected.

.ko.lz4? when would this be used..?

>
>modules_install uses `lz4 -12 -B4 -BD`: 64 KiB linked blocks are as
>small as 4 MiB blocks (i915.ko: 3587140 vs 3585364 bytes) and keep the
>scratch buffer at 128 KiB.
>
>Userspace needs kmod with LZ4 support for depmod and modprobe [1].
>
>On a Dell XPS 13 9310 with an Intel Core i7-1165G7 (Tiger Lake) and an
>NVMe SSD, decompressing and checking the signatures of the 237 modules
>loaded during boot takes 624 ms of CPU time with XZ (i915: 91 ms) and
>131 ms with LZ4 (i915: 12 ms), measured by calling finit_module() again
>for each loaded module. The LZ4 modules take 20.3 MB instead of 11.0 MB.
>Both kernels are v7.4-rc1 with the same patches, including pending
>lib/xz decoder speed-ups. Ten alternating boots of each kernel:
>
>  | ms (median)                               |  XZ  | LZ4  | change |
>  +-------------------------------------------+------+------+--------+
>  | early-i915 (modprobe i915 + dependencies) |  295 |   90 |   -205 |
>  | first i915 message                        |  738 |  547 |   -192 |
>  | GDM autologin                             |  973 |  876 |    -96 |
>  | GRUB start to autologin                   | 1292 | 1197 |    -94 |
>
>QEMU 11.1.2 (KVM, AMD EPYC 7763, 8 vCPUs, module.sig_enforce=1, 10
>rounds), loading the same 237 modules in dependency order:
>
>  | median                   |   XZ    |   LZ4   | uncompressed |
>  +--------------------------+---------+---------+--------------+
>  | one at a time            | 2082 ms | 1499 ms |      1461 ms |
>  | 8 in parallel, like udev |  619 ms |  573 ms |       593 ms |

This table doesn't look very nicely formatted for me.

>
>Link: https://github.com/kmod-project/kmod/pull/505 [1]
>Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
>---
>
>Notes:
>    Changes in v2:
>    - Declare retval as ssize_t, so output of 2 GiB or more is not
>      truncated (Sashiko). The zstd decoder has the same bug, fixed by
>      the separate patch "module/decompress: Use ssize_t for the zstd
>      decompressed size".
>    - Reject empty frames with -EINVAL; v1 failed vmap() of 0 pages with
>      -ENOMEM (Sashiko).
>    - Describe the decoder and link the frame format specification, group
>      the definitions by FLG byte, BD byte and block size field, compare the
>      version with FIELD_GET(), rename LZ4F_DICT_SIZE to LZ4F_WINDOW_SIZE,
>      look up the block maximum size in a table, shorten the dictionary
>      comment (Bradley Morgan).
>    
>    v1: https://lore.kernel.org/all/20261113144233.193782-1-pmenzel@molgen.mpg.de/
>    
>    Claude Opus 5.5 (Claude Code) wrote the decoder, the v2
>    changes and this message. Tests: W=1 compile (GCC 16.2, Debian config
>    for x86_64, tinyconfig for i386); QEMU with module.sig_enforce=1: 11 LZ4
>    frame variants (64 KiB-4 MiB, linked/independent, block/content
>    checksums, content size, stored blocks) load, 9 corrupt, truncated or
>    empty files are rejected with EINVAL or EKEYREJECTED (v1: ENOMEM for the
>    empty ones); the 237-module boot set loads in QEMU. v1 also loaded on a
>    Dell XPS 13 9310 (20 boots); v2 was not booted on hardware.

Could you do a favour and test patches you submit, just in case of the
edgecases :)

>
> kernel/module/Kconfig      |  13 +-
> kernel/module/decompress.c | 243 +++++++++++++++++++++++++++++++++++++
> scripts/Makefile.modinst   |   6 +
> 3 files changed, 261 insertions(+), 1 deletion(-)
>
>diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
>index 43b1bb01fd273..36f12612c74dd 100644
>--- a/kernel/module/Kconfig
>+++ b/kernel/module/Kconfig
>@@ -344,7 +344,7 @@ config MODULE_COMPRESS
> 	  This is fully compatible with signed modules.
> 
> 	  The tool used to work with modules needs to support the selected
>-	  compression type. kmod MAY support gzip, xz and zstd. Other tools
>+	  compression type. kmod MAY support gzip, xz, zstd and lz4. Other tools
> 	  might have a limited selection of the supported types.
> 
> 	  Note that for modules inside an initrd or initramfs, it's more
>@@ -377,6 +377,15 @@ config MODULE_COMPRESS_ZSTD
> 	  Support modules compressed with ZSTD. The installed modules are
> 	  suffixed with .ko.zst.
> 
>+config MODULE_COMPRESS_LZ4
>+	bool "LZ4"
>+	help
>+	  Support modules compressed with LZ4. The installed modules are
>+	  suffixed with .ko.lz4.
>+
>+	  LZ4 modules are larger than XZ or ZSTD ones but decompress
>+	  several times faster.
>+
> endchoice
> 
> config MODULE_COMPRESS_ALL
>@@ -394,6 +403,8 @@ config MODULE_DECOMPRESS
> 	select ZLIB_INFLATE if MODULE_COMPRESS_GZIP
> 	select XZ_DEC if MODULE_COMPRESS_XZ
> 	select ZSTD_DECOMPRESS if MODULE_COMPRESS_ZSTD
>+	select LZ4_DECOMPRESS if MODULE_COMPRESS_LZ4
>+	select XXHASH if MODULE_COMPRESS_LZ4
> 	help
> 	  Support for decompressing kernel modules by the kernel itself
> 	  instead of relying on userspace to perform this task. Useful when
>diff --git a/kernel/module/decompress.c b/kernel/module/decompress.c
>index cce098671be97..028b411015f21 100644
>--- a/kernel/module/decompress.c
>+++ b/kernel/module/decompress.c
>@@ -287,6 +287,249 @@ static ssize_t module_zstd_decompress(struct load_info *info,
> 	kvfree(wksp);
> 	return retval;
> }
>+#elif defined(CONFIG_MODULE_COMPRESS_LZ4)
>+#include <linux/bitfield.h>
>+#include <linux/lz4.h>
>+#include <linux/sizes.h>
>+#include <linux/unaligned.h>
>+#include <linux/xxhash.h>
>+#define MODULE_COMPRESSION	lz4
>+#define MODULE_DECOMPRESS_FN	module_lz4_decompress
>+

Oh these includes mid file are a MESS! No, please no. Good to if defined,
but I'm sure the maintainer wouldn't mind if you ifdefed those includes on
the TOP of the file. Like any other normal kernel file.

>+/*
>+ * Decoder for the LZ4 frame format written by lz4(1), see
>+ * https://github.com/lz4/lz4/blob/dev/doc/lz4_Frame_format.md
>+ *
>+ * Each block is decoded into a scratch buffer and copied into the module
>+ * pages. A linked block may reference up to 64 KiB of the output before
>+ * it, so that much output is kept in front of the scratch buffer.
>+ */

Well, I suggested something better,

/* Decoder for the lz4 frame format,
 * written by lz4 [1]
 * This code adds support for lz4
 * modules inside the kernel, so 
 * that lz4 modules can be used.
 * 
 * [1] - (your link, lz4 frame format src??)
 */

What about this?

I say bikeshed, but let's not make it TOO sloppy, you reckon?

>+#define LZ4F_MAGIC		0x184d2204
>+
>+/* Frame descriptor, FLG byte */
>+#define LZ4F_FLG_VERSION	GENMASK(7, 6)
>+#define LZ4F_FLG_BLOCK_INDEP	BIT(5)
>+#define LZ4F_FLG_BLOCK_CSUM	BIT(4)
>+#define LZ4F_FLG_CONTENT_SIZE	BIT(3)
>+#define LZ4F_FLG_CONTENT_CSUM	BIT(2)
>+#define LZ4F_FLG_RESERVED	BIT(1)
>+#define LZ4F_FLG_DICT_ID	BIT(0)
>+
>+/* Frame descriptor, BD byte */
>+#define LZ4F_BD_BLOCK_MAXSIZE	GENMASK(6, 4)
>+#define LZ4F_BD_RESERVED	(BIT(7) | GENMASK(3, 0))
>+
>+/* Block size field: the top bit marks a block stored uncompressed */
>+#define LZ4F_BLOCK_UNCOMPRESSED	BIT(31)
>+
>+#define LZ4F_WINDOW_SIZE	SZ_64K
>+
>+/* Block maximum sizes for the BD byte's Block MaxSize values 4 to 7 */
>+static const u32 lz4f_block_max[] = { SZ_64K, SZ_256K, SZ_1M, SZ_4M };
>+
>+static int module_lz4_copy_out(struct load_info *info, const u8 *src,
>+			       size_t len, size_t *total)
>+{
>+	while (len) {
>+		size_t offset = *total % PAGE_SIZE;
>+		size_t chunk = min_t(size_t, len, PAGE_SIZE - offset);
>+		struct page *page;
>+
>+		if (offset)
>+			page = info->pages[info->used_pages - 1];
>+		else
>+			page = module_get_next_page(info);
>+		if (IS_ERR(page))
>+			return PTR_ERR(page);
>+
>+		memcpy_to_page(page, offset, src, chunk);
>+		src += chunk;
>+		len -= chunk;
>+		*total += chunk;
>+	}
>+
>+	return 0;
>+}
>+
>+static int module_lz4_verify_content(struct load_info *info, size_t total,
>+				     u32 expected)
>+{
>+	void *data;
>+	u32 csum;
>+
>+	data = vmap(info->pages, info->used_pages, VM_MAP, PAGE_KERNEL_RO);
>+	if (!data)
>+		return -ENOMEM;
>+	csum = xxh32(data, total, 0);
>+	vunmap(data);
>+
>+	if (csum != expected) {
>+		pr_err("LZ4-compressed data is corrupt (content checksum)\n");
>+		return -EINVAL;
>+	}
>+
>+	return 0;
>+}
>+
>+static ssize_t module_lz4_decompress(struct load_info *info,
>+				     const void *buf, size_t size)
>+{
>+	const u8 *in = buf, *end = in + size, *desc;
>+	size_t block_max, total = 0, dict_len = 0;
>+	u64 content_size = 0;
>+	u8 *scratch, *out;
>+	bool linked;
>+	u8 flg, bd;
>+	ssize_t retval;
>+
>+	if (size < 7 || get_unaligned_le32(in) != LZ4F_MAGIC) {
>+		pr_err("not an lz4 compressed module\n");
>+		return -EINVAL;
>+	}
>+
>+	desc = in + 4;
>+	flg = desc[0];
>+	bd = desc[1];
>+	in = desc + 2;
>+	if (FIELD_GET(LZ4F_FLG_VERSION, flg) != 1 ||
>+	    (flg & LZ4F_FLG_RESERVED) || (bd & LZ4F_BD_RESERVED) ||
>+	    FIELD_GET(LZ4F_BD_BLOCK_MAXSIZE, bd) < 4) {
>+		pr_err("LZ4-compressed data has an unsupported frame descriptor\n");

When would this be reached?

>+		return -EINVAL;
>+	}
>+	if (flg & LZ4F_FLG_DICT_ID) {
>+		pr_err("LZ4-compressed data needs a dictionary\n");
>+		return -EINVAL;
>+	}
>+	if (flg & LZ4F_FLG_CONTENT_SIZE) {
>+		if (end - in < 8)
>+			goto truncated_header;
>+		content_size = get_unaligned_le64(in);
>+		in += 8;
>+	}
>+	if (end - in < 1)
>+		goto truncated_header;
>+	if (*in != ((xxh32(desc, in - desc, 0) >> 8) & 0xff)) {
>+		pr_err("LZ4-compressed data has a corrupt frame descriptor\n");

Is there a mechanism in the future in order to understand the garbled crap
and fix it to make it manageable? (This can be future patch)

>+		return -EINVAL;
>+	}
>+	in++;
>+
>+	block_max = lz4f_block_max[FIELD_GET(LZ4F_BD_BLOCK_MAXSIZE, bd) - 4];
>+	linked = !(flg & LZ4F_FLG_BLOCK_INDEP);
>+
>+	scratch = kvmalloc((linked ? LZ4F_WINDOW_SIZE : 0) + block_max,
>+			   GFP_KERNEL);
>+	if (!scratch)
>+		return -ENOMEM;
>+	out = scratch + (linked ? LZ4F_WINDOW_SIZE : 0);
>+
>+	for (;;) {
>+		u32 block_size;
>+		size_t len;
>+		int ret;
>+
>+		if (end - in < 4)
>+			goto truncated;
>+		block_size = get_unaligned_le32(in);
>+		in += 4;
>+		if (!block_size)
>+			break;
>+
>+		len = block_size & ~LZ4F_BLOCK_UNCOMPRESSED;
>+		if (len > block_max) {
>+			pr_err("LZ4-compressed data has an oversized block\n");
>+			retval = -EINVAL;
>+			goto out;
>+		}
>+		if ((size_t)(end - in) < len +
>+		    (flg & LZ4F_FLG_BLOCK_CSUM ? 4 : 0))
>+			goto truncated;
>+
>+		if ((flg & LZ4F_FLG_BLOCK_CSUM) &&
>+		    xxh32(in, len, 0) != get_unaligned_le32(in + len)) {
>+			pr_err("LZ4-compressed data is corrupt (block checksum)\n");
>+			retval = -EINVAL;
>+			goto out;
>+		}
>+
>+		if (block_size & LZ4F_BLOCK_UNCOMPRESSED) {
>+			memcpy(out, in, len);
>+			ret = len;
>+		} else if (linked) {
>+			ret = LZ4_decompress_safe_usingDict(in, out, len,
>+							    block_max,
>+							    out - dict_len,
>+							    dict_len);
>+		} else {
>+			ret = LZ4_decompress_safe(in, out, len, block_max);
>+		}
>+		if (ret < 0) {
>+			pr_err("LZ4-decompression failed\n");
>+			retval = -EINVAL;
>+			goto out;
>+		}
>+
>+		retval = module_lz4_copy_out(info, out, ret, &total);
>+		if (retval)
>+			goto out;
>+
>+		if (linked) {
>+			/* Keep the last 64 KiB as the next block's dictionary. */
>+			size_t keep = min_t(size_t, dict_len + ret,
>+					    LZ4F_WINDOW_SIZE);
>+
>+			memmove(out - keep, out + ret - keep, keep);
>+			dict_len = keep;
>+		}
>+
>+		in += len;
>+		if (flg & LZ4F_FLG_BLOCK_CSUM)
>+			in += 4;
>+	}
>+
>+	if (!total) {
>+		pr_err("LZ4-compressed module is empty\n");
>+		retval = -EINVAL;
>+		goto out;
>+	}
>+
>+	if (flg & LZ4F_FLG_CONTENT_CSUM) {
>+		if (end - in < 4)
>+			goto truncated;
>+		retval = module_lz4_verify_content(info, total,
>+						   get_unaligned_le32(in));
>+		if (retval)
>+			goto out;
>+		in += 4;
>+	}
>+
>+	if (in != end) {
>+		pr_err("LZ4-compressed module has trailing data\n");
>+		retval = -EINVAL;
>+		goto out;
>+	}
>+
>+	if ((flg & LZ4F_FLG_CONTENT_SIZE) && content_size != total) {
>+		pr_err("LZ4-compressed data has a wrong content size\n");
>+		retval = -EINVAL;
>+		goto out;
>+	}
>+
>+	retval = total;
>+	goto out;
>+
>+truncated:
>+	pr_err("LZ4-compressed data is truncated\n");
>+	retval = -EINVAL;
>+out:
>+	kvfree(scratch);
>+	return retval;
>+
>+truncated_header:
>+	pr_err("LZ4-compressed data is truncated\n");
>+	return -EINVAL;
>+}
> #else
> #error "Unexpected configuration for CONFIG_MODULE_DECOMPRESS"
> #endif

My concern here is that some of these gnarly functions are uncommented, so
when a noob reads this code, "Oh no! What does this do! Oops... I broke my
module"


Maybe comment for functions that are gnarly and you think a reader would
need to understand to be able to operate/contribute.

>diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
>index 9ba45e5b32b18..aebc5e89d0e75 100644
>--- a/scripts/Makefile.modinst
>+++ b/scripts/Makefile.modinst
>@@ -57,6 +57,7 @@ ifdef CONFIG_MODULE_COMPRESS_ALL
> suffix-$(CONFIG_MODULE_COMPRESS_GZIP)	:= .gz
> suffix-$(CONFIG_MODULE_COMPRESS_XZ)	:= .xz
> suffix-$(CONFIG_MODULE_COMPRESS_ZSTD)	:= .zst
>+suffix-$(CONFIG_MODULE_COMPRESS_LZ4)	:= .lz4
> endif
> 
> modules := $(patsubst %.o, $(dst)/%.ko$(suffix-y), $(modules))
>@@ -151,6 +152,8 @@ quiet_cmd_xz = XZ      $@
>       cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
> quiet_cmd_zstd = ZSTD    $@
>       cmd_zstd = $(ZSTD) --rm -f -q $<
>+quiet_cmd_lz4 = LZ4     $@
>+      cmd_lz4 = $(LZ4) -12 -B4 -BD --rm -f -q $< $@
> 
> $(dst)/%.ko.gz: $(dst)/%.ko FORCE
> 	$(call cmd,gzip)
>@@ -161,6 +164,9 @@ $(dst)/%.ko.xz: $(dst)/%.ko FORCE
> $(dst)/%.ko.zst: $(dst)/%.ko FORCE
> 	$(call cmd,zstd)
> 
>+$(dst)/%.ko.lz4: $(dst)/%.ko FORCE
>+	$(call cmd,lz4)
>+
> PHONY += FORCE
> FORCE:
> 
>

--- Thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] module: Add LZ4 module compression
  2026-11-15 17:35 ` Bradley Morgan
@ 2026-11-15 21:09   ` Paul Menzel
  2026-11-15 21:16     ` Bradley Morgan
  0 siblings, 1 reply; 4+ messages in thread
From: Paul Menzel @ 2026-11-15 21:09 UTC (permalink / raw)
  To: Bradley Morgan
  Cc: atomlin, da.gomez, dsterba, linux-kbuild, linux-kernel,
	linux-modules, mcgrof, nathan, nsc, petr.pavlu, samitolvanen,
	terrelln

Dear Bradley,


Thank you for commenting on v2.


Am 15.11.26 um 18:35 schrieb Bradley Morgan:
> On 15 November 2026 12:04:13 GMT, Paul Menzel wrote:
>> XZ modules are small but slow to decompress in the kernel. Therefore,
>> support modules compressed in the LZ4 frame format (.ko.lz4), as written
>> by lz4(1). Decode block by block into a scratch buffer and copy into the
>> module pages. Linked blocks keep the last 64 KiB of output in front of
>> the scratch buffer as the dictionary. Verify the header checksum, block
>> checksums, content checksum and content size when the frame has them.
>> Frames with a dictionary ID are rejected.
> 
> .ko.lz4? when would this be used..?

With the first sentence and the table below, it would be used, if fast 
boot times are more important than space.

>> modules_install uses `lz4 -12 -B4 -BD`: 64 KiB linked blocks are as
>> small as 4 MiB blocks (i915.ko: 3587140 vs 3585364 bytes) and keep the
>> scratch buffer at 128 KiB.
>>
>> Userspace needs kmod with LZ4 support for depmod and modprobe [1].
>>
>> On a Dell XPS 13 9310 with an Intel Core i7-1165G7 (Tiger Lake) and an
>> NVMe SSD, decompressing and checking the signatures of the 237 modules
>> loaded during boot takes 624 ms of CPU time with XZ (i915: 91 ms) and
>> 131 ms with LZ4 (i915: 12 ms), measured by calling finit_module() again
>> for each loaded module. The LZ4 modules take 20.3 MB instead of 11.0 MB.
>> Both kernels are v7.4-rc1 with the same patches, including pending
>> lib/xz decoder speed-ups. Ten alternating boots of each kernel:
>>
>>   | ms (median)                               |  XZ  | LZ4  | change |
>>   +-------------------------------------------+------+------+--------+
>>   | early-i915 (modprobe i915 + dependencies) |  295 |   90 |   -205 |
>>   | first i915 message                        |  738 |  547 |   -192 |
>>   | GDM autologin                             |  973 |  876 |    -96 |
>>   | GRUB start to autologin                   | 1292 | 1197 |    -94 |
>>
>> QEMU 11.1.2 (KVM, AMD EPYC 7763, 8 vCPUs, module.sig_enforce=1, 10
>> rounds), loading the same 237 modules in dependency order:
>>
>>   | median                   |   XZ    |   LZ4   | uncompressed |
>>   +--------------------------+---------+---------+--------------+
>>   | one at a time            | 2082 ms | 1499 ms |      1461 ms |
>>   | 8 in parallel, like udev |  619 ms |  573 ms |       593 ms |
> 
> This table doesn't look very nicely formatted for me.

“nicely” is quite subjective, so it’d be great if you suggested your 
preferred format.

>> Link: https://github.com/kmod-project/kmod/pull/505 [1]
>> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
>> ---
>>
>> Notes:
>>     Changes in v2:
>>     - Declare retval as ssize_t, so output of 2 GiB or more is not
>>       truncated (Sashiko). The zstd decoder has the same bug, fixed by
>>       the separate patch "module/decompress: Use ssize_t for the zstd
>>       decompressed size".
>>     - Reject empty frames with -EINVAL; v1 failed vmap() of 0 pages with
>>       -ENOMEM (Sashiko).
>>     - Describe the decoder and link the frame format specification, group
>>       the definitions by FLG byte, BD byte and block size field, compare the
>>       version with FIELD_GET(), rename LZ4F_DICT_SIZE to LZ4F_WINDOW_SIZE,
>>       look up the block maximum size in a table, shorten the dictionary
>>       comment (Bradley Morgan).
>>     
>>     v1: https://lore.kernel.org/all/20261113144233.193782-1-pmenzel@molgen.mpg.de/
>>     
>>     Claude Opus 5.5 (Claude Code) wrote the decoder, the v2
>>     changes and this message. Tests: W=1 compile (GCC 16.2, Debian config
>>     for x86_64, tinyconfig for i386); QEMU with module.sig_enforce=1: 11 LZ4
>>     frame variants (64 KiB-4 MiB, linked/independent, block/content
>>     checksums, content size, stored blocks) load, 9 corrupt, truncated or
>>     empty files are rejected with EINVAL or EKEYREJECTED (v1: ENOMEM for the
>>     empty ones); the 237-module boot set loads in QEMU. v1 also loaded on a
>>     Dell XPS 13 9310 (20 boots); v2 was not booted on hardware.
> 
> Could you do a favour and test patches you submit, just in case of the
> edgecases :)

v1 was tested on real hardware, and for the v2 changes, I deem the QEMU 
tests enough.

>> kernel/module/Kconfig      |  13 +-
>> kernel/module/decompress.c | 243 +++++++++++++++++++++++++++++++++++++
>> scripts/Makefile.modinst   |   6 +
>> 3 files changed, 261 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
>> index 43b1bb01fd273..36f12612c74dd 100644
>> --- a/kernel/module/Kconfig
>> +++ b/kernel/module/Kconfig
>> @@ -344,7 +344,7 @@ config MODULE_COMPRESS
>> 	  This is fully compatible with signed modules.
>>
>> 	  The tool used to work with modules needs to support the selected
>> -	  compression type. kmod MAY support gzip, xz and zstd. Other tools
>> +	  compression type. kmod MAY support gzip, xz, zstd and lz4. Other tools
>> 	  might have a limited selection of the supported types.
>>
>> 	  Note that for modules inside an initrd or initramfs, it's more
>> @@ -377,6 +377,15 @@ config MODULE_COMPRESS_ZSTD
>> 	  Support modules compressed with ZSTD. The installed modules are
>> 	  suffixed with .ko.zst.
>>
>> +config MODULE_COMPRESS_LZ4
>> +	bool "LZ4"
>> +	help
>> +	  Support modules compressed with LZ4. The installed modules are
>> +	  suffixed with .ko.lz4.
>> +
>> +	  LZ4 modules are larger than XZ or ZSTD ones but decompress
>> +	  several times faster.
>> +
>> endchoice
>>
>> config MODULE_COMPRESS_ALL
>> @@ -394,6 +403,8 @@ config MODULE_DECOMPRESS
>> 	select ZLIB_INFLATE if MODULE_COMPRESS_GZIP
>> 	select XZ_DEC if MODULE_COMPRESS_XZ
>> 	select ZSTD_DECOMPRESS if MODULE_COMPRESS_ZSTD
>> +	select LZ4_DECOMPRESS if MODULE_COMPRESS_LZ4
>> +	select XXHASH if MODULE_COMPRESS_LZ4
>> 	help
>> 	  Support for decompressing kernel modules by the kernel itself
>> 	  instead of relying on userspace to perform this task. Useful when
>> diff --git a/kernel/module/decompress.c b/kernel/module/decompress.c
>> index cce098671be97..028b411015f21 100644
>> --- a/kernel/module/decompress.c
>> +++ b/kernel/module/decompress.c
>> @@ -287,6 +287,249 @@ static ssize_t module_zstd_decompress(struct load_info *info,
>> 	kvfree(wksp);
>> 	return retval;
>> }
>> +#elif defined(CONFIG_MODULE_COMPRESS_LZ4)
>> +#include <linux/bitfield.h>
>> +#include <linux/lz4.h>
>> +#include <linux/sizes.h>
>> +#include <linux/unaligned.h>
>> +#include <linux/xxhash.h>
>> +#define MODULE_COMPRESSION	lz4
>> +#define MODULE_DECOMPRESS_FN	module_lz4_decompress
>> +
> 
> Oh these includes mid file are a MESS! No, please no. Good to if defined,
> but I'm sure the maintainer wouldn't mind if you ifdefed those includes on
> the TOP of the file. Like any other normal kernel file.

It’s done the same way for the other variants, so I keep it.

Why so many includes? Unlike zstd, xz and zlib, lib/lz4 has only the 
block API (LZ4_decompress_safe*()), no frame decoder, so this parses the 
LZ4 frame format (v1.6.x) itself: frame descriptor, block sizes and the 
xxh32 header, block and content checksums. I am going to add that to the 
commit message of v3.

>> +/*
>> + * Decoder for the LZ4 frame format written by lz4(1), see
>> + * https://github.com/lz4/lz4/blob/dev/doc/lz4_Frame_format.md
>> + *
>> + * Each block is decoded into a scratch buffer and copied into the module
>> + * pages. A linked block may reference up to 64 KiB of the output before
>> + * it, so that much output is kept in front of the scratch buffer.
>> + */
> 
> Well, I suggested something better,
> 
> /* Decoder for the lz4 frame format,
>   * written by lz4 [1]
>   * This code adds support for lz4
>   * modules inside the kernel, so
>   * that lz4 modules can be used.
>   *
>   * [1] - (your link, lz4 frame format src??)
>   */
> 
> What about this?
> 
> I say bikeshed, but let's not make it TOO sloppy, you reckon?

No, I don’t. The other variants don’t have such a generic comment. So, 
I’d keep.

>> +#define LZ4F_MAGIC		0x184d2204
>> +
>> +/* Frame descriptor, FLG byte */
>> +#define LZ4F_FLG_VERSION	GENMASK(7, 6)
>> +#define LZ4F_FLG_BLOCK_INDEP	BIT(5)
>> +#define LZ4F_FLG_BLOCK_CSUM	BIT(4)
>> +#define LZ4F_FLG_CONTENT_SIZE	BIT(3)
>> +#define LZ4F_FLG_CONTENT_CSUM	BIT(2)
>> +#define LZ4F_FLG_RESERVED	BIT(1)
>> +#define LZ4F_FLG_DICT_ID	BIT(0)
>> +
>> +/* Frame descriptor, BD byte */
>> +#define LZ4F_BD_BLOCK_MAXSIZE	GENMASK(6, 4)
>> +#define LZ4F_BD_RESERVED	(BIT(7) | GENMASK(3, 0))
>> +
>> +/* Block size field: the top bit marks a block stored uncompressed */
>> +#define LZ4F_BLOCK_UNCOMPRESSED	BIT(31)
>> +
>> +#define LZ4F_WINDOW_SIZE	SZ_64K
>> +
>> +/* Block maximum sizes for the BD byte's Block MaxSize values 4 to 7 */
>> +static const u32 lz4f_block_max[] = { SZ_64K, SZ_256K, SZ_1M, SZ_4M };
>> +
>> +static int module_lz4_copy_out(struct load_info *info, const u8 *src,
>> +			       size_t len, size_t *total)
>> +{
>> +	while (len) {
>> +		size_t offset = *total % PAGE_SIZE;
>> +		size_t chunk = min_t(size_t, len, PAGE_SIZE - offset);
>> +		struct page *page;
>> +
>> +		if (offset)
>> +			page = info->pages[info->used_pages - 1];
>> +		else
>> +			page = module_get_next_page(info);
>> +		if (IS_ERR(page))
>> +			return PTR_ERR(page);
>> +
>> +		memcpy_to_page(page, offset, src, chunk);
>> +		src += chunk;
>> +		len -= chunk;
>> +		*total += chunk;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int module_lz4_verify_content(struct load_info *info, size_t total,
>> +				     u32 expected)
>> +{
>> +	void *data;
>> +	u32 csum;
>> +
>> +	data = vmap(info->pages, info->used_pages, VM_MAP, PAGE_KERNEL_RO);
>> +	if (!data)
>> +		return -ENOMEM;
>> +	csum = xxh32(data, total, 0);
>> +	vunmap(data);
>> +
>> +	if (csum != expected) {
>> +		pr_err("LZ4-compressed data is corrupt (content checksum)\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t module_lz4_decompress(struct load_info *info,
>> +				     const void *buf, size_t size)
>> +{
>> +	const u8 *in = buf, *end = in + size, *desc;
>> +	size_t block_max, total = 0, dict_len = 0;
>> +	u64 content_size = 0;
>> +	u8 *scratch, *out;
>> +	bool linked;
>> +	u8 flg, bd;
>> +	ssize_t retval;
>> +
>> +	if (size < 7 || get_unaligned_le32(in) != LZ4F_MAGIC) {
>> +		pr_err("not an lz4 compressed module\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	desc = in + 4;
>> +	flg = desc[0];
>> +	bd = desc[1];
>> +	in = desc + 2;
>> +	if (FIELD_GET(LZ4F_FLG_VERSION, flg) != 1 ||
>> +	    (flg & LZ4F_FLG_RESERVED) || (bd & LZ4F_BD_RESERVED) ||
>> +	    FIELD_GET(LZ4F_BD_BLOCK_MAXSIZE, bd) < 4) {
>> +		pr_err("LZ4-compressed data has an unsupported frame descriptor\n");
> 
> When would this be reached?

This would only be reached for input lz4/liblz4 never write. The frame 
format specification requires a decoder to reject. The Block MaxSize 
part also guards the lz4f_block_max[] index below, and as decompression 
runs before the signature check, the header has to be validated here. 
I'll split the bounds check out with its own comment in v3 to make this 
clearer.

>> +		return -EINVAL;
>> +	}
>> +	if (flg & LZ4F_FLG_DICT_ID) {
>> +		pr_err("LZ4-compressed data needs a dictionary\n");
>> +		return -EINVAL;
>> +	}
>> +	if (flg & LZ4F_FLG_CONTENT_SIZE) {
>> +		if (end - in < 8)
>> +			goto truncated_header;
>> +		content_size = get_unaligned_le64(in);
>> +		in += 8;
>> +	}
>> +	if (end - in < 1)
>> +		goto truncated_header;
>> +	if (*in != ((xxh32(desc, in - desc, 0) >> 8) & 0xff)) {
>> +		pr_err("LZ4-compressed data has a corrupt frame descriptor\n");
> 
> Is there a mechanism in the future in order to understand the garbled crap
> and fix it to make it manageable? (This can be future patch)

No mechanism is planned.

>> +		return -EINVAL;
>> +	}
>> +	in++;
>> +
>> +	block_max = lz4f_block_max[FIELD_GET(LZ4F_BD_BLOCK_MAXSIZE, bd) - 4];
>> +	linked = !(flg & LZ4F_FLG_BLOCK_INDEP);
>> +
>> +	scratch = kvmalloc((linked ? LZ4F_WINDOW_SIZE : 0) + block_max,
>> +			   GFP_KERNEL);
>> +	if (!scratch)
>> +		return -ENOMEM;
>> +	out = scratch + (linked ? LZ4F_WINDOW_SIZE : 0);
>> +
>> +	for (;;) {
>> +		u32 block_size;
>> +		size_t len;
>> +		int ret;
>> +
>> +		if (end - in < 4)
>> +			goto truncated;
>> +		block_size = get_unaligned_le32(in);
>> +		in += 4;
>> +		if (!block_size)
>> +			break;
>> +
>> +		len = block_size & ~LZ4F_BLOCK_UNCOMPRESSED;
>> +		if (len > block_max) {
>> +			pr_err("LZ4-compressed data has an oversized block\n");
>> +			retval = -EINVAL;
>> +			goto out;
>> +		}
>> +		if ((size_t)(end - in) < len +
>> +		    (flg & LZ4F_FLG_BLOCK_CSUM ? 4 : 0))
>> +			goto truncated;
>> +
>> +		if ((flg & LZ4F_FLG_BLOCK_CSUM) &&
>> +		    xxh32(in, len, 0) != get_unaligned_le32(in + len)) {
>> +			pr_err("LZ4-compressed data is corrupt (block checksum)\n");
>> +			retval = -EINVAL;
>> +			goto out;
>> +		}
>> +
>> +		if (block_size & LZ4F_BLOCK_UNCOMPRESSED) {
>> +			memcpy(out, in, len);
>> +			ret = len;
>> +		} else if (linked) {
>> +			ret = LZ4_decompress_safe_usingDict(in, out, len,
>> +							    block_max,
>> +							    out - dict_len,
>> +							    dict_len);
>> +		} else {
>> +			ret = LZ4_decompress_safe(in, out, len, block_max);
>> +		}
>> +		if (ret < 0) {
>> +			pr_err("LZ4-decompression failed\n");
>> +			retval = -EINVAL;
>> +			goto out;
>> +		}
>> +
>> +		retval = module_lz4_copy_out(info, out, ret, &total);
>> +		if (retval)
>> +			goto out;
>> +
>> +		if (linked) {
>> +			/* Keep the last 64 KiB as the next block's dictionary. */
>> +			size_t keep = min_t(size_t, dict_len + ret,
>> +					    LZ4F_WINDOW_SIZE);
>> +
>> +			memmove(out - keep, out + ret - keep, keep);
>> +			dict_len = keep;
>> +		}
>> +
>> +		in += len;
>> +		if (flg & LZ4F_FLG_BLOCK_CSUM)
>> +			in += 4;
>> +	}
>> +
>> +	if (!total) {
>> +		pr_err("LZ4-compressed module is empty\n");
>> +		retval = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	if (flg & LZ4F_FLG_CONTENT_CSUM) {
>> +		if (end - in < 4)
>> +			goto truncated;
>> +		retval = module_lz4_verify_content(info, total,
>> +						   get_unaligned_le32(in));
>> +		if (retval)
>> +			goto out;
>> +		in += 4;
>> +	}
>> +
>> +	if (in != end) {
>> +		pr_err("LZ4-compressed module has trailing data\n");
>> +		retval = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	if ((flg & LZ4F_FLG_CONTENT_SIZE) && content_size != total) {
>> +		pr_err("LZ4-compressed data has a wrong content size\n");
>> +		retval = -EINVAL;
>> +		goto out;
>> +	}
>> +
>> +	retval = total;
>> +	goto out;
>> +
>> +truncated:
>> +	pr_err("LZ4-compressed data is truncated\n");
>> +	retval = -EINVAL;
>> +out:
>> +	kvfree(scratch);
>> +	return retval;
>> +
>> +truncated_header:
>> +	pr_err("LZ4-compressed data is truncated\n");
>> +	return -EINVAL;
>> +}
>> #else
>> #error "Unexpected configuration for CONFIG_MODULE_DECOMPRESS"
>> #endif
> 
> My concern here is that some of these gnarly functions are uncommented, so
> when a noob reads this code, "Oh no! What does this do! Oops... I broke my
> module"
> 
> Maybe comment for functions that are gnarly and you think a reader would
> need to understand to be able to operate/contribute.

I think the decompressor is straight forward, and the other compression 
variants don’t do either, so I won’t add comments unless the maintainers 
ask me to.

>> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
>> index 9ba45e5b32b18..aebc5e89d0e75 100644
>> --- a/scripts/Makefile.modinst
>> +++ b/scripts/Makefile.modinst
>> @@ -57,6 +57,7 @@ ifdef CONFIG_MODULE_COMPRESS_ALL
>> suffix-$(CONFIG_MODULE_COMPRESS_GZIP)	:= .gz
>> suffix-$(CONFIG_MODULE_COMPRESS_XZ)	:= .xz
>> suffix-$(CONFIG_MODULE_COMPRESS_ZSTD)	:= .zst
>> +suffix-$(CONFIG_MODULE_COMPRESS_LZ4)	:= .lz4
>> endif
>>
>> modules := $(patsubst %.o, $(dst)/%.ko$(suffix-y), $(modules))
>> @@ -151,6 +152,8 @@ quiet_cmd_xz = XZ      $@
>>        cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
>> quiet_cmd_zstd = ZSTD    $@
>>        cmd_zstd = $(ZSTD) --rm -f -q $<
>> +quiet_cmd_lz4 = LZ4     $@
>> +      cmd_lz4 = $(LZ4) -12 -B4 -BD --rm -f -q $< $@
>>
>> $(dst)/%.ko.gz: $(dst)/%.ko FORCE
>> 	$(call cmd,gzip)
>> @@ -161,6 +164,9 @@ $(dst)/%.ko.xz: $(dst)/%.ko FORCE
>> $(dst)/%.ko.zst: $(dst)/%.ko FORCE
>> 	$(call cmd,zstd)
>>
>> +$(dst)/%.ko.lz4: $(dst)/%.ko FORCE
>> +	$(call cmd,lz4)
>> +
>> PHONY += FORCE
>> FORCE:

Thanks,

Paul


PS: Just a wish, that you’d use a more motivating commenting style. 
Still: Thank you for taking time to review patches.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] module: Add LZ4 module compression
  2026-11-15 21:09   ` Paul Menzel
@ 2026-11-15 21:16     ` Bradley Morgan
  0 siblings, 0 replies; 4+ messages in thread
From: Bradley Morgan @ 2026-11-15 21:16 UTC (permalink / raw)
  To: Paul Menzel
  Cc: atomlin, da.gomez, dsterba, linux-kbuild, linux-kernel,
	linux-modules, mcgrof, nathan, nsc, petr.pavlu, samitolvanen,
	terrelln

On 15 November 2026 21:09:09 GMT, Paul Menzel <pmenzel@molgen.mpg.de>
wrote:
>Dear Bradley,
>
>
>Thank you for commenting on v2.
>
>
>Am 15.11.26 um 18:35 schrieb Bradley Morgan:
>> On 15 November 2026 12:04:13 GMT, Paul Menzel wrote:
>>> XZ modules are small but slow to decompress in the kernel. Therefore,
>>> support modules compressed in the LZ4 frame format (.ko.lz4), as
>written
>>> by lz4(1). Decode block by block into a scratch buffer and copy into
>the
>>> module pages. Linked blocks keep the last 64 KiB of output in front of
>>> the scratch buffer as the dictionary. Verify the header checksum, block
>>> checksums, content checksum and content size when the frame has them.
>>> Frames with a dictionary ID are rejected.
>> 
>> .ko.lz4? when would this be used..?
>
>With the first sentence and the table below, it would be used, if fast
>boot times are more important than space.
>
>>> modules_install uses `lz4 -12 -B4 -BD`: 64 KiB linked blocks are as
>>> small as 4 MiB blocks (i915.ko: 3587140 vs 3585364 bytes) and keep the
>>> scratch buffer at 128 KiB.
>>> 
>>> Userspace needs kmod with LZ4 support for depmod and modprobe [1].
>>> 
>>> On a Dell XPS 13 9310 with an Intel Core i7-1165G7 (Tiger Lake) and an
>>> NVMe SSD, decompressing and checking the signatures of the 237 modules
>>> loaded during boot takes 624 ms of CPU time with XZ (i915: 91 ms) and
>>> 131 ms with LZ4 (i915: 12 ms), measured by calling finit_module() again
>>> for each loaded module. The LZ4 modules take 20.3 MB instead of 11.0
>MB.
>>> Both kernels are v7.4-rc1 with the same patches, including pending
>>> lib/xz decoder speed-ups. Ten alternating boots of each kernel:
>>> 
>>>   | ms (median)                               |  XZ  | LZ4  | change |
>>>   +-------------------------------------------+------+------+--------+
>>>   | early-i915 (modprobe i915 + dependencies) |  295 |   90 |   -205 |
>>>   | first i915 message                        |  738 |  547 |   -192 |
>>>   | GDM autologin                             |  973 |  876 |    -96 |
>>>   | GRUB start to autologin                   | 1292 | 1197 |    -94 |
>>> 
>>> QEMU 11.1.2 (KVM, AMD EPYC 7763, 8 vCPUs, module.sig_enforce=1, 10
>>> rounds), loading the same 237 modules in dependency order:
>>> 
>>>   | median                   |   XZ    |   LZ4   | uncompressed |
>>>   +--------------------------+---------+---------+--------------+
>>>   | one at a time            | 2082 ms | 1499 ms |      1461 ms |
>>>   | 8 in parallel, like udev |  619 ms |  573 ms |       593 ms |
>> 
>> This table doesn't look very nicely formatted for me.
>
>“nicely” is quite subjective, so it’d be great if you suggested your
>preferred format.

It looks very mangled to me.

>
>>> Link: https://github.com/kmod-project/kmod/pull/505 [1]
>>> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
>>> ---
>>> 
>>> Notes:
>>>     Changes in v2:
>>>     - Declare retval as ssize_t, so output of 2 GiB or more is not
>>>       truncated (Sashiko). The zstd decoder has the same bug, fixed by
>>>       the separate patch "module/decompress: Use ssize_t for the zstd
>>>       decompressed size".
>>>     - Reject empty frames with -EINVAL; v1 failed vmap() of 0 pages
>with
>>>       -ENOMEM (Sashiko).
>>>     - Describe the decoder and link the frame format specification,
>group
>>>       the definitions by FLG byte, BD byte and block size field,
>compare the
>>>       version with FIELD_GET(), rename LZ4F_DICT_SIZE to
>LZ4F_WINDOW_SIZE,
>>>       look up the block maximum size in a table, shorten the dictionary
>>>       comment (Bradley Morgan).
>>>         v1:
>https://lore.kernel.org/all/20261113144233.193782-1-pmenzel@molgen.mpg.de/
>>>         Claude Opus 5.5 (Claude Code) wrote the decoder, the v2
>>>     changes and this message. Tests: W=1 compile (GCC 16.2, Debian
>config
>>>     for x86_64, tinyconfig for i386); QEMU with module.sig_enforce=1:
>11 LZ4
>>>     frame variants (64 KiB-4 MiB, linked/independent, block/content
>>>     checksums, content size, stored blocks) load, 9 corrupt, truncated
>or
>>>     empty files are rejected with EINVAL or EKEYREJECTED (v1: ENOMEM
>for the
>>>     empty ones); the 237-module boot set loads in QEMU. v1 also loaded
>on a
>>>     Dell XPS 13 9310 (20 boots); v2 was not booted on hardware.
>> 
>> Could you do a favour and test patches you submit, just in case of the
>> edgecases :)
>
>v1 was tested on real hardware, and for the v2 changes, I deem the QEMU
>tests enough.
>
>>> kernel/module/Kconfig      |  13 +-
>>> kernel/module/decompress.c | 243 +++++++++++++++++++++++++++++++++++++
>>> scripts/Makefile.modinst   |   6 +
>>> 3 files changed, 261 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
>>> index 43b1bb01fd273..36f12612c74dd 100644
>>> --- a/kernel/module/Kconfig
>>> +++ b/kernel/module/Kconfig
>>> @@ -344,7 +344,7 @@ config MODULE_COMPRESS
>>> 	  This is fully compatible with signed modules.
>>> 
>>> 	  The tool used to work with modules needs to support the selected
>>> -	  compression type. kmod MAY support gzip, xz and zstd. Other tools
>>> +	  compression type. kmod MAY support gzip, xz, zstd and lz4. Other tools
>>> 	  might have a limited selection of the supported types.
>>> 
>>> 	  Note that for modules inside an initrd or initramfs, it's more
>>> @@ -377,6 +377,15 @@ config MODULE_COMPRESS_ZSTD
>>> 	  Support modules compressed with ZSTD. The installed modules are
>>> 	  suffixed with .ko.zst.
>>> 
>>> +config MODULE_COMPRESS_LZ4
>>> +	bool "LZ4"
>>> +	help
>>> +	  Support modules compressed with LZ4. The installed modules are
>>> +	  suffixed with .ko.lz4.
>>> +
>>> +	  LZ4 modules are larger than XZ or ZSTD ones but decompress
>>> +	  several times faster.
>>> +
>>> endchoice
>>> 
>>> config MODULE_COMPRESS_ALL
>>> @@ -394,6 +403,8 @@ config MODULE_DECOMPRESS
>>> 	select ZLIB_INFLATE if MODULE_COMPRESS_GZIP
>>> 	select XZ_DEC if MODULE_COMPRESS_XZ
>>> 	select ZSTD_DECOMPRESS if MODULE_COMPRESS_ZSTD
>>> +	select LZ4_DECOMPRESS if MODULE_COMPRESS_LZ4
>>> +	select XXHASH if MODULE_COMPRESS_LZ4
>>> 	help
>>> 	  Support for decompressing kernel modules by the kernel itself
>>> 	  instead of relying on userspace to perform this task. Useful when
>>> diff --git a/kernel/module/decompress.c b/kernel/module/decompress.c
>>> index cce098671be97..028b411015f21 100644
>>> --- a/kernel/module/decompress.c
>>> +++ b/kernel/module/decompress.c
>>> @@ -287,6 +287,249 @@ static ssize_t module_zstd_decompress(struct
>load_info *info,
>>> 	kvfree(wksp);
>>> 	return retval;
>>> }
>>> +#elif defined(CONFIG_MODULE_COMPRESS_LZ4)
>>> +#include <linux/bitfield.h>
>>> +#include <linux/lz4.h>
>>> +#include <linux/sizes.h>
>>> +#include <linux/unaligned.h>
>>> +#include <linux/xxhash.h>
>>> +#define MODULE_COMPRESSION	lz4
>>> +#define MODULE_DECOMPRESS_FN	module_lz4_decompress
>>> +
>> 
>> Oh these includes mid file are a MESS! No, please no. Good to if
>defined,
>> but I'm sure the maintainer wouldn't mind if you ifdefed those includes
>on
>> the TOP of the file. Like any other normal kernel file.
>
>It’s done the same way for the other variants, so I keep it.
>
>Why so many includes? Unlike zstd, xz and zlib, lib/lz4 has only the block
>API (LZ4_decompress_safe*()), no frame decoder, so this parses the LZ4
>frame format (v1.6.x) itself: frame descriptor, block sizes and the xxh32
>header, block and content checksums. I am going to add that to the commit
>message of v3.
>
>>> +/*
>>> + * Decoder for the LZ4 frame format written by lz4(1), see
>>> + * https://github.com/lz4/lz4/blob/dev/doc/lz4_Frame_format.md
>>> + *
>>> + * Each block is decoded into a scratch buffer and copied into the
>module
>>> + * pages. A linked block may reference up to 64 KiB of the output
>before
>>> + * it, so that much output is kept in front of the scratch buffer.
>>> + */
>> 
>> Well, I suggested something better,
>> 
>> /* Decoder for the lz4 frame format,
>>   * written by lz4 [1]
>>   * This code adds support for lz4
>>   * modules inside the kernel, so
>>   * that lz4 modules can be used.
>>   *
>>   * [1] - (your link, lz4 frame format src??)
>>   */
>> 
>> What about this?
>> 
>> I say bikeshed, but let's not make it TOO sloppy, you reckon?
>
>No, I don’t. The other variants don’t have such a generic comment. So, I’d
>keep.

fine.

>
>>> +#define LZ4F_MAGIC		0x184d2204
>>> +
>>> +/* Frame descriptor, FLG byte */
>>> +#define LZ4F_FLG_VERSION	GENMASK(7, 6)
>>> +#define LZ4F_FLG_BLOCK_INDEP	BIT(5)
>>> +#define LZ4F_FLG_BLOCK_CSUM	BIT(4)
>>> +#define LZ4F_FLG_CONTENT_SIZE	BIT(3)
>>> +#define LZ4F_FLG_CONTENT_CSUM	BIT(2)
>>> +#define LZ4F_FLG_RESERVED	BIT(1)
>>> +#define LZ4F_FLG_DICT_ID	BIT(0)
>>> +
>>> +/* Frame descriptor, BD byte */
>>> +#define LZ4F_BD_BLOCK_MAXSIZE	GENMASK(6, 4)
>>> +#define LZ4F_BD_RESERVED	(BIT(7) | GENMASK(3, 0))
>>> +
>>> +/* Block size field: the top bit marks a block stored uncompressed */
>>> +#define LZ4F_BLOCK_UNCOMPRESSED	BIT(31)
>>> +
>>> +#define LZ4F_WINDOW_SIZE	SZ_64K
>>> +
>>> +/* Block maximum sizes for the BD byte's Block MaxSize values 4 to 7
>*/
>>> +static const u32 lz4f_block_max[] = { SZ_64K, SZ_256K, SZ_1M, SZ_4M };
>>> +
>>> +static int module_lz4_copy_out(struct load_info *info, const u8 *src,
>>> +			       size_t len, size_t *total)
>>> +{
>>> +	while (len) {
>>> +		size_t offset = *total % PAGE_SIZE;
>>> +		size_t chunk = min_t(size_t, len, PAGE_SIZE - offset);
>>> +		struct page *page;
>>> +
>>> +		if (offset)
>>> +			page = info->pages[info->used_pages - 1];
>>> +		else
>>> +			page = module_get_next_page(info);
>>> +		if (IS_ERR(page))
>>> +			return PTR_ERR(page);
>>> +
>>> +		memcpy_to_page(page, offset, src, chunk);
>>> +		src += chunk;
>>> +		len -= chunk;
>>> +		*total += chunk;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int module_lz4_verify_content(struct load_info *info, size_t
>total,
>>> +				     u32 expected)
>>> +{
>>> +	void *data;
>>> +	u32 csum;
>>> +
>>> +	data = vmap(info->pages, info->used_pages, VM_MAP, PAGE_KERNEL_RO);
>>> +	if (!data)
>>> +		return -ENOMEM;
>>> +	csum = xxh32(data, total, 0);
>>> +	vunmap(data);
>>> +
>>> +	if (csum != expected) {
>>> +		pr_err("LZ4-compressed data is corrupt (content checksum)\n");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static ssize_t module_lz4_decompress(struct load_info *info,
>>> +				     const void *buf, size_t size)
>>> +{
>>> +	const u8 *in = buf, *end = in + size, *desc;
>>> +	size_t block_max, total = 0, dict_len = 0;
>>> +	u64 content_size = 0;
>>> +	u8 *scratch, *out;
>>> +	bool linked;
>>> +	u8 flg, bd;
>>> +	ssize_t retval;
>>> +
>>> +	if (size < 7 || get_unaligned_le32(in) != LZ4F_MAGIC) {
>>> +		pr_err("not an lz4 compressed module\n");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	desc = in + 4;
>>> +	flg = desc[0];
>>> +	bd = desc[1];
>>> +	in = desc + 2;
>>> +	if (FIELD_GET(LZ4F_FLG_VERSION, flg) != 1 ||
>>> +	    (flg & LZ4F_FLG_RESERVED) || (bd & LZ4F_BD_RESERVED) ||
>>> +	    FIELD_GET(LZ4F_BD_BLOCK_MAXSIZE, bd) < 4) {
>>> +		pr_err("LZ4-compressed data has an unsupported frame descriptor\n");
>> 
>> When would this be reached?
>
>This would only be reached for input lz4/liblz4 never write. The frame
>format specification requires a decoder to reject. The Block MaxSize part
>also guards the lz4f_block_max[] index below, and as decompression runs
>before the signature check, the header has to be validated here. I'll
>split the bounds check out with its own comment in v3 to make this
>clearer.

Thx

>
>>> +		return -EINVAL;
>>> +	}
>>> +	if (flg & LZ4F_FLG_DICT_ID) {
>>> +		pr_err("LZ4-compressed data needs a dictionary\n");
>>> +		return -EINVAL;
>>> +	}
>>> +	if (flg & LZ4F_FLG_CONTENT_SIZE) {
>>> +		if (end - in < 8)
>>> +			goto truncated_header;
>>> +		content_size = get_unaligned_le64(in);
>>> +		in += 8;
>>> +	}
>>> +	if (end - in < 1)
>>> +		goto truncated_header;
>>> +	if (*in != ((xxh32(desc, in - desc, 0) >> 8) & 0xff)) {
>>> +		pr_err("LZ4-compressed data has a corrupt frame descriptor\n");
>> 
>> Is there a mechanism in the future in order to understand the garbled
>crap
>> and fix it to make it manageable? (This can be future patch)
>
>No mechanism is planned.

Ok.

>
>>> +		return -EINVAL;
>>> +	}
>>> +	in++;
>>> +
>>> +	block_max = lz4f_block_max[FIELD_GET(LZ4F_BD_BLOCK_MAXSIZE, bd) - 4];
>>> +	linked = !(flg & LZ4F_FLG_BLOCK_INDEP);
>>> +
>>> +	scratch = kvmalloc((linked ? LZ4F_WINDOW_SIZE : 0) + block_max,
>>> +			   GFP_KERNEL);
>>> +	if (!scratch)
>>> +		return -ENOMEM;
>>> +	out = scratch + (linked ? LZ4F_WINDOW_SIZE : 0);
>>> +
>>> +	for (;;) {
>>> +		u32 block_size;
>>> +		size_t len;
>>> +		int ret;
>>> +
>>> +		if (end - in < 4)
>>> +			goto truncated;
>>> +		block_size = get_unaligned_le32(in);
>>> +		in += 4;
>>> +		if (!block_size)
>>> +			break;
>>> +
>>> +		len = block_size & ~LZ4F_BLOCK_UNCOMPRESSED;
>>> +		if (len > block_max) {
>>> +			pr_err("LZ4-compressed data has an oversized block\n");
>>> +			retval = -EINVAL;
>>> +			goto out;
>>> +		}
>>> +		if ((size_t)(end - in) < len +
>>> +		    (flg & LZ4F_FLG_BLOCK_CSUM ? 4 : 0))
>>> +			goto truncated;
>>> +
>>> +		if ((flg & LZ4F_FLG_BLOCK_CSUM) &&
>>> +		    xxh32(in, len, 0) != get_unaligned_le32(in + len)) {
>>> +			pr_err("LZ4-compressed data is corrupt (block checksum)\n");
>>> +			retval = -EINVAL;
>>> +			goto out;
>>> +		}
>>> +
>>> +		if (block_size & LZ4F_BLOCK_UNCOMPRESSED) {
>>> +			memcpy(out, in, len);
>>> +			ret = len;
>>> +		} else if (linked) {
>>> +			ret = LZ4_decompress_safe_usingDict(in, out, len,
>>> +							    block_max,
>>> +							    out - dict_len,
>>> +							    dict_len);
>>> +		} else {
>>> +			ret = LZ4_decompress_safe(in, out, len, block_max);
>>> +		}
>>> +		if (ret < 0) {
>>> +			pr_err("LZ4-decompression failed\n");
>>> +			retval = -EINVAL;
>>> +			goto out;
>>> +		}
>>> +
>>> +		retval = module_lz4_copy_out(info, out, ret, &total);
>>> +		if (retval)
>>> +			goto out;
>>> +
>>> +		if (linked) {
>>> +			/* Keep the last 64 KiB as the next block's dictionary. */
>>> +			size_t keep = min_t(size_t, dict_len + ret,
>>> +					    LZ4F_WINDOW_SIZE);
>>> +
>>> +			memmove(out - keep, out + ret - keep, keep);
>>> +			dict_len = keep;
>>> +		}
>>> +
>>> +		in += len;
>>> +		if (flg & LZ4F_FLG_BLOCK_CSUM)
>>> +			in += 4;
>>> +	}
>>> +
>>> +	if (!total) {
>>> +		pr_err("LZ4-compressed module is empty\n");
>>> +		retval = -EINVAL;
>>> +		goto out;
>>> +	}
>>> +
>>> +	if (flg & LZ4F_FLG_CONTENT_CSUM) {
>>> +		if (end - in < 4)
>>> +			goto truncated;
>>> +		retval = module_lz4_verify_content(info, total,
>>> +						   get_unaligned_le32(in));
>>> +		if (retval)
>>> +			goto out;
>>> +		in += 4;
>>> +	}
>>> +
>>> +	if (in != end) {
>>> +		pr_err("LZ4-compressed module has trailing data\n");
>>> +		retval = -EINVAL;
>>> +		goto out;
>>> +	}
>>> +
>>> +	if ((flg & LZ4F_FLG_CONTENT_SIZE) && content_size != total) {
>>> +		pr_err("LZ4-compressed data has a wrong content size\n");
>>> +		retval = -EINVAL;
>>> +		goto out;
>>> +	}
>>> +
>>> +	retval = total;
>>> +	goto out;
>>> +
>>> +truncated:
>>> +	pr_err("LZ4-compressed data is truncated\n");
>>> +	retval = -EINVAL;
>>> +out:
>>> +	kvfree(scratch);
>>> +	return retval;
>>> +
>>> +truncated_header:
>>> +	pr_err("LZ4-compressed data is truncated\n");
>>> +	return -EINVAL;
>>> +}
>>> #else
>>> #error "Unexpected configuration for CONFIG_MODULE_DECOMPRESS"
>>> #endif
>> 
>> My concern here is that some of these gnarly functions are uncommented,
>so
>> when a noob reads this code, "Oh no! What does this do! Oops... I broke
>my
>> module"
>> 
>> Maybe comment for functions that are gnarly and you think a reader would
>> need to understand to be able to operate/contribute.
>
>I think the decompressor is straight forward, and the other compression
>variants don’t do either, so I won’t add comments unless the maintainers
>ask me to.
>
>>> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
>>> index 9ba45e5b32b18..aebc5e89d0e75 100644
>>> --- a/scripts/Makefile.modinst
>>> +++ b/scripts/Makefile.modinst
>>> @@ -57,6 +57,7 @@ ifdef CONFIG_MODULE_COMPRESS_ALL
>>> suffix-$(CONFIG_MODULE_COMPRESS_GZIP)	:= .gz
>>> suffix-$(CONFIG_MODULE_COMPRESS_XZ)	:= .xz
>>> suffix-$(CONFIG_MODULE_COMPRESS_ZSTD)	:= .zst
>>> +suffix-$(CONFIG_MODULE_COMPRESS_LZ4)	:= .lz4
>>> endif
>>> 
>>> modules := $(patsubst %.o, $(dst)/%.ko$(suffix-y), $(modules))
>>> @@ -151,6 +152,8 @@ quiet_cmd_xz = XZ      $@
>>>        cmd_xz = $(XZ) --check=crc32 --lzma2=dict=1MiB -f $<
>>> quiet_cmd_zstd = ZSTD    $@
>>>        cmd_zstd = $(ZSTD) --rm -f -q $<
>>> +quiet_cmd_lz4 = LZ4     $@
>>> +      cmd_lz4 = $(LZ4) -12 -B4 -BD --rm -f -q $< $@
>>> 
>>> $(dst)/%.ko.gz: $(dst)/%.ko FORCE
>>> 	$(call cmd,gzip)
>>> @@ -161,6 +164,9 @@ $(dst)/%.ko.xz: $(dst)/%.ko FORCE
>>> $(dst)/%.ko.zst: $(dst)/%.ko FORCE
>>> 	$(call cmd,zstd)
>>> 
>>> +$(dst)/%.ko.lz4: $(dst)/%.ko FORCE
>>> +	$(call cmd,lz4)
>>> +
>>> PHONY += FORCE
>>> FORCE:
>
>Thanks,
>
>Paul
>
>
>PS: Just a wish, that you’d use a more motivating commenting style. Still: Thank you for taking time to review patches.

Wdym?

>

--- Thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-15 21:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-15 12:04 [PATCH v2] module: Add LZ4 module compression Paul Menzel
2026-11-15 17:35 ` Bradley Morgan
2026-11-15 21:09   ` Paul Menzel
2026-11-15 21:16     ` Bradley Morgan

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).