linux-kernel - Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201307180347.r6I3l5e9077577@www262.sakura.ne.jp>
Date:	Thu, 18 Jul 2013 12:47:05 +0900
From:	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>
To:	Tim Chen <tim.c.chen@...ux.intel.com>
Cc:	herbert@...dor.apana.org.au, linux-kernel@...r.kernel.org,
	linux-crypto@...r.kernel.org
Subject: Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

Tim Chen wrote:
> > > Your approach is quite complicated.  I think something simpler like the
> > > following will work:
> >
> > We cannot benefit from PCLMULQDQ. Is it acceptable for you?
> 
> 
> The following code in crct10dif-pclmul_glue.c
> 
> static const struct x86_cpu_id crct10dif_cpu_id[] = {
>         X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
>         {}
> };
> MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
> 
> will put the module in the device table and get the module
> loaded, as long as the cpu support PCLMULQDQ. So we should be able
> to benefit.

Excuse me, how can crct10dif-pclmul.ko get loaded automatically?
Did you test CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m with below debug message?

diff --git a/arch/x86/crypto/crct10dif-pclmul_glue.c b/arch/x86/crypto/crct10dif-pclmul_glue.c
index 7845d7f..a8a95aa 100644
--- a/arch/x86/crypto/crct10dif-pclmul_glue.c
+++ b/arch/x86/crypto/crct10dif-pclmul_glue.c
@@ -129,9 +129,10 @@ MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
 
 static int __init crct10dif_intel_mod_init(void)
 {
+	printk(KERN_WARNING "********** Checking for X86_FEATURE_PCLMULQDQ\n");
 	if (!x86_match_cpu(crct10dif_cpu_id))
 		return -ENODEV;
-
+	printk(KERN_WARNING "********** Registering crct10dif-pclmul\n");
 	return crypto_register_shash(&alg);
 }

As far as I tested, crct10dif-pclmul.ko will not be loaded unless manually
adding "modprobe crct10dif-pclmul" to initramfs's /init or choosing
CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y.

> So as long as the crct10dif.ko and crct10dif-pclmul.ko are loaded,
> the pclmulqdq t10dif will have a higher priority and get allocated
> and used.

What I'm talking are

  (1) Since mkinitramfs is unable to know that crct10dif-pclmul.ko has higher
      priority than crct10dif.ko , mkinitramfs will not include
      "modprobe crct10dif-pclmul" line in the generated initramfs.

  (2) In order to get benefit from PCLMULQDQ, users have to manually make sure
      that "modprobe crct10dif-pclmul" is called before crc-t10dif.ko (which is
      loaded before sd_mod.ko is loaded) is loaded by their initramfs's /init
      script.

  (3) The cause of (1) is that crct10dif-pclmul.ko will not be loaded
      automatically unless choosing CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y.

  (4) The cause of (3) is that modules.dep does not describe that users will
      benefit by loading crct10dif-pclmul.ko before loading crc-t10dif.ko .

  (5) Currently crct10dif-pclmul.ko cannot be loaded if PCLMULQDQ is not
      supported. This leads to boot failure (since sd_mod.ko cannot be loaded)
      if modules.dep says that "crct10dif-pclmul.ko is required by
      crc-t10dif.ko".

  (6) To solve (4) and (5), modules.dep should say "crct10dif-pclmul.ko is
      preferred for crc-t10dif.ko but is not required by crc-t10dif.ko".
      But there is no such mechanism. Thus, currently available choice is
      "allow loading crct10dif-pclmul.ko even if PCLMULQDQ is not supported"
      or "ignore errors by built-in the crct10dif-pclmul.ko module".

My patch (b) seems to be complicated but is required in order to solve (4)
without asking users to manually add "modprobe crct10dif-pclmul" into their
initramfs. If we choose patch (b) rather than patch (a), we need to solve (5).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/