| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <df277beb-3bc2-4941-941e-adb294d34394@t-8ch.de>
Date: Tue, 3 Feb 2026 13:41:14 +0100
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Petr Pavlu <petr.pavlu@...e.com>
Cc: Nathan Chancellor <nathan@...nel.org>, Arnd Bergmann <arnd@...db.de>,
Luis Chamberlain <mcgrof@...nel.org>, Sami Tolvanen <samitolvanen@...gle.com>,
Daniel Gomez <da.gomez@...sung.com>, Paul Moore <paul@...l-moore.com>,
James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>,
Jonathan Corbet <corbet@....net>, Madhavan Srinivasan <maddy@...ux.ibm.com>,
Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>,
Naveen N Rao <naveen@...nel.org>, Mimi Zohar <zohar@...ux.ibm.com>,
Roberto Sassu <roberto.sassu@...wei.com>, Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
Eric Snowberg <eric.snowberg@...cle.com>, Nicolas Schier <nicolas.schier@...ux.dev>,
Daniel Gomez <da.gomez@...nel.org>, Aaron Tomlin <atomlin@...mlin.com>,
"Christophe Leroy (CS GROUP)" <chleroy@...nel.org>, Nicolas Schier <nsc@...nel.org>,
Nicolas Bouchinet <nicolas.bouchinet@....cyber.gouv.fr>, Xiu Jianfeng <xiujianfeng@...wei.com>,
Fabian Grünbichler <f.gruenbichler@...xmox.com>, Arnout Engelen <arnout@...t.net>,
Mattia Rizzolo <mattia@...reri.org>, kpcyrd <kpcyrd@...hlinux.org>,
Christian Heusel <christian@...sel.eu>, Câju Mihai-Drosi <mcaju95@...il.com>,
Sebastian Andrzej Siewior <bigeasy@...utronix.de>, linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arch@...r.kernel.org, linux-modules@...r.kernel.org,
linux-security-module@...r.kernel.org, linux-doc@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
linux-integrity@...r.kernel.org
Subject: Re: [PATCH v4 08/17] module: Deduplicate signature extraction
On 2026-01-27 16:20:15+0100, Petr Pavlu wrote:
> On 1/13/26 1:28 PM, Thomas Weißschuh wrote:
(...)
> > int module_sig_check(struct load_info *info, int flags)
> > {
> > - int err = -ENODATA;
> > - const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> > + int err;
> > const char *reason;
> > const void *mod = info->hdr;
> > + size_t sig_len;
> > + const u8 *sig;
> > bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
> > MODULE_INIT_IGNORE_VERMAGIC);
> > - /*
> > - * Do not allow mangled modules as a module with version information
> > - * removed is no longer the module that was signed.
> > - */
> > - if (!mangled_module &&
> > - info->len > markerlen &&
> > - memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
> > - /* We truncate the module to discard the signature */
> > - info->len -= markerlen;
> > - err = mod_verify_sig(mod, info);
> > +
> > + err = mod_split_sig(info->hdr, &info->len, mangled_module, &sig_len, &sig, "module");
> > + if (!err) {
> > + err = verify_pkcs7_signature(mod, info->len, sig, sig_len,
> > + VERIFY_USE_SECONDARY_KEYRING,
> > + VERIFYING_MODULE_SIGNATURE,
> > + NULL, NULL);
> > if (!err) {
> > info->sig_ok = true;
> > return 0;
>
> The patch looks to modify the behavior when mangled_module is true.
>
> Previously, module_sig_check() didn't attempt to extract the signature
> in such a case and treated the module as unsigned. The err remained set
> to -ENODATA and the function subsequently consulted module_sig_check()
> and security_locked_down() to determine an appropriate result.
>
> Newly, module_sig_check() calls mod_split_sig(), which skips the
> extraction of the marker ("~Module signature appended~\n") from the end
> of the module and instead attempts to read it as an actual
> module_signature. The value is then passed to mod_check_sig() which
> should return -EBADMSG. The error is propagated to module_sig_check()
> and treated as fatal, without consulting module_sig_check() and
> security_locked_down().
>
> I think the mangled_module flag should not be passed to mod_split_sig()
> and it should be handled solely by module_sig_check().
Ack.
(...)
> > diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
> > index 3265d744d5ce..a57342d39b07 100644
> > --- a/security/integrity/ima/ima_modsig.c
> > +++ b/security/integrity/ima/ima_modsig.c
> > @@ -40,44 +40,30 @@ struct modsig {
> > int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
> > struct modsig **modsig)
> > {
> > - const size_t marker_len = strlen(MODULE_SIG_STRING);
> > - const struct module_signature *sig;
> > + size_t buf_len_sz = buf_len;
> > struct modsig *hdr;
> > size_t sig_len;
> > - const void *p;
> > + const u8 *sig;
> > int rc;
> >
> > - if (buf_len <= marker_len + sizeof(*sig))
> > - return -ENOENT;
> > -
> > - p = buf + buf_len - marker_len;
> > - if (memcmp(p, MODULE_SIG_STRING, marker_len))
> > - return -ENOENT;
> > -
> > - buf_len -= marker_len;
> > - sig = (const struct module_signature *)(p - sizeof(*sig));
> > -
> > - rc = mod_check_sig(sig, buf_len, func_tokens[func]);
> > + rc = mod_split_sig(buf, &buf_len_sz, true, &sig_len, &sig, func_tokens[func]);
>
> Passing mangled=true to mod_split_sig() seems incorrect here. It causes
> that the function doesn't properly extract the signature marker at the
> end of the module, no?
Indeed, thanks.
I am thinking about dropping this patch from the series for now.
It was meant for IMA modsig compatibility, which is not part of the
series anymore.
> > if (rc)
> > return rc;
> >
> > - sig_len = be32_to_cpu(sig->sig_len);
> > - buf_len -= sig_len + sizeof(*sig);
> > -
> > /* Allocate sig_len additional bytes to hold the raw PKCS#7 data. */
> > hdr = kzalloc(struct_size(hdr, raw_pkcs7, sig_len), GFP_KERNEL);
> > if (!hdr)
> > return -ENOMEM;
> >
> > hdr->raw_pkcs7_len = sig_len;
> > - hdr->pkcs7_msg = pkcs7_parse_message(buf + buf_len, sig_len);
> > + hdr->pkcs7_msg = pkcs7_parse_message(sig, sig_len);
> > if (IS_ERR(hdr->pkcs7_msg)) {
> > rc = PTR_ERR(hdr->pkcs7_msg);
> > kfree(hdr);
> > return rc;
> > }
> >
> > - memcpy(hdr->raw_pkcs7, buf + buf_len, sig_len);
> > + memcpy(hdr->raw_pkcs7, sig, sig_len);
> >
> > /* We don't know the hash algorithm yet. */
> > hdr->hash_algo = HASH_ALGO__LAST;
> >
Powered by blists - more mailing lists