Linux Modules
 help / color / mirror / Atom feed
* Re: [PATCH v9 2/9] crypto: Add ML-DSA/Dilithium verify support
From: David Howells @ 2025-11-19 14:20 UTC (permalink / raw)
  To: Eric Biggers
  Cc: dhowells, Herbert Xu, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, linux-crypto,
	keyrings, linux-modules, linux-kernel
In-Reply-To: <20251119035905.GA1743@sol>

Eric Biggers <ebiggers@kernel.org> wrote:

> I've written an implementation of ML-DSA verification in about 600
> lines.  It may grow slightly as I clean it up and test it, but we
> definitely don't need 4800 lines.  I'll send it out once it's ready.

Could you post what you have now so that I can have a look at it, please?  It
doesn't matter if it's fully tested, but I can at least compare the algorithm
to that of Leancrypto.

David


^ permalink raw reply

* Re: [PATCH v3] ima: Access decompressed kernel module to verify appended signature
From: Coiby Xu @ 2025-11-19 14:05 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-integrity, Karel Srot, Paul Moore, Luis Chamberlain,
	Petr Pavlu, Daniel Gomez, Sami Tolvanen, Roberto Sassu,
	Dmitry Kasatkin, Eric Snowberg, James Morris, Serge E. Hallyn,
	Fan Wu, Stephen Smalley, Ondrej Mosnacek, open list,
	open list:MODULE SUPPORT, open list:SECURITY SUBSYSTEM,
	open list:SELINUX SECURITY MODULE
In-Reply-To: <fc1d67e411ef53460517db4c03bdcf1b9d9f8a8f.camel@linux.ibm.com>

On Wed, Nov 19, 2025 at 08:29:22AM -0500, Mimi Zohar wrote:
>Hi Coiby,

Hi Mimi,

>
>On Wed, 2025-11-19 at 11:47 +0800, Coiby Xu wrote:
>> Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
>> is enabled, IMA has no way to verify the appended module signature as it
>> can't decompress the module.
>>
>> Define a new kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
>> IMA can know only to collect original module data hash on
>> READING_MODULE_COMPRESSED and defer appraising/measuring it until on
>> READING_MODULE when the module has been decompressed.
>
>This paragraph is a bit awkward.  Perhaps something like:
>
>-> so IMA can calculate the compressed kernel module data hash and defer
>measuring/appraising ...
>
>>
>> Before enabling in-kernel module decompression, a kernel module in
>> initramfs can still be loaded with ima_policy=secure_boot. So adjust the
>> kernel module rule in secure_boot policy to allow either an IMA
>> signature OR an appended signature i.e. to use
>> "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
>>
>> Reported-by: Karel Srot <ksrot@redhat.com>
>> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
>> Suggested-by: Paul Moore <paul@paul-moore.com>
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>
>Thanks, Coiby!
>
>The patch applies cleanly to linus' tree, but needs to be applied to next-
>integrity.  Please re-base.

I've sent v4 which has been rebased onto next tree with improved
wording as suggested.

>
>-- 
>
>thanks,
>
>Mimi
>

-- 
Best regards,
Coiby


^ permalink raw reply

* [PATCH v4] ima: Access decompressed kernel module to verify appended signature
From: Coiby Xu @ 2025-11-19 14:03 UTC (permalink / raw)
  To: linux-integrity, Mimi Zohar
  Cc: Karel Srot, Paul Moore, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, James Morris, Serge E. Hallyn, Fan Wu,
	Stephen Smalley, Ondrej Mosnacek, open list,
	open list:MODULE SUPPORT, open list:SECURITY SUBSYSTEM,
	open list:SELINUX SECURITY MODULE
In-Reply-To: <20251031074016.1975356-1-coxu@redhat.com>

Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
is enabled, IMA has no way to verify the appended module signature as it
can't decompress the module.

Define a new kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
IMA can calculate the compressed kernel module data hash on
READING_MODULE_COMPRESSED and defer appraising/measuring it until on
READING_MODULE when the module has been decompressed.

Before enabling in-kernel module decompression, a kernel module in
initramfs can still be loaded with ima_policy=secure_boot. So adjust the
kernel module rule in secure_boot policy to allow either an IMA
signature OR an appended signature i.e. to use
"appraise func=MODULE_CHECK appraise_type=imasig|modsig".

Reported-by: Karel Srot <ksrot@redhat.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 include/linux/kernel_read_file.h    |  1 +
 kernel/module/main.c                | 17 ++++++++++++++---
 security/integrity/ima/ima_main.c   | 24 ++++++++++++++++--------
 security/integrity/ima/ima_policy.c |  3 ++-
 security/ipe/hooks.c                |  1 +
 security/selinux/hooks.c            |  5 +++--
 6 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 90451e2e12bd..d613a7b4dd35 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -14,6 +14,7 @@
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
 	id(POLICY, security-policy)		\
 	id(X509_CERTIFICATE, x509-certificate)	\
+	id(MODULE_COMPRESSED, kernel-module-compressed) \
 	id(MAX_ID, )
 
 #define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..7b3ec2fa6e7c 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3675,24 +3675,35 @@ static int idempotent_wait_for_completion(struct idempotent *u)
 
 static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
 {
+	bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
 	struct load_info info = { };
 	void *buf = NULL;
 	int len;
+	int err;
 
-	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
+	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
+			       compressed ? READING_MODULE_COMPRESSED :
+					    READING_MODULE);
 	if (len < 0) {
 		mod_stat_inc(&failed_kreads);
 		return len;
 	}
 
-	if (flags & MODULE_INIT_COMPRESSED_FILE) {
-		int err = module_decompress(&info, buf, len);
+	if (compressed) {
+		err = module_decompress(&info, buf, len);
 		vfree(buf); /* compressed data is no longer needed */
 		if (err) {
 			mod_stat_inc(&failed_decompress);
 			mod_stat_add_long(len, &invalid_decompress_bytes);
 			return err;
 		}
+		err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
+						     READING_MODULE);
+		if (err) {
+			mod_stat_inc(&failed_kreads);
+			free_copy(&info, flags);
+			return err;
+		}
 	} else {
 		info.hdr = buf;
 		info.len = len;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index ebaebccfbe9a..edd0fd3d77a0 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -235,7 +235,8 @@ static void ima_file_free(struct file *file)
 
 static int process_measurement(struct file *file, const struct cred *cred,
 			       struct lsm_prop *prop, char *buf, loff_t size,
-			       int mask, enum ima_hooks func)
+			       int mask, enum ima_hooks func,
+			       enum kernel_read_file_id read_id)
 {
 	struct inode *real_inode, *inode = file_inode(file);
 	struct ima_iint_cache *iint = NULL;
@@ -406,6 +407,12 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
 		goto out_locked;
 
+	/* Defer measuring/appraising kernel modules to READING_MODULE */
+	if (read_id == READING_MODULE_COMPRESSED) {
+		must_appraise = 0;
+		goto out_locked;
+	}
+
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
 
@@ -486,14 +493,14 @@ static int ima_file_mmap(struct file *file, unsigned long reqprot,
 
 	if (reqprot & PROT_EXEC) {
 		ret = process_measurement(file, current_cred(), &prop, NULL,
-					  0, MAY_EXEC, MMAP_CHECK_REQPROT);
+					  0, MAY_EXEC, MMAP_CHECK_REQPROT, 0);
 		if (ret)
 			return ret;
 	}
 
 	if (prot & PROT_EXEC)
 		return process_measurement(file, current_cred(), &prop, NULL,
-					   0, MAY_EXEC, MMAP_CHECK);
+					   0, MAY_EXEC, MMAP_CHECK, 0);
 
 	return 0;
 }
@@ -577,7 +584,7 @@ static int ima_bprm_check(struct linux_binprm *bprm)
 
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(bprm->file, current_cred(),
-				   &prop, NULL, 0, MAY_EXEC, BPRM_CHECK);
+				   &prop, NULL, 0, MAY_EXEC, BPRM_CHECK, 0);
 }
 
 /**
@@ -607,7 +614,7 @@ static int ima_creds_check(struct linux_binprm *bprm, const struct file *file)
 
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement((struct file *)file, bprm->cred, &prop, NULL,
-				   0, MAY_EXEC, CREDS_CHECK);
+				   0, MAY_EXEC, CREDS_CHECK, 0);
 }
 
 /**
@@ -655,7 +662,7 @@ static int ima_file_check(struct file *file, int mask)
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, NULL, 0,
 				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
-					   MAY_APPEND), FILE_CHECK);
+					   MAY_APPEND), FILE_CHECK, 0);
 }
 
 static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
@@ -874,12 +881,13 @@ static int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
 	func = read_idmap[read_id] ?: FILE_CHECK;
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, NULL, 0,
-				   MAY_READ, func);
+				   MAY_READ, func, 0);
 }
 
 const int read_idmap[READING_MAX_ID] = {
 	[READING_FIRMWARE] = FIRMWARE_CHECK,
 	[READING_MODULE] = MODULE_CHECK,
+	[READING_MODULE_COMPRESSED] = MODULE_CHECK,
 	[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
 	[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
 	[READING_POLICY] = POLICY_CHECK
@@ -917,7 +925,7 @@ static int ima_post_read_file(struct file *file, char *buf, loff_t size,
 	func = read_idmap[read_id] ?: FILE_CHECK;
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, buf, size,
-				   MAY_READ, func);
+				   MAY_READ, func, read_id);
 }
 
 /**
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 164d62832f8e..7468afaab686 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -244,7 +244,8 @@ static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
 
 static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	{.action = APPRAISE, .func = MODULE_CHECK,
-	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED |
+		  IMA_CHECK_BLACKLIST},
 	{.action = APPRAISE, .func = FIRMWARE_CHECK,
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
 	{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index d0323b81cd8f..1053a4acf589 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -118,6 +118,7 @@ int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
 		op = IPE_OP_FIRMWARE;
 		break;
 	case READING_MODULE:
+	case READING_MODULE_COMPRESSED:
 		op = IPE_OP_KERNEL_MODULE;
 		break;
 	case READING_KEXEC_INITRAMFS:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dfc22da42f30..c1ff69d5d76e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4275,7 +4275,7 @@ static int selinux_kernel_read_file(struct file *file,
 {
 	int rc = 0;
 
-	BUILD_BUG_ON_MSG(READING_MAX_ID > 7,
+	BUILD_BUG_ON_MSG(READING_MAX_ID > 8,
 			 "New kernel_read_file_id introduced; update SELinux!");
 
 	switch (id) {
@@ -4283,6 +4283,7 @@ static int selinux_kernel_read_file(struct file *file,
 		rc = selinux_kernel_load_from_file(file, SYSTEM__FIRMWARE_LOAD);
 		break;
 	case READING_MODULE:
+	case READING_MODULE_COMPRESSED:
 		rc = selinux_kernel_load_from_file(file, SYSTEM__MODULE_LOAD);
 		break;
 	case READING_KEXEC_IMAGE:
@@ -4311,7 +4312,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
 {
 	int rc = 0;
 
-	BUILD_BUG_ON_MSG(LOADING_MAX_ID > 7,
+	BUILD_BUG_ON_MSG(LOADING_MAX_ID > 8,
 			 "New kernel_load_data_id introduced; update SELinux!");
 
 	switch (id) {

base-commit: 43369273518f57b7d56c1cf12d636a809b7bd81b
-- 
2.51.1


^ permalink raw reply related

* Re: [PATCH v3] ima: Access decompressed kernel module to verify appended signature
From: Mimi Zohar @ 2025-11-19 13:29 UTC (permalink / raw)
  To: Coiby Xu, linux-integrity
  Cc: Karel Srot, Paul Moore, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, James Morris, Serge E. Hallyn, Fan Wu,
	Stephen Smalley, Ondrej Mosnacek, open list,
	open list:MODULE SUPPORT, open list:SECURITY SUBSYSTEM,
	open list:SELINUX SECURITY MODULE
In-Reply-To: <20251119034718.618008-1-coxu@redhat.com>

Hi Coiby,

On Wed, 2025-11-19 at 11:47 +0800, Coiby Xu wrote:
> Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
> is enabled, IMA has no way to verify the appended module signature as it
> can't decompress the module.
> 
> Define a new kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
> IMA can know only to collect original module data hash on
> READING_MODULE_COMPRESSED and defer appraising/measuring it until on
> READING_MODULE when the module has been decompressed.

This paragraph is a bit awkward.  Perhaps something like:

-> so IMA can calculate the compressed kernel module data hash and defer
measuring/appraising ...

> 
> Before enabling in-kernel module decompression, a kernel module in
> initramfs can still be loaded with ima_policy=secure_boot. So adjust the
> kernel module rule in secure_boot policy to allow either an IMA
> signature OR an appended signature i.e. to use
> "appraise func=MODULE_CHECK appraise_type=imasig|modsig".
> 
> Reported-by: Karel Srot <ksrot@redhat.com>
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> Suggested-by: Paul Moore <paul@paul-moore.com>
> Signed-off-by: Coiby Xu <coxu@redhat.com>

Thanks, Coiby!

The patch applies cleanly to linus' tree, but needs to be applied to next-
integrity.  Please re-base.

-- 

thanks,

Mimi

^ permalink raw reply

* Re: [PATCH v3 7/9] module: Move lockdown check into generic module loader
From: Sebastian Andrzej Siewior @ 2025-11-19 11:20 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Masahiro Yamada, Nathan Chancellor, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet,
	Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
	Christophe Leroy, Naveen N Rao, Mimi Zohar, Roberto Sassu,
	Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
	Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	Christian Heusel, Câju Mihai-Drosi, linux-kbuild,
	linux-kernel, linux-arch, linux-modules, linux-security-module,
	linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <20250429-module-hashes-v3-7-00e9258def9e@weissschuh.net>

On 2025-04-29 15:04:34 [+0200], Thomas Weißschuh wrote:
> The lockdown check buried in module_sig_check() will not compose well
> with the introduction of hash-based module validation.

An explanation of why would be nice. 

> Move it into module_integrity_check() which will work better.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

Sebastian

^ permalink raw reply

* Re: [PATCH v17 44/47] dept: introduce APIs to set page usage and use subclasses_evt for the usage
From: Byungchul Park @ 2025-11-19 10:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel_team, torvalds, damien.lemoal, linux-ide, adilger.kernel,
	linux-ext4, mingo, peterz, will, tglx, rostedt, joel, sashal,
	daniel.vetter, duyuyang, johannes.berg, tj, tytso, willy, david,
	amir73il, gregkh, kernel-team, linux-mm, akpm, mhocko, minchan,
	hannes, vdavydov.dev, sj, jglisse, dennis, cl, penberg, rientjes,
	vbabka, ngupta, linux-block, josef, linux-fsdevel, jack, jlayton,
	dan.j.williams, hch, djwong, dri-devel, rodrigosiqueiramelo,
	melissa.srw, hamohammed.sa, harry.yoo, chris.p.wilson,
	gwan-gyeong.mun, max.byungchul.park, boqun.feng, longman,
	yunseong.kim, ysk, yeoreum.yun, netdev, matthew.brost, her0gyugyu,
	corbet, catalin.marinas, bp, dave.hansen, x86, hpa, luto,
	sumit.semwal, gustavo, christian.koenig, andi.shyti, arnd,
	lorenzo.stoakes, Liam.Howlett, rppt, surenb, mcgrof, petr.pavlu,
	da.gomez, samitolvanen, paulmck, frederic, neeraj.upadhyay,
	joelagnelf, josh, urezki, mathieu.desnoyers, jiangshanlai,
	qiang.zhang, juri.lelli, vincent.guittot, dietmar.eggemann,
	bsegall, mgorman, vschneid, chuck.lever, neil, okorniev, Dai.Ngo,
	tom, trondmy, anna, kees, bigeasy, clrkwllms, mark.rutland,
	ada.coupriediaz, kristina.martsenko, wangkefeng.wang, broonie,
	kevin.brodsky, dwmw, shakeel.butt, ast, ziy, yuzhao, baolin.wang,
	usamaarif642, joel.granados, richard.weiyang, geert+renesas,
	tim.c.chen, linux, alexander.shishkin, lillian, chenhuacai,
	francesco, guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
	thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
	linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
	linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel
In-Reply-To: <20251002081247.51255-45-byungchul@sk.com>

On Thu, Oct 02, 2025 at 05:12:44PM +0900, Byungchul Park wrote:
> False positive reports have been observed since dept works with the
> assumption that all the pages have the same dept class, but the class
> should be split since the problematic call paths are different depending
> on what the page is used for.
> 
> At least, ones in block device's address_space and ones in regular
> file's address_space have exclusively different usages.
> 
> Thus, define usage candidates like:
> 
>    DEPT_PAGE_REGFILE_CACHE /* page in regular file's address_space */
>    DEPT_PAGE_BDEV_CACHE    /* page in block device's address_space */
>    DEPT_PAGE_DEFAULT       /* the others */

1. I'd like to annotate a page to DEPT_PAGE_REGFILE_CACHE when the page
   starts to be associated with a page cache for fs data.

2. And I'd like to annotate a page to DEPT_PAGE_BDEV_CACHE when the page
   starts to be associated with meta data of fs e.g. super block.

3. Lastly, I'd like to reset the annotated value if any, that has been
   set in the page, when the page ends the assoication with either page
   cache or meta block of fs e.g. freeing the page.

Can anyone suggest good places in code for the annotation 1, 2, 3?  It'd
be totally appreciated. :-)

	Byungchul

> Introduce APIs to set each page's usage properly and make sure not to
> interact between at least between DEPT_PAGE_REGFILE_CACHE and
> DEPT_PAGE_BDEV_CACHE.  However, besides the exclusive usages, allow any
> other combinations to interact to the other for example:
> 
>    PG_locked for DEPT_PAGE_DEFAULT page can wait for PG_locked for
>    DEPT_PAGE_REGFILE_CACHE page and vice versa.
> 
>    PG_locked for DEPT_PAGE_DEFAULT page can wait for PG_locked for
>    DEPT_PAGE_BDEV_CACHE page and vice versa.
> 
>    PG_locked for DEPT_PAGE_DEFAULT page can wait for PG_locked for
>    DEPT_PAGE_DEFAULT page.
> 
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> ---
>  include/linux/dept.h       | 31 +++++++++++++++-
>  include/linux/mm_types.h   |  1 +
>  include/linux/page-flags.h | 76 +++++++++++++++++++++++++++++++++++++-
>  3 files changed, 104 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/dept.h b/include/linux/dept.h
> index 0ac13129f308..fbbc41048fac 100644
> --- a/include/linux/dept.h
> +++ b/include/linux/dept.h
> @@ -21,8 +21,8 @@ struct task_struct;
>  #define DEPT_MAX_WAIT_HIST		64
>  #define DEPT_MAX_ECXT_HELD		48
>  
> -#define DEPT_MAX_SUBCLASSES		16
> -#define DEPT_MAX_SUBCLASSES_EVT		2
> +#define DEPT_MAX_SUBCLASSES		24
> +#define DEPT_MAX_SUBCLASSES_EVT		3
>  #define DEPT_MAX_SUBCLASSES_USR		(DEPT_MAX_SUBCLASSES / DEPT_MAX_SUBCLASSES_EVT)
>  #define DEPT_MAX_SUBCLASSES_CACHE	2
>  
> @@ -390,6 +390,32 @@ struct dept_ext_wgen {
>  	unsigned int wgen;
>  };
>  
> +enum {
> +	DEPT_PAGE_DEFAULT = 0,
> +	DEPT_PAGE_REGFILE_CACHE,	/* regular file page cache */
> +	DEPT_PAGE_BDEV_CACHE,		/* block device cache */
> +	DEPT_PAGE_USAGE_NR,		/* nr of usages options */
> +};
> +
> +#define DEPT_PAGE_USAGE_SHIFT 16
> +#define DEPT_PAGE_USAGE_MASK ((1U << DEPT_PAGE_USAGE_SHIFT) - 1)
> +#define DEPT_PAGE_USAGE_PENDING_MASK (DEPT_PAGE_USAGE_MASK << DEPT_PAGE_USAGE_SHIFT)
> +
> +/*
> + * Identify each page's usage type
> + */
> +struct dept_page_usage {
> +	/*
> +	 * low 16 bits  : the current usage type
> +	 * high 16 bits : usage type requested to be set
> +	 *
> +	 * Do not apply the type requested immediately but defer until
> +	 * after clearing PG_locked bit of the folio or page e.g. by
> +	 * folio_unlock().
> +	 */
> +	atomic_t type; /* Update and read atomically */
> +};
> +
>  struct dept_event_site {
>  	/*
>  	 * event site name
> @@ -562,6 +588,7 @@ extern void dept_hardirqs_off(void);
>  struct dept_key { };
>  struct dept_map { };
>  struct dept_ext_wgen { };
> +struct dept_page_usage { };
>  struct dept_event_site { };
>  
>  #define DEPT_MAP_INITIALIZER(n, k) { }
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 5ebc565309af..8ccbb030500c 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -224,6 +224,7 @@ struct page {
>  	struct page *kmsan_shadow;
>  	struct page *kmsan_origin;
>  #endif
> +	struct dept_page_usage usage;
>  	struct dept_ext_wgen pg_locked_wgen;
>  } _struct_page_alignment;
>  
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index d3c4954c4218..3fd3660ddc6f 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -204,6 +204,68 @@ enum pageflags {
>  
>  extern struct dept_map pg_locked_map;
>  
> +static inline int dept_set_page_usage(struct page *p,
> +		unsigned int new_type)
> +{
> +	unsigned int type = atomic_read(&p->usage.type);
> +
> +	if (WARN_ON_ONCE(new_type >= DEPT_PAGE_USAGE_NR))
> +		return -1;
> +
> +	new_type <<= DEPT_PAGE_USAGE_SHIFT;
> +retry:
> +	new_type &= ~DEPT_PAGE_USAGE_MASK;
> +	new_type |= type & DEPT_PAGE_USAGE_MASK;
> +
> +	if (!atomic_try_cmpxchg(&p->usage.type, &type, new_type))
> +		goto retry;
> +
> +	return 0;
> +}
> +
> +static inline int dept_reset_page_usage(struct page *p)
> +{
> +	return dept_set_page_usage(p, DEPT_PAGE_DEFAULT);
> +}
> +
> +static inline void dept_update_page_usage(struct page *p)
> +{
> +	unsigned int type = atomic_read(&p->usage.type);
> +	unsigned int new_type;
> +
> +retry:
> +	new_type = type & DEPT_PAGE_USAGE_PENDING_MASK;
> +	new_type >>= DEPT_PAGE_USAGE_SHIFT;
> +	new_type |= type & DEPT_PAGE_USAGE_PENDING_MASK;
> +
> +	/*
> +	 * Already updated by others.
> +	 */
> +	if (type == new_type)
> +		return;
> +
> +	if (!atomic_try_cmpxchg(&p->usage.type, &type, new_type))
> +		goto retry;
> +}
> +
> +static inline unsigned long dept_event_flags(struct page *p, bool wait)
> +{
> +	unsigned int type;
> +
> +	type = atomic_read(&p->usage.type) & DEPT_PAGE_USAGE_MASK;
> +
> +	if (WARN_ON_ONCE(type >= DEPT_PAGE_USAGE_NR))
> +		return 0;
> +
> +	/*
> +	 * event
> +	 */
> +	if (!wait)
> +		return 1UL << type;
> +
> +	return (1UL << DEPT_PAGE_DEFAULT) | (1UL << type);
> +}
> +
>  /*
>   * Place the following annotations in its suitable point in code:
>   *
> @@ -214,20 +276,28 @@ extern struct dept_map pg_locked_map;
>  
>  static inline void dept_page_set_bit(struct page *p, int bit_nr)
>  {
> +	dept_update_page_usage(p);
>  	if (bit_nr == PG_locked)
>  		dept_request_event(&pg_locked_map, &p->pg_locked_wgen);
>  }
>  
>  static inline void dept_page_clear_bit(struct page *p, int bit_nr)
>  {
> +	unsigned long evt_f;
> +
> +	evt_f = dept_event_flags(p, false);
>  	if (bit_nr == PG_locked)
> -		dept_event(&pg_locked_map, 1UL, _RET_IP_, __func__, &p->pg_locked_wgen);
> +		dept_event(&pg_locked_map, evt_f, _RET_IP_, __func__, &p->pg_locked_wgen);
>  }
>  
>  static inline void dept_page_wait_on_bit(struct page *p, int bit_nr)
>  {
> +	unsigned long evt_f;
> +
> +	dept_update_page_usage(p);
> +	evt_f = dept_event_flags(p, true);
>  	if (bit_nr == PG_locked)
> -		dept_wait(&pg_locked_map, 1UL, _RET_IP_, __func__, 0, -1L);
> +		dept_wait(&pg_locked_map, evt_f, _RET_IP_, __func__, 0, -1L);
>  }
>  
>  static inline void dept_folio_set_bit(struct folio *f, int bit_nr)
> @@ -245,6 +315,8 @@ static inline void dept_folio_wait_on_bit(struct folio *f, int bit_nr)
>  	dept_page_wait_on_bit(&f->page, bit_nr);
>  }
>  #else
> +#define dept_set_page_usage(p, t)		do { } while (0)
> +#define dept_reset_page_usage(p)		do { } while (0)
>  #define dept_page_set_bit(p, bit_nr)		do { } while (0)
>  #define dept_page_clear_bit(p, bit_nr)		do { } while (0)
>  #define dept_page_wait_on_bit(p, bit_nr)	do { } while (0)
> -- 
> 2.17.1

^ permalink raw reply

* Re: [PATCH v9 2/9] crypto: Add ML-DSA/Dilithium verify support
From: Eric Biggers @ 2025-11-19  3:59 UTC (permalink / raw)
  To: David Howells
  Cc: Herbert Xu, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, linux-crypto,
	keyrings, linux-modules, linux-kernel
In-Reply-To: <20251117201233.GA3993@sol>

On Mon, Nov 17, 2025 at 12:12:33PM -0800, Eric Biggers wrote:
> My point is that a smaller, cleaner, and more maintainable
> implementation of ML-DSA is possible,

I've written an implementation of ML-DSA verification in about 600
lines.  It may grow slightly as I clean it up and test it, but we
definitely don't need 4800 lines.  I'll send it out once it's ready.

By the way, your proposed KUnit test suite has no negative test cases :(
We can do better than that.

- Eric

^ permalink raw reply

* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Coiby Xu @ 2025-11-19  3:52 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Paul Moore, linux-integrity, linux-security-module, Karel Srot,
	James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <fca9a7b41a5e428fadfe2d7e3b004ada2763375c.camel@linux.ibm.com>

On Tue, Nov 18, 2025 at 07:19:50AM -0500, Mimi Zohar wrote:
>On Thu, 2025-11-13 at 12:06 +0800, Coiby Xu wrote:
>> On Fri, Nov 07, 2025 at 02:28:13PM -0500, Mimi Zohar wrote:
>> > On Thu, 2025-11-06 at 17:15 -0500, Mimi Zohar wrote:
>> > > On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
>> > > > On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
>> > > > > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
>> > > > [...]
>> > > > >
>> > > > > Hi Coiby,
>> > > > >
>> > > > > Based on the conversation with Paul, there is no reason to remove the existing
>> > > > > security_kernel_post_read_file() call.
>> > > > >
>> > > > > The changes are similar to the 2nd link, but a bit different.
>> > > > > - Define a single enumeration named READING_MODULE_COMPRESSED.
>> > > > >
>> > > > > - In module/main.c add a new security_kernel_post_read_file() call immediately
>> > > > > after decompressing the kernel module.  Like a previous version of this patch,
>> > > > > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
>> > > > > based on MODULE_INIT_COMPRESSED_FILE.
>> > > > >
>> > > > > - In ima_post_read_file() defer verifying the signature when the enumeration is
>> > > > > READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)
>> > > >
>> > > > Hi Mimi,
>> > > >
>> > > > Thanks for summarizing your conversation with Paul! I can confirm Paul's
>> > > > approach works
>> > > > https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
>> > > >
>> > > > While testing the patch today, I realized there is another
>> > > > issue/challenge introduced by in-kernel module decompression. IMA
>> > > > appraisal is to verify the digest of compressed kernel module but
>> > > > currently the passed buffer is uncompressed module. When IMA uses
>> > > > uncompressed module data to calculate the digest, xattr signature
>> > > > verification will fail. If we always make IMA read the original kernel
>> > > > module data again to calculate the digest, does it look like a
>> > > > quick-and-dirty fix? If we can assume people won't load kernel module so
>> > > > often, the performance impact is negligible. Otherwise we may have to
>> > > > introduce a new LSM hook so IMA can access uncompressed and original
>> > > > module data one time.
>> > >
>> > > ima_collect_measurement() stores the file hash info in the iint and uses that
>> > > information to verify the signature as stored in the security xattr.
>> > > Decompressing the kernel module shouldn't affect the xattr signature
>> > > verification.
>> >
>> > In the case when the compressed kernel module hasn't previously been measured or
>> > appraised before loading the kernel module, we need to "collect" the file data
>> > hash on READING_MODULE_COMPRESSED, but defer appraising/measuring it.
>> >
>> > An alternative to your suggestion of re-reading the original kernel module data
>> > to calculate the digest or defining a new hook, would be to define "collect" as
>> > a new "action" and pass the kernel_read_file_id enumeration to
>> > process_measurement().  IMA_COLLECTED already exists.  Only IMA_COLLECT would
>> > need to be defined.  The new collect "action" should be limited to
>> > func=MODULE_CHECK.
>> >
>> > The downside of this alternative is that it requires a new collect rule:
>> > collect func=MODULE_CHECK mask=MAY_READ uid=0
>> > appraise func=MODULE_CHECK appraise_type=imasig|modsig
>
>As it turns out, the "collect" rule is unnecessary.  On
>READING_MODULE_COMPRESSED, process_measurement() should calculate the compressed
>file hash.  Extending the IMA measurement list and verifying the signature can
>then be differed to READING_MODULE.
>
>>
>> Thank for suggesting an alternative! I've implemented the idea in
>> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_collect
>>
>> Note besides a new collect rule, another change is needed. Currently,
>> process_measurement only accepts enum ima_hooks thus it can't tell if
>> it's READING_MODULE_COMPRESSED so to only do collect action. So I
>> create a fake MODULE_COMPRESSED_CHECK func.
>
>Correct, either extending process_measurement() with the read_idmap enum or
>defining the fake hook would work.
>
>>
>> And for the idea of re-reading the original kernel module data, it has
>> been implemented in
>> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
>>
>> Both branches have applied your requested three changes including
>> respecting the 80 char line limit. Additionally, I made a change to the
>> IPE LSM because of the new READING_MODULE_COMPRESSED kernel_read_file_id
>> enumerate.
>>
>> After comparing the two implementations, personally I prefer re-reading
>> the original kernel module data because the change is smaller and it's
>> more user-friendly. But if there are other reasons I don't know, I'll
>> post the patches of the new collect action approach to the mailing list.
>
>The "re-reading" option fails some of the tests.  As the "collect" rule isn't
>needed, let's stick with the first option.

Thanks for evaluating the two options! Yeah, without the "collect" rule,
the 1st option is much better as it doesn't have the issue of re-reading
the module.

-- 
Best regards,
Coiby


^ permalink raw reply

* [PATCH v3] ima: Access decompressed kernel module to verify appended signature
From: Coiby Xu @ 2025-11-19  3:47 UTC (permalink / raw)
  To: linux-integrity, Mimi Zohar
  Cc: Karel Srot, Paul Moore, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, James Morris, Serge E. Hallyn, Fan Wu,
	Stephen Smalley, Ondrej Mosnacek, open list,
	open list:MODULE SUPPORT, open list:SECURITY SUBSYSTEM,
	open list:SELINUX SECURITY MODULE
In-Reply-To: <20251031074016.1975356-1-coxu@redhat.com>

Currently, when in-kernel module decompression (CONFIG_MODULE_DECOMPRESS)
is enabled, IMA has no way to verify the appended module signature as it
can't decompress the module.

Define a new kernel_read_file_id enumerate READING_MODULE_COMPRESSED so
IMA can know only to collect original module data hash on
READING_MODULE_COMPRESSED and defer appraising/measuring it until on
READING_MODULE when the module has been decompressed.

Before enabling in-kernel module decompression, a kernel module in
initramfs can still be loaded with ima_policy=secure_boot. So adjust the
kernel module rule in secure_boot policy to allow either an IMA
signature OR an appended signature i.e. to use
"appraise func=MODULE_CHECK appraise_type=imasig|modsig".

Reported-by: Karel Srot <ksrot@redhat.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Suggested-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 include/linux/kernel_read_file.h    |  1 +
 kernel/module/main.c                | 17 ++++++++++++++---
 security/integrity/ima/ima_main.c   | 24 ++++++++++++++++--------
 security/integrity/ima/ima_policy.c |  3 ++-
 security/ipe/hooks.c                |  1 +
 security/selinux/hooks.c            |  5 +++--
 6 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/include/linux/kernel_read_file.h b/include/linux/kernel_read_file.h
index 90451e2e12bd..d613a7b4dd35 100644
--- a/include/linux/kernel_read_file.h
+++ b/include/linux/kernel_read_file.h
@@ -14,6 +14,7 @@
 	id(KEXEC_INITRAMFS, kexec-initramfs)	\
 	id(POLICY, security-policy)		\
 	id(X509_CERTIFICATE, x509-certificate)	\
+	id(MODULE_COMPRESSED, kernel-module-compressed) \
 	id(MAX_ID, )
 
 #define __fid_enumify(ENUM, dummy) READING_ ## ENUM,
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c66b26184936..7b3ec2fa6e7c 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3675,24 +3675,35 @@ static int idempotent_wait_for_completion(struct idempotent *u)
 
 static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
 {
+	bool compressed = !!(flags & MODULE_INIT_COMPRESSED_FILE);
 	struct load_info info = { };
 	void *buf = NULL;
 	int len;
+	int err;
 
-	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL, READING_MODULE);
+	len = kernel_read_file(f, 0, &buf, INT_MAX, NULL,
+			       compressed ? READING_MODULE_COMPRESSED :
+					    READING_MODULE);
 	if (len < 0) {
 		mod_stat_inc(&failed_kreads);
 		return len;
 	}
 
-	if (flags & MODULE_INIT_COMPRESSED_FILE) {
-		int err = module_decompress(&info, buf, len);
+	if (compressed) {
+		err = module_decompress(&info, buf, len);
 		vfree(buf); /* compressed data is no longer needed */
 		if (err) {
 			mod_stat_inc(&failed_decompress);
 			mod_stat_add_long(len, &invalid_decompress_bytes);
 			return err;
 		}
+		err = security_kernel_post_read_file(f, (char *)info.hdr, info.len,
+						     READING_MODULE);
+		if (err) {
+			mod_stat_inc(&failed_kreads);
+			free_copy(&info, flags);
+			return err;
+		}
 	} else {
 		info.hdr = buf;
 		info.len = len;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index cdd225f65a62..49f8b2b1a9af 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -235,7 +235,8 @@ static void ima_file_free(struct file *file)
 
 static int process_measurement(struct file *file, const struct cred *cred,
 			       struct lsm_prop *prop, char *buf, loff_t size,
-			       int mask, enum ima_hooks func)
+			       int mask, enum ima_hooks func,
+			       enum kernel_read_file_id read_id)
 {
 	struct inode *real_inode, *inode = file_inode(file);
 	struct ima_iint_cache *iint = NULL;
@@ -406,6 +407,12 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
 		goto out_locked;
 
+	/* Defer measuring/appraising kernel modules to READING_MODULE */
+	if (read_id == READING_MODULE_COMPRESSED) {
+		must_appraise = 0;
+		goto out_locked;
+	}
+
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
 
@@ -486,14 +493,14 @@ static int ima_file_mmap(struct file *file, unsigned long reqprot,
 
 	if (reqprot & PROT_EXEC) {
 		ret = process_measurement(file, current_cred(), &prop, NULL,
-					  0, MAY_EXEC, MMAP_CHECK_REQPROT);
+					  0, MAY_EXEC, MMAP_CHECK_REQPROT, 0);
 		if (ret)
 			return ret;
 	}
 
 	if (prot & PROT_EXEC)
 		return process_measurement(file, current_cred(), &prop, NULL,
-					   0, MAY_EXEC, MMAP_CHECK);
+					   0, MAY_EXEC, MMAP_CHECK, 0);
 
 	return 0;
 }
@@ -578,13 +585,13 @@ static int ima_bprm_check(struct linux_binprm *bprm)
 
 	security_current_getlsmprop_subj(&prop);
 	ret = process_measurement(bprm->file, current_cred(),
-				  &prop, NULL, 0, MAY_EXEC, BPRM_CHECK);
+				  &prop, NULL, 0, MAY_EXEC, BPRM_CHECK, 0);
 	if (ret)
 		return ret;
 
 	security_cred_getlsmprop(bprm->cred, &prop);
 	return process_measurement(bprm->file, bprm->cred, &prop, NULL, 0,
-				   MAY_EXEC, CREDS_CHECK);
+				   MAY_EXEC, CREDS_CHECK, 0);
 }
 
 /**
@@ -632,7 +639,7 @@ static int ima_file_check(struct file *file, int mask)
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, NULL, 0,
 				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
-					   MAY_APPEND), FILE_CHECK);
+					   MAY_APPEND), FILE_CHECK, 0);
 }
 
 static int __ima_inode_hash(struct inode *inode, struct file *file, char *buf,
@@ -851,12 +858,13 @@ static int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
 	func = read_idmap[read_id] ?: FILE_CHECK;
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, NULL, 0,
-				   MAY_READ, func);
+				   MAY_READ, func, 0);
 }
 
 const int read_idmap[READING_MAX_ID] = {
 	[READING_FIRMWARE] = FIRMWARE_CHECK,
 	[READING_MODULE] = MODULE_CHECK,
+	[READING_MODULE_COMPRESSED] = MODULE_CHECK,
 	[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
 	[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
 	[READING_POLICY] = POLICY_CHECK
@@ -894,7 +902,7 @@ static int ima_post_read_file(struct file *file, char *buf, loff_t size,
 	func = read_idmap[read_id] ?: FILE_CHECK;
 	security_current_getlsmprop_subj(&prop);
 	return process_measurement(file, current_cred(), &prop, buf, size,
-				   MAY_READ, func);
+				   MAY_READ, func, read_id);
 }
 
 /**
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 128fab897930..ae520e6bb1cf 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -241,7 +241,8 @@ static struct ima_rule_entry build_appraise_rules[] __ro_after_init = {
 
 static struct ima_rule_entry secure_boot_rules[] __ro_after_init = {
 	{.action = APPRAISE, .func = MODULE_CHECK,
-	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
+	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED |
+		  IMA_CHECK_BLACKLIST},
 	{.action = APPRAISE, .func = FIRMWARE_CHECK,
 	 .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
 	{.action = APPRAISE, .func = KEXEC_KERNEL_CHECK,
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index d0323b81cd8f..1053a4acf589 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -118,6 +118,7 @@ int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
 		op = IPE_OP_FIRMWARE;
 		break;
 	case READING_MODULE:
+	case READING_MODULE_COMPRESSED:
 		op = IPE_OP_KERNEL_MODULE;
 		break;
 	case READING_KEXEC_INITRAMFS:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index dfc22da42f30..c1ff69d5d76e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4275,7 +4275,7 @@ static int selinux_kernel_read_file(struct file *file,
 {
 	int rc = 0;
 
-	BUILD_BUG_ON_MSG(READING_MAX_ID > 7,
+	BUILD_BUG_ON_MSG(READING_MAX_ID > 8,
 			 "New kernel_read_file_id introduced; update SELinux!");
 
 	switch (id) {
@@ -4283,6 +4283,7 @@ static int selinux_kernel_read_file(struct file *file,
 		rc = selinux_kernel_load_from_file(file, SYSTEM__FIRMWARE_LOAD);
 		break;
 	case READING_MODULE:
+	case READING_MODULE_COMPRESSED:
 		rc = selinux_kernel_load_from_file(file, SYSTEM__MODULE_LOAD);
 		break;
 	case READING_KEXEC_IMAGE:
@@ -4311,7 +4312,7 @@ static int selinux_kernel_load_data(enum kernel_load_data_id id, bool contents)
 {
 	int rc = 0;
 
-	BUILD_BUG_ON_MSG(LOADING_MAX_ID > 7,
+	BUILD_BUG_ON_MSG(LOADING_MAX_ID > 8,
 			 "New kernel_load_data_id introduced; update SELinux!");
 
 	switch (id) {

base-commit: 6a23ae0a96a600d1d12557add110e0bb6e32730c
-- 
2.51.1


^ permalink raw reply related

* Re: [PATCH v2] gendwarfksyms: Skip files with no exports
From: Sami Tolvanen @ 2025-11-18 16:26 UTC (permalink / raw)
  To: Giuliano Procida
  Cc: Miguel Ojeda, Alex Gaynor, linux-modules, linux-kbuild,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	rust-for-linux, linux-kernel, patches, stable, Haiyue Wang
In-Reply-To: <20251118145741.1042013-1-gprocida@google.com>

Hi Giuliano,

On Tue, Nov 18, 2025 at 6:58 AM Giuliano Procida <gprocida@google.com> wrote:
>
> Hi.
>
> > Thus do the last one: don't attempt to process files if we have no symbol
> > versions to calculate.
>
> This results in the -T foo option being ignored in the case there were
> no symbols. I think it would be better, consistent with the
> documentation and expectations, for the file to be produced empty.

The kernel build doesn't produce empty symtypes files because symbol
versioning is skipped for (non-Rust) object files that have no
exports, and before rustc 1.91, we never ran gendwarfksyms for Rust
object files that didn't have exports.

> This means that just the for loop should be skipped, say by adding the
> condition there with &&.

No, I think the current behavior is correct, we shouldn't produce empty files.

> If you disagree, then please update the documentation to match the new
> behaviour.

I re-read the documentation and it doesn't really state how the -T
flag behaves if the tool is used to process a file with no exports.
While this doesn't impact kernel builds, a patch to clarify the
documentation is always welcome!

Sami

^ permalink raw reply

* Re: [PATCH v2] gendwarfksyms: Skip files with no exports
From: Miguel Ojeda @ 2025-11-18 16:20 UTC (permalink / raw)
  To: Giuliano Procida, Sami Tolvanen
  Cc: Miguel Ojeda, Alex Gaynor, linux-modules, linux-kbuild,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	rust-for-linux, linux-kernel, patches, stable, Haiyue Wang
In-Reply-To: <20251118145741.1042013-1-gprocida@google.com>

On Tue, Nov 18, 2025 at 3:58 PM Giuliano Procida <gprocida@google.com> wrote:
>
> This results in the -T foo option being ignored in the case there were
> no symbols. I think it would be better, consistent with the
> documentation and expectations, for the file to be produced empty.
>
> This means that just the for loop should be skipped, say by adding the
> condition there with &&.
>
> If you disagree, then please update the documentation to match the new
> behaviour.

Sounds reasonable. If there were users relying on that, then it may be
best to keep the behavior as it was. But up to the maintainers what
would make the most sense here of course (moving Sami to the To:).

Since the commit is in mainline and it seems you already thought about
the solution, I imagine a patch would be welcome.

Thanks!

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH] gendwarfksyms: Fix build on 32-bit hosts
From: Michal Suchánek @ 2025-11-18 16:18 UTC (permalink / raw)
  To: Sami Tolvanen
  Cc: linux-modules, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	linux-kbuild, linux-kernel
In-Reply-To: <20251117203806.970840-2-samitolvanen@google.com>

Hello,

On Mon, Nov 17, 2025 at 08:38:07PM +0000, Sami Tolvanen wrote:
> We have interchangeably used unsigned long for some of the types
> defined in elfutils, assuming they're always 64-bit. This obviously
> fails when building gendwarfksyms on 32-bit hosts. Fix the types.
> 
> Reported-by: Michal Suchánek <msuchanek@suse.de>
> Closes: https://lore.kernel.org/linux-modules/aRcxzPxtJblVSh1y@kitsune.suse.cz/
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
> ---
>  scripts/gendwarfksyms/dwarf.c   | 4 +++-
>  scripts/gendwarfksyms/symbols.c | 5 +++--
>  2 files changed, 6 insertions(+), 3 deletions(-)

with this patch gendwarfksyms builds on 32bit x86 and Arm.

Tested-by: Michal Suchánek <msuchanek@suse.de>

Thanks

Michal

> 
> diff --git a/scripts/gendwarfksyms/dwarf.c b/scripts/gendwarfksyms/dwarf.c
> index 3538a7d9cb07..e76d732f5f60 100644
> --- a/scripts/gendwarfksyms/dwarf.c
> +++ b/scripts/gendwarfksyms/dwarf.c
> @@ -750,6 +750,7 @@ static void process_enumerator_type(struct state *state, struct die *cache,
>  				    Dwarf_Die *die)
>  {
>  	bool overridden = false;
> +	unsigned long override;
>  	Dwarf_Word value;
>  
>  	if (stable) {
> @@ -761,7 +762,8 @@ static void process_enumerator_type(struct state *state, struct die *cache,
>  			return;
>  
>  		overridden = kabi_get_enumerator_value(
> -			state->expand.current_fqn, cache->fqn, &value);
> +			state->expand.current_fqn, cache->fqn, &override);
> +		value = override;
>  	}
>  
>  	process_list_comma(state, cache);
> diff --git a/scripts/gendwarfksyms/symbols.c b/scripts/gendwarfksyms/symbols.c
> index ecddcb5ffcdf..42cd27c9cec4 100644
> --- a/scripts/gendwarfksyms/symbols.c
> +++ b/scripts/gendwarfksyms/symbols.c
> @@ -3,6 +3,7 @@
>   * Copyright (C) 2024 Google LLC
>   */
>  
> +#include <inttypes.h>
>  #include "gendwarfksyms.h"
>  
>  #define SYMBOL_HASH_BITS 12
> @@ -242,7 +243,7 @@ static void elf_for_each_global(int fd, elf_symbol_callback_t func, void *arg)
>  				error("elf_getdata failed: %s", elf_errmsg(-1));
>  
>  			if (shdr->sh_entsize != sym_size)
> -				error("expected sh_entsize (%lu) to be %zu",
> +				error("expected sh_entsize (%" PRIu64 ") to be %zu",
>  				      shdr->sh_entsize, sym_size);
>  
>  			nsyms = shdr->sh_size / shdr->sh_entsize;
> @@ -292,7 +293,7 @@ static void set_symbol_addr(struct symbol *sym, void *arg)
>  		hash_add(symbol_addrs, &sym->addr_hash,
>  			 symbol_addr_hash(&sym->addr));
>  
> -		debug("%s -> { %u, %lx }", sym->name, sym->addr.section,
> +		debug("%s -> { %u, %" PRIx64 " }", sym->name, sym->addr.section,
>  		      sym->addr.address);
>  	} else if (sym->addr.section != addr->section ||
>  		   sym->addr.address != addr->address) {
> 
> base-commit: 6a23ae0a96a600d1d12557add110e0bb6e32730c
> -- 
> 2.52.0.rc1.455.g30608eb744-goog
> 

^ permalink raw reply

* Re: [PATCH v2] gendwarfksyms: Skip files with no exports
From: Giuliano Procida @ 2025-11-18 14:57 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Sami Tolvanen, Alex Gaynor, linux-modules, linux-kbuild,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	rust-for-linux, linux-kernel, patches, stable, Haiyue Wang
In-Reply-To: <20251110131913.1789896-1-ojeda@kernel.org>

Hi.

> Thus do the last one: don't attempt to process files if we have no symbol
> versions to calculate.

This results in the -T foo option being ignored in the case there were
no symbols. I think it would be better, consistent with the
documentation and expectations, for the file to be produced empty.

This means that just the for loop should be skipped, say by adding the
condition there with &&.

If you disagree, then please update the documentation to match the new
behaviour.

Regards,
Giuliano.

^ permalink raw reply

* Re: [PATCH v9 2/9] crypto: Add ML-DSA/Dilithium verify support
From: James Bottomley @ 2025-11-18 12:59 UTC (permalink / raw)
  To: David Howells
  Cc: Eric Biggers, Herbert Xu, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, linux-crypto,
	keyrings, linux-modules, linux-kernel
In-Reply-To: <2187237.1763455154@warthog.procyon.org.uk>

On Tue, 2025-11-18 at 08:39 +0000, David Howells wrote:
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> > But even if you don't accept that, Google keeps effective joint
> > ownership of the code through their CLA and so could grant a dual
> > licence to the kernel anyway without needing to refer to any
> > contributors.
> 
> Actually, the fact that BoringSSL's ML-DSA implementation uses C++
> with heavy use of integer-parametered templating is more
> insurmountable for borrowing their code.  Yes, it does allow them to
> reduce their LoC to ~3000 and is much more readable, but I can't do
> that in C.

I was only commenting on the legality of copying not the technical
feasibility. 

>   Now, if we want to work on persuading Linus to allow C++ into the
> kernel...

Having worked on it, C++ is a bit of a messy language in that there are
many more idioms for the same construct than in C, so I can see why C
is preferred for cleaner coding.  On the other hand if the safety
profiles work actually produces something useful (unlike safe C++ which
just flamed out) I could see that being a reason to revisit.

Regards,

James


^ permalink raw reply

* Re: [PATCH v2] lsm,ima: new LSM hook security_kernel_module_read_file to access decompressed kernel module
From: Mimi Zohar @ 2025-11-18 12:19 UTC (permalink / raw)
  To: Coiby Xu
  Cc: Paul Moore, linux-integrity, linux-security-module, Karel Srot,
	James Morris, Serge E. Hallyn, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Roberto Sassu, Dmitry Kasatkin,
	Eric Snowberg, open list, open list:MODULE SUPPORT
In-Reply-To: <42qcfcxxlmwphctzvji76hy5tycfabiiv5u6zw6lgg2p3e2jwv@fp4g2y7ecf2y>

On Thu, 2025-11-13 at 12:06 +0800, Coiby Xu wrote:
> On Fri, Nov 07, 2025 at 02:28:13PM -0500, Mimi Zohar wrote:
> > On Thu, 2025-11-06 at 17:15 -0500, Mimi Zohar wrote:
> > > On Thu, 2025-11-06 at 21:29 +0800, Coiby Xu wrote:
> > > > On Wed, Nov 05, 2025 at 03:47:25PM -0500, Mimi Zohar wrote:
> > > > > On Wed, 2025-11-05 at 08:18 +0800, Coiby Xu wrote:
> > > > [...]
> > > > > 
> > > > > Hi Coiby,
> > > > > 
> > > > > Based on the conversation with Paul, there is no reason to remove the existing
> > > > > security_kernel_post_read_file() call.
> > > > > 
> > > > > The changes are similar to the 2nd link, but a bit different.
> > > > > - Define a single enumeration named READING_MODULE_COMPRESSED.
> > > > > 
> > > > > - In module/main.c add a new security_kernel_post_read_file() call immediately
> > > > > after decompressing the kernel module.  Like a previous version of this patch,
> > > > > call kernel_read_file() with either READING_MODULE or READING_MODULE_COMPRESSED
> > > > > based on MODULE_INIT_COMPRESSED_FILE.
> > > > > 
> > > > > - In ima_post_read_file() defer verifying the signature when the enumeration is
> > > > > READING_MODULE_COMPRESSED.  (No need for a new function ima_read_kernel_module.)
> > > > 
> > > > Hi Mimi,
> > > > 
> > > > Thanks for summarizing your conversation with Paul! I can confirm Paul's
> > > > approach works
> > > > https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
> > > > 
> > > > While testing the patch today, I realized there is another
> > > > issue/challenge introduced by in-kernel module decompression. IMA
> > > > appraisal is to verify the digest of compressed kernel module but
> > > > currently the passed buffer is uncompressed module. When IMA uses
> > > > uncompressed module data to calculate the digest, xattr signature
> > > > verification will fail. If we always make IMA read the original kernel
> > > > module data again to calculate the digest, does it look like a
> > > > quick-and-dirty fix? If we can assume people won't load kernel module so
> > > > often, the performance impact is negligible. Otherwise we may have to
> > > > introduce a new LSM hook so IMA can access uncompressed and original
> > > > module data one time.
> > > 
> > > ima_collect_measurement() stores the file hash info in the iint and uses that
> > > information to verify the signature as stored in the security xattr.
> > > Decompressing the kernel module shouldn't affect the xattr signature
> > > verification.
> > 
> > In the case when the compressed kernel module hasn't previously been measured or
> > appraised before loading the kernel module, we need to "collect" the file data
> > hash on READING_MODULE_COMPRESSED, but defer appraising/measuring it.
> > 
> > An alternative to your suggestion of re-reading the original kernel module data
> > to calculate the digest or defining a new hook, would be to define "collect" as
> > a new "action" and pass the kernel_read_file_id enumeration to
> > process_measurement().  IMA_COLLECTED already exists.  Only IMA_COLLECT would
> > need to be defined.  The new collect "action" should be limited to
> > func=MODULE_CHECK.
> > 
> > The downside of this alternative is that it requires a new collect rule:
> > collect func=MODULE_CHECK mask=MAY_READ uid=0
> > appraise func=MODULE_CHECK appraise_type=imasig|modsig

As it turns out, the "collect" rule is unnecessary.  On
READING_MODULE_COMPRESSED, process_measurement() should calculate the compressed
file hash.  Extending the IMA measurement list and verifying the signature can
then be differed to READING_MODULE.

> 
> Thank for suggesting an alternative! I've implemented the idea in
> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_collect
> 
> Note besides a new collect rule, another change is needed. Currently,
> process_measurement only accepts enum ima_hooks thus it can't tell if
> it's READING_MODULE_COMPRESSED so to only do collect action. So I
> create a fake MODULE_COMPRESSED_CHECK func.

Correct, either extending process_measurement() with the read_idmap enum or
defining the fake hook would work.

> 
> And for the idea of re-reading the original kernel module data, it has
> been implemented in 
> https://github.com/coiby/linux/tree/in_kernel_decompression_ima_no_lsm_hook_paul
> 
> Both branches have applied your requested three changes including
> respecting the 80 char line limit. Additionally, I made a change to the
> IPE LSM because of the new READING_MODULE_COMPRESSED kernel_read_file_id
> enumerate.
> 
> After comparing the two implementations, personally I prefer re-reading
> the original kernel module data because the change is smaller and it's
> more user-friendly. But if there are other reasons I don't know, I'll
> post the patches of the new collect action approach to the mailing list.

The "re-reading" option fails some of the tests.  As the "collect" rule isn't
needed, let's stick with the first option.

-- 

thanks,

Mimi

^ permalink raw reply

* Re: [PATCH v9 2/9] crypto: Add ML-DSA/Dilithium verify support
From: David Howells @ 2025-11-18  8:39 UTC (permalink / raw)
  To: James Bottomley
  Cc: dhowells, Eric Biggers, Herbert Xu, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, linux-crypto,
	keyrings, linux-modules, linux-kernel
In-Reply-To: <13213304ac049113655ab8fe1bae76cc84a3330e.camel@HansenPartnership.com>

James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> But even if you don't accept that, Google keeps effective joint
> ownership of the code through their CLA and so could grant a dual
> licence to the kernel anyway without needing to refer to any
> contributors.

Actually, the fact that BoringSSL's ML-DSA implementation uses C++ with heavy
use of integer-parametered templating is more insurmountable for borrowing
their code.  Yes, it does allow them to reduce their LoC to ~3000 and is much
more readable, but I can't do that in C.  Now, if we want to work on
persuading Linus to allow C++ into the kernel...

David


^ permalink raw reply

* Re: gendwarksyms not 32bit-clean
From: Sami Tolvanen @ 2025-11-17 20:39 UTC (permalink / raw)
  To: Michal Suchánek; +Cc: linux-modules, linux-kbuild, linux-kernel
In-Reply-To: <aRcxzPxtJblVSh1y@kitsune.suse.cz>

Hi Michal,

On Fri, Nov 14, 2025 at 5:42 AM Michal Suchánek <msuchanek@suse.de> wrote:
> I tried to build kernel with gendwarfksyms and it does not work on
> 32bit:
[...]
>
> Can you fix it, or make it depend on host being 64bit?

Thanks for the report. Can you test if this fixes the issue for you?

https://lore.kernel.org/linux-modules/20251117203806.970840-2-samitolvanen@google.com/

Sami

^ permalink raw reply

* [PATCH] gendwarfksyms: Fix build on 32-bit hosts
From: Sami Tolvanen @ 2025-11-17 20:38 UTC (permalink / raw)
  To: linux-modules
  Cc: Luis Chamberlain, Petr Pavlu, Daniel Gomez, linux-kbuild,
	linux-kernel, Michal Suchánek, Sami Tolvanen

We have interchangeably used unsigned long for some of the types
defined in elfutils, assuming they're always 64-bit. This obviously
fails when building gendwarfksyms on 32-bit hosts. Fix the types.

Reported-by: Michal Suchánek <msuchanek@suse.de>
Closes: https://lore.kernel.org/linux-modules/aRcxzPxtJblVSh1y@kitsune.suse.cz/
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 scripts/gendwarfksyms/dwarf.c   | 4 +++-
 scripts/gendwarfksyms/symbols.c | 5 +++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/scripts/gendwarfksyms/dwarf.c b/scripts/gendwarfksyms/dwarf.c
index 3538a7d9cb07..e76d732f5f60 100644
--- a/scripts/gendwarfksyms/dwarf.c
+++ b/scripts/gendwarfksyms/dwarf.c
@@ -750,6 +750,7 @@ static void process_enumerator_type(struct state *state, struct die *cache,
 				    Dwarf_Die *die)
 {
 	bool overridden = false;
+	unsigned long override;
 	Dwarf_Word value;
 
 	if (stable) {
@@ -761,7 +762,8 @@ static void process_enumerator_type(struct state *state, struct die *cache,
 			return;
 
 		overridden = kabi_get_enumerator_value(
-			state->expand.current_fqn, cache->fqn, &value);
+			state->expand.current_fqn, cache->fqn, &override);
+		value = override;
 	}
 
 	process_list_comma(state, cache);
diff --git a/scripts/gendwarfksyms/symbols.c b/scripts/gendwarfksyms/symbols.c
index ecddcb5ffcdf..42cd27c9cec4 100644
--- a/scripts/gendwarfksyms/symbols.c
+++ b/scripts/gendwarfksyms/symbols.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2024 Google LLC
  */
 
+#include <inttypes.h>
 #include "gendwarfksyms.h"
 
 #define SYMBOL_HASH_BITS 12
@@ -242,7 +243,7 @@ static void elf_for_each_global(int fd, elf_symbol_callback_t func, void *arg)
 				error("elf_getdata failed: %s", elf_errmsg(-1));
 
 			if (shdr->sh_entsize != sym_size)
-				error("expected sh_entsize (%lu) to be %zu",
+				error("expected sh_entsize (%" PRIu64 ") to be %zu",
 				      shdr->sh_entsize, sym_size);
 
 			nsyms = shdr->sh_size / shdr->sh_entsize;
@@ -292,7 +293,7 @@ static void set_symbol_addr(struct symbol *sym, void *arg)
 		hash_add(symbol_addrs, &sym->addr_hash,
 			 symbol_addr_hash(&sym->addr));
 
-		debug("%s -> { %u, %lx }", sym->name, sym->addr.section,
+		debug("%s -> { %u, %" PRIx64 " }", sym->name, sym->addr.section,
 		      sym->addr.address);
 	} else if (sym->addr.section != addr->section ||
 		   sym->addr.address != addr->address) {

base-commit: 6a23ae0a96a600d1d12557add110e0bb6e32730c
-- 
2.52.0.rc1.455.g30608eb744-goog


^ permalink raw reply related

* Re: [PATCH v9 2/9] crypto: Add ML-DSA/Dilithium verify support
From: James Bottomley @ 2025-11-17 20:19 UTC (permalink / raw)
  To: David Howells, Eric Biggers
  Cc: Herbert Xu, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, linux-crypto,
	keyrings, linux-modules, linux-kernel
In-Reply-To: <2165074.1763409175@warthog.procyon.org.uk>

On Mon, 2025-11-17 at 19:52 +0000, David Howells wrote:
> Eric Biggers <ebiggers@kernel.org> wrote:
> 
> > In comparison, BoringSSL has an entire implementation of ML-DSA,
> > ...
> 
> ... which cannot be used in the kernel due to the licence.

The licence problem is not insurmountable.  There are now real lawyers
(not just me) prepared to stand up and say Apache-2 is GPL-2
compatible:

https://digitalcommons.law.scu.edu/chtlj/vol40/iss3/2/

But even if you don't accept that, Google keeps effective joint
ownership of the code through their CLA and so could grant a dual
licence to the kernel anyway without needing to refer to any
contributors.

Regards,

James


^ permalink raw reply

* Re: [PATCH v9 2/9] crypto: Add ML-DSA/Dilithium verify support
From: Eric Biggers @ 2025-11-17 20:12 UTC (permalink / raw)
  To: David Howells
  Cc: Herbert Xu, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, linux-crypto,
	keyrings, linux-modules, linux-kernel
In-Reply-To: <2165074.1763409175@warthog.procyon.org.uk>

On Mon, Nov 17, 2025 at 07:52:55PM +0000, David Howells wrote:
> Eric Biggers <ebiggers@kernel.org> wrote:
> 
> > In comparison, BoringSSL has an entire implementation of ML-DSA, ...
> 
> ... which cannot be used in the kernel due to the licence.
> 
> David

First, BoringSSL's license changed to Apache only recently.  An older
version of the code is available under a BSD style license.

Second, even with the new license I can get permission to relicense it
if needed.

Third, regardless of license BoringSSL's code can't be reused directly
in the kernel anyway, for various reasons.  My point is that a smaller,
cleaner, and more maintainable implementation of ML-DSA is possible, and
your submission misses the mark.

- Eric

^ permalink raw reply

* Re: [PATCH v9 2/9] crypto: Add ML-DSA/Dilithium verify support
From: David Howells @ 2025-11-17 20:05 UTC (permalink / raw)
  To: Eric Biggers
  Cc: dhowells, Herbert Xu, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, linux-crypto,
	keyrings, linux-modules, linux-kernel
In-Reply-To: <2165074.1763409175@warthog.procyon.org.uk>

> Eric Biggers <ebiggers@kernel.org> wrote:
> 
> > In comparison, BoringSSL has an entire implementation of ML-DSA, ...
> 
> ... which cannot be used in the kernel due to the licence.

... and it's written in C++.  Now, whilst I'm in favour generally of moving
the kernel to C++, it could be a hard sell.

David


^ permalink raw reply

* Re: [PATCH v9 2/9] crypto: Add ML-DSA/Dilithium verify support
From: David Howells @ 2025-11-17 19:52 UTC (permalink / raw)
  To: Eric Biggers
  Cc: dhowells, Herbert Xu, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, linux-crypto,
	keyrings, linux-modules, linux-kernel
In-Reply-To: <20251117171003.GC1584@sol>

Eric Biggers <ebiggers@kernel.org> wrote:

> In comparison, BoringSSL has an entire implementation of ML-DSA, ...

... which cannot be used in the kernel due to the licence.

David


^ permalink raw reply

* Re: Pick up keys-pqc branch for linux-next?
From: Eric Biggers @ 2025-11-17 17:11 UTC (permalink / raw)
  To: David Howells
  Cc: Stephen Rothwell, Mark Brown, Herbert Xu, linux-crypto, keyrings,
	linux-modules, linux-next, linux-kernel
In-Reply-To: <2157243.1763392923@warthog.procyon.org.uk>

On Mon, Nov 17, 2025 at 03:22:03PM +0000, David Howells wrote:
> Hi Stephen,
> 
> Can you pick up my keys-pqc branch for linux-next please?  It can be found at:
> 
> 	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/
> 	keys-pqc
> 
> Note that it's based on Eric Bigger's libcrypto/lbcrypto-next branch which I
> believe you already have in order to get SHA-3/SHAKE support.
> 

I don't really see the point yet, since this isn't going to be ready for
the next merge window anyway.

- Eric

^ permalink raw reply

* Re: [PATCH v9 2/9] crypto: Add ML-DSA/Dilithium verify support
From: Eric Biggers @ 2025-11-17 17:10 UTC (permalink / raw)
  To: David Howells
  Cc: Herbert Xu, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, linux-crypto,
	keyrings, linux-modules, linux-kernel
In-Reply-To: <20251117145606.2155773-3-dhowells@redhat.com>

On Mon, Nov 17, 2025 at 02:55:51PM +0000, David Howells wrote:
>  lib/crypto/Kconfig                            |   1 +
>  lib/crypto/Makefile                           |   2 +
>  lib/crypto/mldsa/Kconfig                      |  29 ++
>  lib/crypto/mldsa/Makefile                     |  20 +
>  lib/crypto/mldsa/crypto_mldsa_44.c            | 166 ++++++++
>  lib/crypto/mldsa/crypto_mldsa_65.c            | 166 ++++++++
>  lib/crypto/mldsa/crypto_mldsa_87.c            | 166 ++++++++
>  lib/crypto/mldsa/dilithium.h                  | 304 ++++++++++++++
>  lib/crypto/mldsa/dilithium_44.c               |  33 ++
>  lib/crypto/mldsa/dilithium_44.h               | 291 ++++++++++++++
>  lib/crypto/mldsa/dilithium_65.c               |  33 ++
>  lib/crypto/mldsa/dilithium_65.h               | 291 ++++++++++++++
>  lib/crypto/mldsa/dilithium_87.c               |  33 ++
>  lib/crypto/mldsa/dilithium_87.h               | 291 ++++++++++++++
>  lib/crypto/mldsa/dilithium_common.c           | 117 ++++++
>  lib/crypto/mldsa/dilithium_debug.h            |  49 +++
>  lib/crypto/mldsa/dilithium_ntt.c              |  89 +++++
>  lib/crypto/mldsa/dilithium_ntt.h              |  35 ++
>  lib/crypto/mldsa/dilithium_pack.h             | 119 ++++++
>  lib/crypto/mldsa/dilithium_poly.c             | 377 ++++++++++++++++++
>  lib/crypto/mldsa/dilithium_poly.h             | 181 +++++++++
>  lib/crypto/mldsa/dilithium_poly_c.h           | 141 +++++++
>  lib/crypto/mldsa/dilithium_poly_common.h      |  35 ++
>  lib/crypto/mldsa/dilithium_polyvec.h          | 343 ++++++++++++++++
>  lib/crypto/mldsa/dilithium_polyvec_c.h        |  81 ++++
>  lib/crypto/mldsa/dilithium_reduce.h           |  85 ++++
>  lib/crypto/mldsa/dilithium_rounding.c         | 128 ++++++
>  lib/crypto/mldsa/dilithium_service_helpers.h  |  99 +++++
>  lib/crypto/mldsa/dilithium_signature_c.c      | 279 +++++++++++++
>  lib/crypto/mldsa/dilithium_signature_c.h      |  37 ++
>  lib/crypto/mldsa/dilithium_signature_impl.h   | 370 +++++++++++++++++
>  lib/crypto/mldsa/dilithium_type.h             | 108 +++++
>  lib/crypto/mldsa/dilithium_zetas.c            |  68 ++++
>  .../mldsa/signature_domain_separation.c       | 204 ++++++++++
>  .../mldsa/signature_domain_separation.h       |  30 ++
>  35 files changed, 4801 insertions(+)

Over the past week I've been starting to review this massive addition.

I don't think this is on the right track.  This implementation is really
messy, with lots of unused functionality and unnecessary abstractions,
and code that doesn't follow kernel conventions.

In comparison, BoringSSL has an entire implementation of ML-DSA,
*including key generation and signing*, in a bit over 3000 lines in one
file.  But about half of that code is specific to key generation or
signing, which the kernel doesn't need, so in principle
verification-only shouldn't be much more than a thousand.  I find it to
be much easier to understand than leancrypto as well.

Historically we've had a lot of problems with people integrating code
from external sources into the kernel, like mpi, with properly "owning"
it because they feel like it's not their code and someone else is
responsible.  I feel like that's going to be a big problem here.

I think we can do better here and put together a smaller implementation
for the kernel that we'll actually be able to maintain.

- Eric

^ permalink raw reply

* Re: Where to add FIPS tests
From: Eric Biggers @ 2025-11-17 16:54 UTC (permalink / raw)
  To: David Howells
  Cc: Herbert Xu, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Jason A . Donenfeld, Ard Biesheuvel,
	Stephan Mueller, Lukas Wunner, Ignat Korchagin, linux-crypto,
	keyrings, linux-modules, linux-kernel
In-Reply-To: <2158596.1763395299@warthog.procyon.org.uk>

On Mon, Nov 17, 2025 at 04:01:39PM +0000, David Howells wrote:
> Hi Herbert,
> 
> I'm wondering from where I should invoke the FIPS tests for ML-DSA.
> 
> Currently, the asymmetric key type has some FIPS selftests for RSA and ECDSA
> built into it, but I wonder if that's the best way.  The problem is that it
> does the selftest during module init - but that can only test whatever
> algorithms are built into the base kernel image and initialised at the time
> late_initcall() happens.
> 
> It might be better to put the tests into the algorithm modules themselves -
> but that then has a potential circular dependency issue.  However, that might
> not matter as the asymmetric key type won't be built as a module and will be
> built into the kernel (though some of the components such as X.509 and PKCS#7
> can be built as modules).
> 
> If I don't involve X.509/PKCS#7 in the selftest, then doing it from the ML-DSA
> modules during module init would be fine.
> 
> Do you (or anyone else) have any thoughts?

The FIPS self-test should just go in the algorithm module itself and
test ML-DSA directly.  See the other lib/crypto/ FIPS self-tests.
Please check the FIPS Implementation Guidance documentation to see what
is needed; it isn't actually very much.

- Eric

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox