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]
Date:	Fri, 19 Jul 2013 11:08:49 -0700
From:	Tim Chen <tim.c.chen@...ux.intel.com>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Herbert Xu <herbert@...dor.apana.org.au>
Cc:	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
	linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
	ak <ak@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module
 dependency.

On Fri, 2013-07-19 at 16:49 +0200, Rafael J. Wysocki wrote:
> > > > > This should cause udev to load the crct10dif_pclml module when cpu
> > > > > support the PCLMULQDQ (feature code 0081).  I did my testing during
> > > > > development on 3.10 and the module was indeed loaded.
> > > > >
> > > > > However, I found that the following commit under 3.11-rc1 broke
> > > > > the mechanism after some bisection.
> > > > >
> > > > > commit ac212b6980d8d5eda705864fc5a8ecddc6d6eacc
> > > > > Author: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > > > > Date:   Fri May 3 00:26:22 2013 +0200
> > > > >
> > > > >      ACPI / processor: Use common hotplug infrastructure
> > > > >      
> > > > >      Split the ACPI processor driver into two parts, one that is
> > > > >      non-modular, resides in the ACPI core and handles the enumeration
> > > > >      and hotplug of processors and one that implements the rest of the
> > > > >      existing processor driver functionality.
> > > > >      
> > > > > Rafael, can you check and see if this can be fixed so those optimized
> > > > > crypto modules for Intel cpu that support them can be loaded?
> > > > 
> > > > I think this is an ordering issue between udev startup and the time when 
> > > > devices are registered.
> > > 
> > > Something that can be fixed?
> > 
> > I'm sure it can be fixes, but I'm not sure whether it's a udev bug or real
> > breakage in the kernel yet.  I need to figure out that part.
> 
> OK
> 
> I wonder if the appended patch makes the issue go away for you?
> 

Rafael, the patch did fix the cpu feature based module loading issue.
Thanks for your quick response.

Herbert, the patch below should fix the boot failure issue.  

I also added the change to rename crct10dif.c to crct10dif_generic.c 
and added MODULE_ALIAS(crct10dif) to  crct10dif_generic.c.  
This was according to our discussion:
https://lkml.org/lkml/2013/5/21/216  

However, when I have the

CONFIG_CRYPTO_CRCT10DIF=y
CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
CONFIG_CRC_T10DIF=y

combination, the crc-t10dif library function was still 
initialized before the crct10dif-pclmul.ko was loaded, 
leading to the generic version being used.  This
is also one of Tetsuo's concerns.  In theory, loading
the generic version should also load the pclmul version as
they have the same MODULE_ALIAS, before crc-t10dif library
try to allocate the crypto transform. 

Something else that I'm missing and need to be changed?

I will be away for a week without internet access so my
response will be slow.

Thanks.

Tim

Signed-off-by: Tim Chen <tim.c.chen@...ux.intel.com>
---
Fix boot failure as crc-t10dif library may not cause crct10dif.ko
containing the generic algorithm to be loaded.

Rename crct10dif.c to crct10dif_generic.c and add module alias.
This should load all the modules supporting crct10dif algorithm
at the same time.
---
 crypto/Makefile                             | 2 +-
 crypto/{crct10dif.c => crct10dif_generic.c} | 1 +
 lib/crc-t10dif.c                            | 3 ++-
 3 files changed, 4 insertions(+), 2 deletions(-)
 rename crypto/{crct10dif.c => crct10dif_generic.c} (99%)

diff --git a/crypto/Makefile b/crypto/Makefile
index 2d5ed08..3fd76fa 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -83,7 +83,7 @@ obj-$(CONFIG_CRYPTO_ZLIB) += zlib.o
 obj-$(CONFIG_CRYPTO_MICHAEL_MIC) += michael_mic.o
 obj-$(CONFIG_CRYPTO_CRC32C) += crc32c.o
 obj-$(CONFIG_CRYPTO_CRC32) += crc32.o
-obj-$(CONFIG_CRYPTO_CRCT10DIF) += crct10dif.o
+obj-$(CONFIG_CRYPTO_CRCT10DIF) += crct10dif_generic.o
 obj-$(CONFIG_CRYPTO_AUTHENC) += authenc.o authencesn.o
 obj-$(CONFIG_CRYPTO_LZO) += lzo.o
 obj-$(CONFIG_CRYPTO_LZ4) += lz4.o
diff --git a/crypto/crct10dif.c b/crypto/crct10dif_generic.c
similarity index 99%
rename from crypto/crct10dif.c
rename to crypto/crct10dif_generic.c
index 92aca96..45a9acd 100644
--- a/crypto/crct10dif.c
+++ b/crypto/crct10dif_generic.c
@@ -176,3 +176,4 @@ module_exit(crct10dif_mod_fini);
 MODULE_AUTHOR("Tim Chen <tim.c.chen@...ux.intel.com>");
 MODULE_DESCRIPTION("T10 DIF CRC calculation.");
 MODULE_LICENSE("GPL");
+MODULE_ALIAS(crct10dif);
diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c
index d102d83..37b3eb4 100644
--- a/lib/crc-t10dif.c
+++ b/lib/crc-t10dif.c
@@ -15,7 +15,8 @@
 #include <linux/init.h>
 #include <crypto/hash.h>
 
-static struct crypto_shash *crct10dif_tfm;
+__u16 crc_t10dif_generic(__u16 crc, const unsigned char *buffer, size_t len);
+static struct crypto_shash *crct10dif_tfm = crc_t10dif_generic;
 
 __u16 crc_t10dif(const unsigned char *buffer, size_t len)
 {
-- 
1.7.11.7





--
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/