* Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot
From: joeyli @ 2013-09-05 10:13 UTC (permalink / raw)
To: Matt Fleming
Cc: linux-kernel, linux-security-module, linux-efi, linux-pm,
linux-crypto, opensuse-kernel, David Howells, Rafael J. Wysocki,
Matthew Garrett, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH, JKosina,
Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
Michal Marek, Gary Lin, Vivek Goyal, Takashi Iwai
In-Reply-To: <20130905085348.GJ28598@console-pimps.org>
Hi Matt,
First, thanks for your review!
於 四，2013-09-05 於 09:53 +0100，Matt Fleming 提到：
> On Thu, 22 Aug, at 07:01:50PM, Lee, Chun-Yi wrote:
> > +static int efi_status_to_err(efi_status_t status)
> > +{
> > + int err;
> > +
> > + switch (status) {
> > + case EFI_INVALID_PARAMETER:
> > + err = -EINVAL;
> > + break;
> > + case EFI_OUT_OF_RESOURCES:
> > + err = -ENOSPC;
> > + break;
> > + case EFI_DEVICE_ERROR:
> > + err = -EIO;
> > + break;
> > + case EFI_WRITE_PROTECTED:
> > + err = -EROFS;
> > + break;
> > + case EFI_SECURITY_VIOLATION:
> > + err = -EACCES;
> > + break;
> > + case EFI_NOT_FOUND:
> > + err = -ENODATA;
> > + break;
> > + default:
> > + err = -EINVAL;
> > + }
> > +
> > + return err;
> > +}
>
> Please don't reimplement this. Instead make the existing function
> global.
>
OK, I will make the function to global.
> [...]
>
> > +static void *load_wake_key_data(unsigned long *datasize)
> > +{
> > + u32 attr;
> > + void *wkey_data;
> > + efi_status_t status;
> > +
> > + if (!efi_enabled(EFI_RUNTIME_SERVICES))
> > + return ERR_PTR(-EPERM);
> > +
> > + /* obtain the size */
> > + *datasize = 0;
> > + status = efi.get_variable(EFI_S4_WAKE_KEY_NAME, &EFI_HIBERNATE_GUID,
> > + NULL, datasize, NULL);
> > + if (status != EFI_BUFFER_TOO_SMALL) {
> > + wkey_data = ERR_PTR(efi_status_to_err(status));
> > + pr_err("PM: Couldn't get wake key data size: 0x%lx\n", status);
> > + goto error;
> > + }
>
> Is it safe to completely bypass the efivars interface and access
> efi.get_variable() directly? I wouldn't have thought so, unless you can
> guarantee that the kernel isn't going to access any of the EFI runtime
> services while you execute this function.
>
This S4WakeKey is a VOLATILE variable that could not modify by
SetVariable() at runtime. So, it's read only even through efivars.
Does it what your concern?
Thanks a lot!
Joey Lee
^ permalink raw reply
* Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot
From: joeyli @ 2013-09-05 10:13 UTC (permalink / raw)
To: Matt Fleming
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
opensuse-kernel-stAJ6ESoqRxg9hUCZPvPmw, David Howells,
Rafael J. Wysocki, Matthew Garrett, Len Brown, Pavel Machek,
Josh Boyer, Vojtech Pavlik, Matt Fleming, James Bottomley,
Greg KH, JKosina-IBi9RG/b67k, Rusty Russell, Herbert Xu,
David S. Miller, H. Peter Anvin, Michal Marek, Gary Lin,
Vivek Goyal, Takashi Iwai
In-Reply-To: <20130905085348.GJ28598-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org>
Hi Matt,
First, thanks for your review!
於 四，2013-09-05 於 09:53 +0100，Matt Fleming 提到：
> On Thu, 22 Aug, at 07:01:50PM, Lee, Chun-Yi wrote:
> > +static int efi_status_to_err(efi_status_t status)
> > +{
> > + int err;
> > +
> > + switch (status) {
> > + case EFI_INVALID_PARAMETER:
> > + err = -EINVAL;
> > + break;
> > + case EFI_OUT_OF_RESOURCES:
> > + err = -ENOSPC;
> > + break;
> > + case EFI_DEVICE_ERROR:
> > + err = -EIO;
> > + break;
> > + case EFI_WRITE_PROTECTED:
> > + err = -EROFS;
> > + break;
> > + case EFI_SECURITY_VIOLATION:
> > + err = -EACCES;
> > + break;
> > + case EFI_NOT_FOUND:
> > + err = -ENODATA;
> > + break;
> > + default:
> > + err = -EINVAL;
> > + }
> > +
> > + return err;
> > +}
>
> Please don't reimplement this. Instead make the existing function
> global.
>
OK, I will make the function to global.
> [...]
>
> > +static void *load_wake_key_data(unsigned long *datasize)
> > +{
> > + u32 attr;
> > + void *wkey_data;
> > + efi_status_t status;
> > +
> > + if (!efi_enabled(EFI_RUNTIME_SERVICES))
> > + return ERR_PTR(-EPERM);
> > +
> > + /* obtain the size */
> > + *datasize = 0;
> > + status = efi.get_variable(EFI_S4_WAKE_KEY_NAME, &EFI_HIBERNATE_GUID,
> > + NULL, datasize, NULL);
> > + if (status != EFI_BUFFER_TOO_SMALL) {
> > + wkey_data = ERR_PTR(efi_status_to_err(status));
> > + pr_err("PM: Couldn't get wake key data size: 0x%lx\n", status);
> > + goto error;
> > + }
>
> Is it safe to completely bypass the efivars interface and access
> efi.get_variable() directly? I wouldn't have thought so, unless you can
> guarantee that the kernel isn't going to access any of the EFI runtime
> services while you execute this function.
>
This S4WakeKey is a VOLATILE variable that could not modify by
SetVariable() at runtime. So, it's read only even through efivars.
Does it what your concern?
Thanks a lot!
Joey Lee
^ permalink raw reply
* Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot
From: joeyli @ 2013-09-05 10:13 UTC (permalink / raw)
To: Matt Fleming
Cc: linux-kernel, linux-security-module, linux-efi, linux-pm,
linux-crypto, opensuse-kernel, David Howells, Rafael J. Wysocki,
Matthew Garrett, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH, JKosina,
Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
Michal Marek, Gary Lin, Vivek Goyal, Takashi Iwai
In-Reply-To: <20130905085348.GJ28598@console-pimps.org>
Hi Matt,
First, thanks for your review!
於 四，2013-09-05 於 09:53 +0100，Matt Fleming 提到：
> On Thu, 22 Aug, at 07:01:50PM, Lee, Chun-Yi wrote:
> > +static int efi_status_to_err(efi_status_t status)
> > +{
> > + int err;
> > +
> > + switch (status) {
> > + case EFI_INVALID_PARAMETER:
> > + err = -EINVAL;
> > + break;
> > + case EFI_OUT_OF_RESOURCES:
> > + err = -ENOSPC;
> > + break;
> > + case EFI_DEVICE_ERROR:
> > + err = -EIO;
> > + break;
> > + case EFI_WRITE_PROTECTED:
> > + err = -EROFS;
> > + break;
> > + case EFI_SECURITY_VIOLATION:
> > + err = -EACCES;
> > + break;
> > + case EFI_NOT_FOUND:
> > + err = -ENODATA;
> > + break;
> > + default:
> > + err = -EINVAL;
> > + }
> > +
> > + return err;
> > +}
>
> Please don't reimplement this. Instead make the existing function
> global.
>
OK, I will make the function to global.
> [...]
>
> > +static void *load_wake_key_data(unsigned long *datasize)
> > +{
> > + u32 attr;
> > + void *wkey_data;
> > + efi_status_t status;
> > +
> > + if (!efi_enabled(EFI_RUNTIME_SERVICES))
> > + return ERR_PTR(-EPERM);
> > +
> > + /* obtain the size */
> > + *datasize = 0;
> > + status = efi.get_variable(EFI_S4_WAKE_KEY_NAME, &EFI_HIBERNATE_GUID,
> > + NULL, datasize, NULL);
> > + if (status != EFI_BUFFER_TOO_SMALL) {
> > + wkey_data = ERR_PTR(efi_status_to_err(status));
> > + pr_err("PM: Couldn't get wake key data size: 0x%lx\n", status);
> > + goto error;
> > + }
>
> Is it safe to completely bypass the efivars interface and access
> efi.get_variable() directly? I wouldn't have thought so, unless you can
> guarantee that the kernel isn't going to access any of the EFI runtime
> services while you execute this function.
>
This S4WakeKey is a VOLATILE variable that could not modify by
SetVariable() at runtime. So, it's read only even through efivars.
Does it what your concern?
Thanks a lot!
Joey Lee
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot
From: joeyli @ 2013-09-05 10:13 UTC (permalink / raw)
To: Matt Fleming
Cc: linux-kernel, linux-security-module, linux-efi, linux-pm,
linux-crypto, opensuse-kernel, David Howells, Rafael J. Wysocki,
Matthew Garrett, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH, JKosina,
Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
Michal Marek, Gary Lin, Vivek Goyal, Takashi Iwai
In-Reply-To: <20130905085348.GJ28598@console-pimps.org>
Hi Matt,
First, thanks for your review!
於 四，2013-09-05 於 09:53 +0100，Matt Fleming 提到：
> On Thu, 22 Aug, at 07:01:50PM, Lee, Chun-Yi wrote:
> > +static int efi_status_to_err(efi_status_t status)
> > +{
> > + int err;
> > +
> > + switch (status) {
> > + case EFI_INVALID_PARAMETER:
> > + err = -EINVAL;
> > + break;
> > + case EFI_OUT_OF_RESOURCES:
> > + err = -ENOSPC;
> > + break;
> > + case EFI_DEVICE_ERROR:
> > + err = -EIO;
> > + break;
> > + case EFI_WRITE_PROTECTED:
> > + err = -EROFS;
> > + break;
> > + case EFI_SECURITY_VIOLATION:
> > + err = -EACCES;
> > + break;
> > + case EFI_NOT_FOUND:
> > + err = -ENODATA;
> > + break;
> > + default:
> > + err = -EINVAL;
> > + }
> > +
> > + return err;
> > +}
>
> Please don't reimplement this. Instead make the existing function
> global.
>
OK, I will make the function to global.
> [...]
>
> > +static void *load_wake_key_data(unsigned long *datasize)
> > +{
> > + u32 attr;
> > + void *wkey_data;
> > + efi_status_t status;
> > +
> > + if (!efi_enabled(EFI_RUNTIME_SERVICES))
> > + return ERR_PTR(-EPERM);
> > +
> > + /* obtain the size */
> > + *datasize = 0;
> > + status = efi.get_variable(EFI_S4_WAKE_KEY_NAME, &EFI_HIBERNATE_GUID,
> > + NULL, datasize, NULL);
> > + if (status != EFI_BUFFER_TOO_SMALL) {
> > + wkey_data = ERR_PTR(efi_status_to_err(status));
> > + pr_err("PM: Couldn't get wake key data size: 0x%lx\n", status);
> > + goto error;
> > + }
>
> Is it safe to completely bypass the efivars interface and access
> efi.get_variable() directly? I wouldn't have thought so, unless you can
> guarantee that the kernel isn't going to access any of the EFI runtime
> services while you execute this function.
>
This S4WakeKey is a VOLATILE variable that could not modify by
SetVariable() at runtime. So, it's read only even through efivars.
Does it what your concern?
Thanks a lot!
Joey Lee
^ permalink raw reply
* Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot
From: joeyli @ 2013-09-05 10:13 UTC (permalink / raw)
To: Matt Fleming
Cc: linux-kernel, linux-security-module, linux-efi, linux-pm,
linux-crypto, opensuse-kernel, David Howells, Rafael J. Wysocki,
Matthew Garrett, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH, JKosina,
Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
Michal Marek, Gary Lin, Vivek Goyal, Takashi Iwai
In-Reply-To: <20130905085348.GJ28598@console-pimps.org>
Hi Matt,
First, thanks for your review!
於 四，2013-09-05 於 09:53 +0100，Matt Fleming 提到：
> On Thu, 22 Aug, at 07:01:50PM, Lee, Chun-Yi wrote:
> > +static int efi_status_to_err(efi_status_t status)
> > +{
> > + int err;
> > +
> > + switch (status) {
> > + case EFI_INVALID_PARAMETER:
> > + err = -EINVAL;
> > + break;
> > + case EFI_OUT_OF_RESOURCES:
> > + err = -ENOSPC;
> > + break;
> > + case EFI_DEVICE_ERROR:
> > + err = -EIO;
> > + break;
> > + case EFI_WRITE_PROTECTED:
> > + err = -EROFS;
> > + break;
> > + case EFI_SECURITY_VIOLATION:
> > + err = -EACCES;
> > + break;
> > + case EFI_NOT_FOUND:
> > + err = -ENODATA;
> > + break;
> > + default:
> > + err = -EINVAL;
> > + }
> > +
> > + return err;
> > +}
>
> Please don't reimplement this. Instead make the existing function
> global.
>
OK, I will make the function to global.
> [...]
>
> > +static void *load_wake_key_data(unsigned long *datasize)
> > +{
> > + u32 attr;
> > + void *wkey_data;
> > + efi_status_t status;
> > +
> > + if (!efi_enabled(EFI_RUNTIME_SERVICES))
> > + return ERR_PTR(-EPERM);
> > +
> > + /* obtain the size */
> > + *datasize = 0;
> > + status = efi.get_variable(EFI_S4_WAKE_KEY_NAME, &EFI_HIBERNATE_GUID,
> > + NULL, datasize, NULL);
> > + if (status != EFI_BUFFER_TOO_SMALL) {
> > + wkey_data = ERR_PTR(efi_status_to_err(status));
> > + pr_err("PM: Couldn't get wake key data size: 0x%lx\n", status);
> > + goto error;
> > + }
>
> Is it safe to completely bypass the efivars interface and access
> efi.get_variable() directly? I wouldn't have thought so, unless you can
> guarantee that the kernel isn't going to access any of the EFI runtime
> services while you execute this function.
>
This S4WakeKey is a VOLATILE variable that could not modify by
SetVariable() at runtime. So, it's read only even through efivars.
Does it what your concern?
Thanks a lot!
Joey Lee
^ permalink raw reply
* Re: [PATCH 11/18] Hibernate: introduced RSA key-pair to verify signature of snapshot
From: Matt Fleming @ 2013-09-05 8:53 UTC (permalink / raw)
To: Lee, Chun-Yi
Cc: linux-kernel, linux-security-module, linux-efi, linux-pm,
linux-crypto, opensuse-kernel, David Howells, Rafael J. Wysocki,
Matthew Garrett, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH, JKosina,
Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
Michal Marek, Gary Lin, Vivek Goyal, Lee, Chun-Yi, Takashi Iwai
In-Reply-To: <1377169317-5959-12-git-send-email-jlee@suse.com>
On Thu, 22 Aug, at 07:01:50PM, Lee, Chun-Yi wrote:
> +static int efi_status_to_err(efi_status_t status)
> +{
> + int err;
> +
> + switch (status) {
> + case EFI_INVALID_PARAMETER:
> + err = -EINVAL;
> + break;
> + case EFI_OUT_OF_RESOURCES:
> + err = -ENOSPC;
> + break;
> + case EFI_DEVICE_ERROR:
> + err = -EIO;
> + break;
> + case EFI_WRITE_PROTECTED:
> + err = -EROFS;
> + break;
> + case EFI_SECURITY_VIOLATION:
> + err = -EACCES;
> + break;
> + case EFI_NOT_FOUND:
> + err = -ENODATA;
> + break;
> + default:
> + err = -EINVAL;
> + }
> +
> + return err;
> +}
Please don't reimplement this. Instead make the existing function
global.
[...]
> +static void *load_wake_key_data(unsigned long *datasize)
> +{
> + u32 attr;
> + void *wkey_data;
> + efi_status_t status;
> +
> + if (!efi_enabled(EFI_RUNTIME_SERVICES))
> + return ERR_PTR(-EPERM);
> +
> + /* obtain the size */
> + *datasize = 0;
> + status = efi.get_variable(EFI_S4_WAKE_KEY_NAME, &EFI_HIBERNATE_GUID,
> + NULL, datasize, NULL);
> + if (status != EFI_BUFFER_TOO_SMALL) {
> + wkey_data = ERR_PTR(efi_status_to_err(status));
> + pr_err("PM: Couldn't get wake key data size: 0x%lx\n", status);
> + goto error;
> + }
Is it safe to completely bypass the efivars interface and access
efi.get_variable() directly? I wouldn't have thought so, unless you can
guarantee that the kernel isn't going to access any of the EFI runtime
services while you execute this function.
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply
* [PATCH] crypto: x86: restore avx2_supported check
From: Jussi Kivilinna @ 2013-09-03 13:49 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu
Commit 3d387ef08c4 (Revert "crypto: blowfish - add AVX2/x86_64 implementation
of blowfish cipher") reverted too much as it removed the 'assembler supports
AVX2' check and therefore disabled remaining AVX2 implementations of Camellia
and Serpent. Patch restores the check and enables these implementations.
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
---
arch/x86/crypto/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/crypto/Makefile b/arch/x86/crypto/Makefile
index 7d6ba9d..75b08e1e 100644
--- a/arch/x86/crypto/Makefile
+++ b/arch/x86/crypto/Makefile
@@ -3,6 +3,8 @@
#
avx_supported := $(call as-instr,vpxor %xmm0$(comma)%xmm0$(comma)%xmm0,yes,no)
+avx2_supported := $(call as-instr,vpgatherdd %ymm0$(comma)(%eax$(comma)%ymm1\
+ $(comma)4)$(comma)%ymm2,yes,no)
obj-$(CONFIG_CRYPTO_ABLK_HELPER_X86) += ablk_helper.o
obj-$(CONFIG_CRYPTO_GLUE_HELPER_X86) += glue_helper.o
^ permalink raw reply related
* [PATCH] crypto: sha256_ssse3 - use correct module alias for sha224
From: Jussi Kivilinna @ 2013-09-03 13:26 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu
Commit a710f761f (crypto: sha256_ssse3 - add sha224 support) attempted to add
MODULE_ALIAS for SHA-224, but it ended up being "sha384", probably because
mix-up with previous commit 340991e30 (crypto: sha512_ssse3 - add sha384
support). Patch corrects module alias to "sha224".
Reported-by: Pierre-Mayeul Badaire <pierre-mayeul.badaire@m4x.org>
Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi>
---
arch/x86/crypto/sha256_ssse3_glue.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/crypto/sha256_ssse3_glue.c b/arch/x86/crypto/sha256_ssse3_glue.c
index 50226c4..85021a4 100644
--- a/arch/x86/crypto/sha256_ssse3_glue.c
+++ b/arch/x86/crypto/sha256_ssse3_glue.c
@@ -319,4 +319,4 @@ MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("SHA256 Secure Hash Algorithm, Supplemental SSE3 accelerated");
MODULE_ALIAS("sha256");
-MODULE_ALIAS("sha384");
+MODULE_ALIAS("sha224");
^ permalink raw reply related
* Re: Mistake ?
From: Jussi Kivilinna @ 2013-09-03 13:06 UTC (permalink / raw)
To: Pierre-Mayeul Badaire; +Cc: herbert, linux-crypto@vger.kernel.org
In-Reply-To: <5225DDC0.8080202@iki.fi>
On 03.09.2013 16:01, Jussi Kivilinna wrote:
> On 03.09.2013 15:36, Pierre-Mayeul Badaire wrote:
>> Good afternoon,
>>
>> Don't you have a mistake on the MODULE_ALIAS at the last line of the commit ? Shouldn't it be MODULE_ALIAS("sha224") here ?
>
> Yes, that's correct, it should be "ssh224" instead of "sha384". I'll post patch soon.
"sha224".
-Jussi
>
> -Jussi
>
>>
>> Reference:
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a710f761fc9ae5728765a5917f8beabb49f98483
>>
>> Best regards,
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: Mistake ?
From: Jussi Kivilinna @ 2013-09-03 13:01 UTC (permalink / raw)
To: Pierre-Mayeul Badaire; +Cc: herbert, linux-crypto@vger.kernel.org
In-Reply-To: <5da2c075ce2fc47b4174aed8995fb62f@kiwi.fruits>
On 03.09.2013 15:36, Pierre-Mayeul Badaire wrote:
> Good afternoon,
>
> Don't you have a mistake on the MODULE_ALIAS at the last line of the commit ? Shouldn't it be MODULE_ALIAS("sha224") here ?
Yes, that's correct, it should be "ssh224" instead of "sha384". I'll post patch soon.
-Jussi
>
> Reference:
> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=a710f761fc9ae5728765a5917f8beabb49f98483
>
> Best regards,
>
^ permalink raw reply
* possible race in crypto_alg_mod_lookup
From: James Yonan @ 2013-09-03 12:14 UTC (permalink / raw)
To: linux-crypto
I'm seeing a GPF when code on several CPUs calls crypto_alloc_aead at
the same time, and in order for crypto_alloc_aead to satisfy the
request, it needs to lookup a kernel module (in this case aesni_intel
and aes_x86_64).
Shouldn't the bulk of the code in crypto_alg_mod_lookup be protected by
a mutex to prevent module lookup/load races?
> [ 1215.030400] general protection fault: 0000 [#1] SMP
> [ 1215.030413] Modules linked in: gcm jytest(OF) isofs dm_crypt fb_sys_fops psmouse syscopyarea sysfillrect sysimgblt serio_raw i2c_piix4 microcode mac_hid crc32_pclmul ghash_clmulni_intel aesni_intel aes_x86_64 lrw gf128mul glue_helper ablk_helper cryptd floppy
> [ 1215.030418] CPU: 31 PID: 2341 Comm: peer Tainted: GF O 3.10.10-031010-generic #201308291422
> [ 1215.030420] Hardware name: Xen HVM domU, BIOS 4.2.amazon 06/22/2013
> [ 1215.030422] task: ffff881decef4650 ti: ffff881ded528000 task.ti: ffff881ded528000
> [ 1215.030434] RIP: 0010:[<ffffffff813158eb>] [<ffffffff813158eb>] crypto_larval_kill+0x2b/0x90
> [ 1215.030436] RSP: 0018:ffff881ded529b20 EFLAGS: 00010282
> [ 1215.030437] RAX: dead000000200200 RBX: ffff883c5fadc200 RCX: dead000000200200
> [ 1215.030440] RDX: dead000000100100 RSI: 0000000000003a98 RDI: ffffffff81c82720
> [ 1215.030442] RBP: ffff881ded529b30 R08: 0000000000017950 R09: ffffea00f17e8800
> [ 1215.030444] R10: ffffffff81056610 R11: 0000000000000002 R12: fffffffffffffffe
> [ 1215.030446] R13: ffff881ded529bc8 R14: 0000000000000000 R15: ffff881decef4650
> [ 1215.030452] FS: 00007ffd369e6700(0000) GS:ffff883c9dde0000(0000) knlGS:0000000000000000
> [ 1215.030453] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 1215.030456] CR2: 00007f256543e0c0 CR3: 0000001deb69c000 CR4: 00000000000407e0
> [ 1215.030459] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 1215.030460] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 1215.030461] Stack:
> [ 1215.030468] ffff883c5fadc200 fffffffffffffffe ffff881ded529b50 ffffffff81315dab
> [ 1215.030473] ffffffff8183ffc0 0000000000000000 ffff881ded529ba0 ffffffff81315a8d
> [ 1215.030479] 0000000000000286 0000000000000020 ffff881ded529bc0 0000000000000000
> [ 1215.030481] Call Trace:
> [ 1215.030559] encrypt crypto_alloc_aead failed, err=-2
> [ 1215.030566] [<ffffffff81315dab>] crypto_alg_mod_lookup+0x6b/0x90
> [ 1215.030570] [<ffffffff81315a8d>] crypto_alloc_tfm+0x6d/0xe0
> [ 1215.030575] [<ffffffff8131c399>] crypto_alloc_ahash+0x19/0x20
> [ 1215.030585] [<ffffffffa001a996>] cryptd_alloc_ahash+0x76/0xb0 [cryptd]
> [ 1215.030593] [<ffffffff811885dc>] ? __kmalloc+0x15c/0x1a0
> [ 1215.030598] [<ffffffff81315989>] ? crypto_create_tfm+0x39/0xd0
> [ 1215.030605] [<ffffffff817057a7>] ? wait_for_completion_interruptible_timeout+0x117/0x140
> [ 1215.030609] [<ffffffff81315989>] ? crypto_create_tfm+0x39/0xd0
> [ 1215.030615] [<ffffffffa02ce2c1>] ghash_async_init_tfm+0x21/0x40 [ghash_clmulni_intel]
> [ 1215.030620] [<ffffffff813159e9>] crypto_create_tfm+0x99/0xd0
> [ 1215.030624] [<ffffffff81316c59>] crypto_spawn_tfm2+0x39/0x70
> [ 1215.030629] [<ffffffff811885dc>] ? __kmalloc+0x15c/0x1a0
> [ 1215.030635] [<ffffffffa0053bdc>] crypto_gcm_init_tfm+0x2c/0xb0 [gcm]
> [ 1215.030641] [<ffffffff81315829>] __crypto_alloc_tfm+0xc9/0x160
> [ 1215.030644] [<ffffffff8131873b>] crypto_alloc_aead+0x6b/0xc0
. . .
> [ 1215.030743] Code: 66 66 66 66 90 55 48 89 e5 53 48 89 fb 48 c7 c7 20 27 c8 81 48 83 ec 08 e8 e3 ec 3e 00 48 8b 13 48 8b 43 08 48 c7 c7 20 27 c8 81 <48> 89 42 08 48 89 10 48 b8 00 01 10 00 00 00 ad de 48 ba 00 02
> [ 1215.030748] RIP [<ffffffff813158eb>] crypto_larval_kill+0x2b/0x90
> [ 1215.030749] RSP <ffff881ded529b20>
> [ 1215.039543] ---[ end trace 3599c09d681d8550 ]---
James
^ permalink raw reply
* Re: [PATCH 10/18] efi: Enable secure boot lockdown automatically when enabled in firmware
From: Matt Fleming @ 2013-09-03 10:49 UTC (permalink / raw)
To: Lee, Chun-Yi
Cc: linux-kernel, linux-security-module, linux-efi, linux-pm,
linux-crypto, opensuse-kernel, David Howells, Rafael J. Wysocki,
Matthew Garrett, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH, JKosina,
Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
Michal Marek, Gary Lin, Vivek Goyal, Matthew Garrett,
Lee, Chun-Yi
In-Reply-To: <1377169317-5959-11-git-send-email-jlee@suse.com>
On Thu, 22 Aug, at 07:01:49PM, Lee, Chun-Yi wrote:
> From: Matthew Garrett <mjg@redhat.com>
>
> The firmware has a set of flags that indicate whether secure boot is enabled
> and enforcing. Use them to indicate whether the kernel should lock itself
> down. We also indicate the machine is in secure boot mode by adding the
> EFI_SECURE_BOOT bit for use with efi_enabled.
>
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> Signed-off-by: Josh Boyer <jwboyer@redhat.com>
> Acked-by: Lee, Chun-Yi <jlee@suse.com>
> Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
> ---
> Documentation/x86/zero-page.txt | 2 ++
> arch/x86/boot/compressed/eboot.c | 32 ++++++++++++++++++++++++++++++++
> arch/x86/include/asm/bootparam_utils.h | 8 ++++++--
> arch/x86/include/uapi/asm/bootparam.h | 3 ++-
> arch/x86/kernel/setup.c | 7 +++++++
> include/linux/cred.h | 2 ++
> include/linux/efi.h | 1 +
> 7 files changed, 52 insertions(+), 3 deletions(-)
[...]
> +static int get_secure_boot(efi_system_table_t *_table)
> +{
> + u8 sb, setup;
> + unsigned long datasize = sizeof(sb);
> + efi_guid_t var_guid = EFI_GLOBAL_VARIABLE_GUID;
> + efi_status_t status;
> +
> + status = efi_call_phys5(sys_table->runtime->get_variable,
> + L"SecureBoot", &var_guid, NULL, &datasize, &sb);
> +
The _table argument isn't needed because it's never used.
[...]
> io_delay_init();
>
> + if (boot_params.secure_boot) {
> +#ifdef CONFIG_EFI
> + set_bit(EFI_SECURE_BOOT, &x86_efi_facility);
> +#endif
> + secureboot_enable();
> + }
> +
efi_enabled(EFI_BOOT) should be checked also, instead of assuming that
secure_boot contains a sensible value.
--
Matt Fleming, Intel Open Source Technology Center
^ permalink raw reply
* Re: [PATCH] Fix bitoperation for compilation with clang
From: Herbert Xu @ 2013-09-02 10:35 UTC (permalink / raw)
To: dl9pf; +Cc: linux-kernel, pageexec, llvmlinux, behanw, davem, linux-crypto
In-Reply-To: <1377799764-14373-1-git-send-email-dl9pf@gmx.de>
On Thu, Aug 29, 2013 at 08:09:24PM +0200, dl9pf@gmx.de wrote:
> From: Jan-Simon Möller <dl9pf@gmx.de>
>
> v2: Fix bug in statement as pointed out by Herbert Xu. Kudos to pipacs.
>
> Author: PaX Team <pageexec at freemail.hu>
> ML-Post: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120507/142707.html
> URL: http://llvm.linuxfoundation.org
>
> Merge: Jan-Simon Möller <dl9pf at gmx.de>
>
> Description:
>
> Fix for warning:
> linux/crypto/fcrypt.c:143:47: warning: signed shift result (0x598000000) requires 36 bits to
> represent, but 'int' only has 32 bits [-Wshift-overflow]
> Z(0xef), Z(0x70), Z(0xcf), Z(0xc2), Z(0x2a), Z(0xb3), Z(0x61), Z(0xad),
> ^~~~~~~
> linux/crypto/fcrypt.c:113:29: note: expanded from macro 'Z'
> ^ ~~
> linux/include/uapi/linux/byteorder/little_endian.h:38:53: note: expanded from macro
> '__cpu_to_be32'
> ^
> linux/include/uapi/linux/swab.h:116:21: note: expanded from macro '__swab32'
> ___constant_swab32(x) : \
> ^
> linux/include/uapi/linux/swab.h:18:12: note: expanded from macro '___constant_swab32'
> (((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
> ^
>
> Solution - make sure we don't exceed the 32 bit range.
> #define Z(x) cpu_to_be32(((x & 0x1f) << 27) | (x >> 5))
>
> Signed-off-by: Jan-Simon Möller <dl9pf@gmx.de>
Patch applied.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v2 00/10] Series of fixes for NX driver
From: Herbert Xu @ 2013-09-02 10:35 UTC (permalink / raw)
To: Marcelo Cerri; +Cc: linux-kernel, linux-crypto, linuxppc-dev, benh
In-Reply-To: <1377787000-4966-1-git-send-email-mhcerri@linux.vnet.ibm.com>
On Thu, Aug 29, 2013 at 11:36:30AM -0300, Marcelo Cerri wrote:
> This series of patches contains fixes in several algorithms implemented
> by the NX driver. The patches can be separated in three different
> categories:
>
> - Changes to split the data in several hyper calls to respect the
> limits of data that the co-processador can handle. This affects
> all AES modes.
> - Fixes in how the driver handle zero length messages. This affects
> XCBC and GCM.
> - Fixes for SHA-2 when chunks bigger than the block size are provided.
>
> v2:
> - Fixed conflict.
All applied. Thanks!
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot
From: joeyli @ 2013-09-02 2:12 UTC (permalink / raw)
To: Florian Weimer
Cc: Matthew Garrett, linux-kernel, linux-security-module, linux-efi,
linux-pm, linux-crypto, opensuse-kernel, David Howells,
Rafael J. Wysocki, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH, JKosina,
Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <87vc2ksdfa.fsf@mid.deneb.enyo.de>
於 日，2013-09-01 於 18:40 +0200，Florian Weimer 提到：
> * Matthew Garrett:
>
> > On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
> >
> >> But if you don't generate fresh keys on every boot, the persistent
> >> keys are mor exposed to other UEFI applications. Correct me if I'm
> >> wrong, but I don't think UEFI variables are segregated between
> >> different UEFI applications, so if anyone gets a generic UEFI variable
> >> dumper (or setter) signed by the trusted key, this cryptographic
> >> validation of hibernate snapshots is bypassable.
> >
> > If anyone can execute arbitrary code in your UEFI environment then
> > you've already lost.
>
> This is not about arbitrary code execution. The problematic
> applications which conflict with this proposed functionality are not
> necessarily malicious by themselves and even potentially useful.
>
> For example, if you want to provision a bunch of machines and you have
> to set certain UEFI variables, it might be helpful to do so in an
> unattended fashion, just by booting from a USB stick with a suitable
> UEFI application. Is this evil? I don't think so.
> --
Yes, if there have the kind of UEFI tools like you said, and it leak to
attacker, then we lost.
Even we re-generate key-pair for every S4, the tool, if it can set
variable, means it can replace the public key that was stored by efi
bootloader in bootservices variable. Then we still lost.
When the tool can only dump variable but not set, then re-generate
key-pair to every S4 can prevent it. If the tool can also set variable,
then I don't think there have any way to protect key-pair in UEFI
variables.
Using TPM is a way to protect key-pair, but user need key-in password
when generate and use key to sign stuff. It noises to user, but the best
way to keep the password is in brain.
Thanks a lot!
Joey Lee
^ permalink raw reply
* Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot
From: joeyli @ 2013-09-02 2:12 UTC (permalink / raw)
To: Florian Weimer
Cc: Matthew Garrett, linux-kernel, linux-security-module, linux-efi,
linux-pm, linux-crypto, opensuse-kernel, David Howells,
Rafael J. Wysocki, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH, JKosina,
Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <87vc2ksdfa.fsf@mid.deneb.enyo.de>
於 日，2013-09-01 於 18:40 +0200，Florian Weimer 提到：
> * Matthew Garrett:
>
> > On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
> >
> >> But if you don't generate fresh keys on every boot, the persistent
> >> keys are mor exposed to other UEFI applications. Correct me if I'm
> >> wrong, but I don't think UEFI variables are segregated between
> >> different UEFI applications, so if anyone gets a generic UEFI variable
> >> dumper (or setter) signed by the trusted key, this cryptographic
> >> validation of hibernate snapshots is bypassable.
> >
> > If anyone can execute arbitrary code in your UEFI environment then
> > you've already lost.
>
> This is not about arbitrary code execution. The problematic
> applications which conflict with this proposed functionality are not
> necessarily malicious by themselves and even potentially useful.
>
> For example, if you want to provision a bunch of machines and you have
> to set certain UEFI variables, it might be helpful to do so in an
> unattended fashion, just by booting from a USB stick with a suitable
> UEFI application. Is this evil? I don't think so.
> --
Yes, if there have the kind of UEFI tools like you said, and it leak to
attacker, then we lost.
Even we re-generate key-pair for every S4, the tool, if it can set
variable, means it can replace the public key that was stored by efi
bootloader in bootservices variable. Then we still lost.
When the tool can only dump variable but not set, then re-generate
key-pair to every S4 can prevent it. If the tool can also set variable,
then I don't think there have any way to protect key-pair in UEFI
variables.
Using TPM is a way to protect key-pair, but user need key-in password
when generate and use key to sign stuff. It noises to user, but the best
way to keep the password is in brain.
Thanks a lot!
Joey Lee
^ permalink raw reply
* Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot
From: joeyli @ 2013-09-02 2:12 UTC (permalink / raw)
To: Florian Weimer
Cc: Matthew Garrett, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
opensuse-kernel-stAJ6ESoqRxg9hUCZPvPmw, David Howells,
Rafael J. Wysocki, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH,
JKosina-IBi9RG/b67k, Rusty Russell, Herbert Xu, David S. Miller,
H. Peter Anvin, Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <87vc2ksdfa.fsf-ZqZwdwZz9NfTBotR3TxKnbNAH6kLmebB@public.gmane.org>
於 日，2013-09-01 於 18:40 +0200，Florian Weimer 提到：
> * Matthew Garrett:
>
> > On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
> >
> >> But if you don't generate fresh keys on every boot, the persistent
> >> keys are mor exposed to other UEFI applications. Correct me if I'm
> >> wrong, but I don't think UEFI variables are segregated between
> >> different UEFI applications, so if anyone gets a generic UEFI variable
> >> dumper (or setter) signed by the trusted key, this cryptographic
> >> validation of hibernate snapshots is bypassable.
> >
> > If anyone can execute arbitrary code in your UEFI environment then
> > you've already lost.
>
> This is not about arbitrary code execution. The problematic
> applications which conflict with this proposed functionality are not
> necessarily malicious by themselves and even potentially useful.
>
> For example, if you want to provision a bunch of machines and you have
> to set certain UEFI variables, it might be helpful to do so in an
> unattended fashion, just by booting from a USB stick with a suitable
> UEFI application. Is this evil? I don't think so.
> --
Yes, if there have the kind of UEFI tools like you said, and it leak to
attacker, then we lost.
Even we re-generate key-pair for every S4, the tool, if it can set
variable, means it can replace the public key that was stored by efi
bootloader in bootservices variable. Then we still lost.
When the tool can only dump variable but not set, then re-generate
key-pair to every S4 can prevent it. If the tool can also set variable,
then I don't think there have any way to protect key-pair in UEFI
variables.
Using TPM is a way to protect key-pair, but user need key-in password
when generate and use key to sign stuff. It noises to user, but the best
way to keep the password is in brain.
Thanks a lot!
Joey Lee
^ permalink raw reply
* Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot
From: joeyli @ 2013-09-02 2:12 UTC (permalink / raw)
To: Florian Weimer
Cc: Matthew Garrett, linux-kernel, linux-security-module, linux-efi,
linux-pm, linux-crypto, opensuse-kernel, David Howells,
Rafael J. Wysocki, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH, JKosina,
Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <87vc2ksdfa.fsf@mid.deneb.enyo.de>
於 日，2013-09-01 於 18:40 +0200，Florian Weimer 提到：
> * Matthew Garrett:
>
> > On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
> >
> >> But if you don't generate fresh keys on every boot, the persistent
> >> keys are mor exposed to other UEFI applications. Correct me if I'm
> >> wrong, but I don't think UEFI variables are segregated between
> >> different UEFI applications, so if anyone gets a generic UEFI variable
> >> dumper (or setter) signed by the trusted key, this cryptographic
> >> validation of hibernate snapshots is bypassable.
> >
> > If anyone can execute arbitrary code in your UEFI environment then
> > you've already lost.
>
> This is not about arbitrary code execution. The problematic
> applications which conflict with this proposed functionality are not
> necessarily malicious by themselves and even potentially useful.
>
> For example, if you want to provision a bunch of machines and you have
> to set certain UEFI variables, it might be helpful to do so in an
> unattended fashion, just by booting from a USB stick with a suitable
> UEFI application. Is this evil? I don't think so.
> --
Yes, if there have the kind of UEFI tools like you said, and it leak to
attacker, then we lost.
Even we re-generate key-pair for every S4, the tool, if it can set
variable, means it can replace the public key that was stored by efi
bootloader in bootservices variable. Then we still lost.
When the tool can only dump variable but not set, then re-generate
key-pair to every S4 can prevent it. If the tool can also set variable,
then I don't think there have any way to protect key-pair in UEFI
variables.
Using TPM is a way to protect key-pair, but user need key-in password
when generate and use key to sign stuff. It noises to user, but the best
way to keep the password is in brain.
Thanks a lot!
Joey Lee
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot
From: joeyli @ 2013-09-02 2:12 UTC (permalink / raw)
To: Florian Weimer
Cc: Matthew Garrett, linux-kernel, linux-security-module, linux-efi,
linux-pm, linux-crypto, opensuse-kernel, David Howells,
Rafael J. Wysocki, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH, JKosina,
Rusty Russell, Herbert Xu, David S. Miller, H. Peter Anvin,
Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <87vc2ksdfa.fsf@mid.deneb.enyo.de>
於 日，2013-09-01 於 18:40 +0200，Florian Weimer 提到：
> * Matthew Garrett:
>
> > On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
> >
> >> But if you don't generate fresh keys on every boot, the persistent
> >> keys are mor exposed to other UEFI applications. Correct me if I'm
> >> wrong, but I don't think UEFI variables are segregated between
> >> different UEFI applications, so if anyone gets a generic UEFI variable
> >> dumper (or setter) signed by the trusted key, this cryptographic
> >> validation of hibernate snapshots is bypassable.
> >
> > If anyone can execute arbitrary code in your UEFI environment then
> > you've already lost.
>
> This is not about arbitrary code execution. The problematic
> applications which conflict with this proposed functionality are not
> necessarily malicious by themselves and even potentially useful.
>
> For example, if you want to provision a bunch of machines and you have
> to set certain UEFI variables, it might be helpful to do so in an
> unattended fashion, just by booting from a USB stick with a suitable
> UEFI application. Is this evil? I don't think so.
> --
Yes, if there have the kind of UEFI tools like you said, and it leak to
attacker, then we lost.
Even we re-generate key-pair for every S4, the tool, if it can set
variable, means it can replace the public key that was stored by efi
bootloader in bootservices variable. Then we still lost.
When the tool can only dump variable but not set, then re-generate
key-pair to every S4 can prevent it. If the tool can also set variable,
then I don't think there have any way to protect key-pair in UEFI
variables.
Using TPM is a way to protect key-pair, but user need key-in password
when generate and use key to sign stuff. It noises to user, but the best
way to keep the password is in brain.
Thanks a lot!
Joey Lee
^ permalink raw reply
* Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot
From: Matthew Garrett @ 2013-09-01 16:46 UTC (permalink / raw)
To: Florian Weimer
Cc: joeyli, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
opensuse-kernel-stAJ6ESoqRxg9hUCZPvPmw, David Howells,
Rafael J. Wysocki, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH,
JKosina-IBi9RG/b67k, Rusty Russell, Herbert Xu, David S. Miller,
H. Peter Anvin, Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <87vc2ksdfa.fsf-ZqZwdwZz9NfTBotR3TxKnbNAH6kLmebB@public.gmane.org>
On Sun, Sep 01, 2013 at 06:40:41PM +0200, Florian Weimer wrote:
> * Matthew Garrett:
>
> > On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
> >
> >> But if you don't generate fresh keys on every boot, the persistent
> >> keys are mor exposed to other UEFI applications. Correct me if I'm
> >> wrong, but I don't think UEFI variables are segregated between
> >> different UEFI applications, so if anyone gets a generic UEFI variable
> >> dumper (or setter) signed by the trusted key, this cryptographic
> >> validation of hibernate snapshots is bypassable.
> >
> > If anyone can execute arbitrary code in your UEFI environment then
> > you've already lost.
>
> This is not about arbitrary code execution. The problematic
> applications which conflict with this proposed functionality are not
> necessarily malicious by themselves and even potentially useful.
A signed application that permits the modification of arbitrary boot
services variables *is* malicious. No implementation is designed to be
safe in that scenario. Why bother with modifying encryption keys when
you can just modify MOK instead?
--
Matthew Garrett | mjg59-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org
^ permalink raw reply
* Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot
From: Florian Weimer @ 2013-09-01 16:40 UTC (permalink / raw)
To: Matthew Garrett
Cc: joeyli, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
opensuse-kernel-stAJ6ESoqRxg9hUCZPvPmw, David Howells,
Rafael J. Wysocki, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH,
JKosina-IBi9RG/b67k, Rusty Russell, Herbert Xu, David S. Miller,
H. Peter Anvin, Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <20130901160429.GA1375-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
* Matthew Garrett:
> On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
>
>> But if you don't generate fresh keys on every boot, the persistent
>> keys are mor exposed to other UEFI applications. Correct me if I'm
>> wrong, but I don't think UEFI variables are segregated between
>> different UEFI applications, so if anyone gets a generic UEFI variable
>> dumper (or setter) signed by the trusted key, this cryptographic
>> validation of hibernate snapshots is bypassable.
>
> If anyone can execute arbitrary code in your UEFI environment then
> you've already lost.
This is not about arbitrary code execution. The problematic
applications which conflict with this proposed functionality are not
necessarily malicious by themselves and even potentially useful.
For example, if you want to provision a bunch of machines and you have
to set certain UEFI variables, it might be helpful to do so in an
unattended fashion, just by booting from a USB stick with a suitable
UEFI application. Is this evil? I don't think so.
^ permalink raw reply
* Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot
From: Matthew Garrett @ 2013-09-01 16:04 UTC (permalink / raw)
To: Florian Weimer
Cc: joeyli, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
opensuse-kernel-stAJ6ESoqRxg9hUCZPvPmw, David Howells,
Rafael J. Wysocki, Len Brown, Pavel Machek, Josh Boyer,
Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH,
JKosina-IBi9RG/b67k, Rusty Russell, Herbert Xu, David S. Miller,
H. Peter Anvin, Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <87r4d8vn71.fsf-ZqZwdwZz9NfTBotR3TxKnbNAH6kLmebB@public.gmane.org>
On Sun, Sep 01, 2013 at 12:41:22PM +0200, Florian Weimer wrote:
> But if you don't generate fresh keys on every boot, the persistent
> keys are mor exposed to other UEFI applications. Correct me if I'm
> wrong, but I don't think UEFI variables are segregated between
> different UEFI applications, so if anyone gets a generic UEFI variable
> dumper (or setter) signed by the trusted key, this cryptographic
> validation of hibernate snapshots is bypassable.
If anyone can execute arbitrary code in your UEFI environment then
you've already lost.
--
Matthew Garrett | mjg59-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org
^ permalink raw reply
* Re: [RFC PATCH 00/18 v3] Signature verification of hibernate snapshot
From: Florian Weimer @ 2013-09-01 10:41 UTC (permalink / raw)
To: joeyli
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-security-module-u79uwXL29TY76Z2rM5mHXA,
linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
opensuse-kernel-stAJ6ESoqRxg9hUCZPvPmw, David Howells,
Rafael J. Wysocki, Matthew Garrett, Len Brown, Pavel Machek,
Josh Boyer, Vojtech Pavlik, Matt Fleming, James Bottomley,
Greg KH, JKosina-IBi9RG/b67k, Rusty Russell, Herbert Xu,
David S. Miller, H. Peter Anvin, Michal Marek, Gary Lin,
Vivek Goyal
In-Reply-To: <1377734505.19568.39.camel-ONCj+Eqt86TasUa73XJKwA@public.gmane.org>
* joeyli:
> Yes, Matthew raised this concern at before. I modified patch to load
> private key in efi stub kernel, before ExitBootServices(), that means we
> don't need generate key-pair at every system boot. So, the above
> procedure of efi bootloader will only run one time.
But if you don't generate fresh keys on every boot, the persistent
keys are mor exposed to other UEFI applications. Correct me if I'm
wrong, but I don't think UEFI variables are segregated between
different UEFI applications, so if anyone gets a generic UEFI variable
dumper (or setter) signed by the trusted key, this cryptographic
validation of hibernate snapshots is bypassable.
^ permalink raw reply
* Re: [PATCH 1/3] crypto: omap-des: Add omap-des driver for OMAP4/AM43xx
From: Rajendra Nayak @ 2013-08-30 9:19 UTC (permalink / raw)
To: Joel Fernandes
Cc: Herbert Xu, David S. Miller, Mark Greer, Tony Lindgren,
Lokesh Vutla, Linux OMAP List, Linux Kernel Mailing List,
Linux ARM Kernel List, Linux Crypto Mailing List
In-Reply-To: <1377818873-21174-2-git-send-email-joelf@ti.com>
[]..
> +
> +#define pr_fmt(fmt) "%s: " fmt, __func__
> +
> +#ifdef DEBUG
> +#define prn(num) printk(#num "=%d\n", num)
> +#define prx(num) printk(#num "=%x\n", num)
> +#else
> +#define prn(num) do { } while (0)
> +#define prx(num) do { } while (0)
> +#endif
> +
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/errno.h>
> +#include <linux/kernel.h>
> +#include <linux/platform_device.h>
> +#include <linux/scatterlist.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/dmaengine.h>
> +#include <linux/omap-dma.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/io.h>
> +#include <linux/crypto.h>
> +#include <linux/interrupt.h>
> +#include <crypto/scatterwalk.h>
> +#include <crypto/des.h>
> +
> +#define DST_MAXBURST 2
> +
> +#define DES_BLOCK_WORDS (DES_BLOCK_SIZE >> 2)
> +
> +#define _calc_walked(inout) (dd->inout##_walk.offset - dd->inout##_sg->offset)
> +
> +#define DES_REG_KEY(dd, x) ((dd)->pdata->key_ofs - \
> + ((x ^ 0x01) * 0x04))
> +
> +#define DES_REG_IV(dd, x) ((dd)->pdata->iv_ofs + ((x) * 0x04))
> +
> +#define DES_REG_CTRL(dd) ((dd)->pdata->ctrl_ofs)
> +#define DES_REG_CTRL_CBC (1 << 4)
> +#define DES_REG_CTRL_TDES (1 << 3)
> +#define DES_REG_CTRL_DIRECTION (1 << 2)
> +#define DES_REG_CTRL_INPUT_READY (1 << 1)
> +#define DES_REG_CTRL_OUTPUT_READY (1 << 0)
Why not use bitops like you have done below.
> +
> +#define DES_REG_DATA_N(dd, x) ((dd)->pdata->data_ofs + ((x) * 0x04))
> +
> +#define DES_REG_REV(dd) ((dd)->pdata->rev_ofs)
> +
> +#define DES_REG_MASK(dd) ((dd)->pdata->mask_ofs)
> +
> +#define DES_REG_LENGTH_N(x) (0x24 + ((x) * 0x04))
> +
> +#define DES_REG_IRQ_STATUS(dd) ((dd)->pdata->irq_status_ofs)
> +#define DES_REG_IRQ_ENABLE(dd) ((dd)->pdata->irq_enable_ofs)
> +#define DES_REG_IRQ_DATA_IN BIT(1)
> +#define DES_REG_IRQ_DATA_OUT BIT(2)
> +
> +#define FLAGS_MODE_MASK 0x000f
> +#define FLAGS_ENCRYPT BIT(0)
> +#define FLAGS_CBC BIT(1)
> +#define FLAGS_INIT BIT(4)
> +#define FLAGS_BUSY BIT(6)
> +
[]..
> +struct omap_des_pdata {
> + struct omap_des_algs_info *algs_info;
> + unsigned int algs_info_size;
> +
> + void (*trigger)(struct omap_des_dev *dd, int length);
Is this really used? How does a DT platform pass function pointers?
> +
> + u32 key_ofs;
> + u32 iv_ofs;
> + u32 ctrl_ofs;
> + u32 data_ofs;
> + u32 rev_ofs;
> + u32 mask_ofs;
> + u32 irq_enable_ofs;
> + u32 irq_status_ofs;
> +
> + u32 dma_enable_in;
> + u32 dma_enable_out;
> + u32 dma_start;
> +
> + u32 major_mask;
> + u32 major_shift;
> + u32 minor_mask;
> + u32 minor_shift;
> +};
> +
> +struct omap_des_dev {
> + struct list_head list;
> + unsigned long phys_base;
> + void __iomem *io_base;
> + struct omap_des_ctx *ctx;
> + struct device *dev;
> + unsigned long flags;
> + int err;
> +
> + /* spinlock used for queues */
> + spinlock_t lock;
> + struct crypto_queue queue;
> +
> + struct tasklet_struct done_task;
> + struct tasklet_struct queue_task;
> +
> + struct ablkcipher_request *req;
> + /*
> + * total is used by PIO mode for book keeping so introduce
> + * variable total_save as need it to calc page_order
> + */
> + size_t total;
> + size_t total_save;
> +
> + struct scatterlist *in_sg;
> + struct scatterlist *out_sg;
> +
> + /* Buffers for copying for unaligned cases */
> + struct scatterlist in_sgl;
> + struct scatterlist out_sgl;
> + struct scatterlist *orig_out;
> + int sgs_copied;
> +
> + struct scatter_walk in_walk;
> + struct scatter_walk out_walk;
> + int dma_in;
> + struct dma_chan *dma_lch_in;
> + int dma_out;
> + struct dma_chan *dma_lch_out;
> + int in_sg_len;
> + int out_sg_len;
> + int pio_only;
> + const struct omap_des_pdata *pdata;
> +};
> +
> +/* keep registered devices data here */
> +static LIST_HEAD(dev_list);
> +static DEFINE_SPINLOCK(list_lock);
> +
[]..
> +
> +static int omap_des_crypt_dma_start(struct omap_des_dev *dd)
> +{
> + struct crypto_tfm *tfm = crypto_ablkcipher_tfm(
> + crypto_ablkcipher_reqtfm(dd->req));
> + int err;
> +
> + pr_debug("total: %d\n", dd->total);
> +
> + if (!dd->pio_only) {
> + err = dma_map_sg(dd->dev, dd->in_sg, dd->in_sg_len,
> + DMA_TO_DEVICE);
> + if (!err) {
> + dev_err(dd->dev, "dma_map_sg() error\n");
> + return -EINVAL;
> + }
> +
> + err = dma_map_sg(dd->dev, dd->out_sg, dd->out_sg_len,
> + DMA_FROM_DEVICE);
> + if (!err) {
> + dev_err(dd->dev, "dma_map_sg() error\n");
> + return -EINVAL;
> + }
> + }
> +
> + err = omap_des_crypt_dma(tfm, dd->in_sg, dd->out_sg, dd->in_sg_len,
> + dd->out_sg_len);
> + if (err && !dd->pio_only) {
> + dma_unmap_sg(dd->dev, dd->in_sg, dd->in_sg_len, DMA_TO_DEVICE);
> + dma_unmap_sg(dd->dev, dd->out_sg, dd->out_sg_len,
> + DMA_FROM_DEVICE);
> + }
> +
> + return err;
> +}
> +
> +static void omap_des_finish_req(struct omap_des_dev *dd, int err)
> +{
> + struct ablkcipher_request *req = dd->req;
> +
> + pr_debug("err: %d\n", err);
> +
> + pm_runtime_put(dd->dev);
You seem to do a pm_runtime_get_sync() in omap_des_write_ctrl() and a
pm_runtime_put() here and not a pm_runtime_put_sync()?
> + dd->flags &= ~FLAGS_BUSY;
> +
> + req->base.complete(&req->base, err);
> +}
> +
> +static int omap_des_crypt_dma_stop(struct omap_des_dev *dd)
> +{
> + int err = 0;
> +
> + pr_debug("total: %d\n", dd->total);
> +
> + omap_des_dma_stop(dd);
> +
> + dmaengine_terminate_all(dd->dma_lch_in);
> + dmaengine_terminate_all(dd->dma_lch_out);
> +
> + dma_unmap_sg(dd->dev, dd->in_sg, dd->in_sg_len, DMA_TO_DEVICE);
> + dma_unmap_sg(dd->dev, dd->out_sg, dd->out_sg_len, DMA_FROM_DEVICE);
> +
> + return err;
> +}
> +
> +int omap_des_copy_needed(struct scatterlist *sg)
> +{
> + while (sg) {
> + if (!IS_ALIGNED(sg->offset, 4))
> + return -1;
> + if (!IS_ALIGNED(sg->length, DES_BLOCK_SIZE))
> + return -1;
> + sg = sg_next(sg);
> + }
> + return 0;
> +}
> +
> +int omap_des_copy_sgs(struct omap_des_dev *dd)
> +{
> + void *buf_in, *buf_out;
> + int pages;
> +
> + pages = dd->total >> PAGE_SHIFT;
> +
> + if (dd->total & (PAGE_SIZE-1))
> + pages++;
> +
> + BUG_ON(!pages);
> +
> + buf_in = (void *)__get_free_pages(GFP_ATOMIC, pages);
> + buf_out = (void *)__get_free_pages(GFP_ATOMIC, pages);
> +
> + if (!buf_in || !buf_out) {
> + pr_err("Couldn't allocated pages for unaligned cases.\n");
> + return -1;
> + }
> +
> + dd->orig_out = dd->out_sg;
> +
> + sg_copy_buf(buf_in, dd->in_sg, 0, dd->total, 0);
> +
> + sg_init_table(&dd->in_sgl, 1);
> + sg_set_buf(&dd->in_sgl, buf_in, dd->total);
> + dd->in_sg = &dd->in_sgl;
> +
> + sg_init_table(&dd->out_sgl, 1);
> + sg_set_buf(&dd->out_sgl, buf_out, dd->total);
> + dd->out_sg = &dd->out_sgl;
> +
> + return 0;
> +}
> +
[]..
> +
> +#ifdef CONFIG_OF
> +static const struct omap_des_pdata omap_des_pdata_omap4 = {
> + .algs_info = omap_des_algs_info_ecb_cbc,
> + .algs_info_size = ARRAY_SIZE(omap_des_algs_info_ecb_cbc),
> + .trigger = omap_des_dma_trigger_omap4,
> + .key_ofs = 0x14,
> + .iv_ofs = 0x18,
> + .ctrl_ofs = 0x20,
> + .data_ofs = 0x28,
> + .rev_ofs = 0x30,
> + .mask_ofs = 0x34,
> + .irq_status_ofs = 0x3c,
> + .irq_enable_ofs = 0x40,
> + .dma_enable_in = BIT(5),
> + .dma_enable_out = BIT(6),
> + .major_mask = 0x0700,
> + .major_shift = 8,
> + .minor_mask = 0x003f,
> + .minor_shift = 0,
> +};
> +
> +static irqreturn_t omap_des_irq(int irq, void *dev_id)
> +{
> + struct omap_des_dev *dd = dev_id;
> + u32 status, i;
> + u32 *src, *dst;
> +
> + status = omap_des_read(dd, DES_REG_IRQ_STATUS(dd));
> + if (status & DES_REG_IRQ_DATA_IN) {
> + omap_des_write(dd, DES_REG_IRQ_ENABLE(dd), 0x0);
> +
> + BUG_ON(!dd->in_sg);
> +
> + BUG_ON(_calc_walked(in) > dd->in_sg->length);
> +
> + src = sg_virt(dd->in_sg) + _calc_walked(in);
> +
> + for (i = 0; i < DES_BLOCK_WORDS; i++) {
> + omap_des_write(dd, DES_REG_DATA_N(dd, i), *src);
> +
> + scatterwalk_advance(&dd->in_walk, 4);
> + if (dd->in_sg->length == _calc_walked(in)) {
> + dd->in_sg = scatterwalk_sg_next(dd->in_sg);
> + if (dd->in_sg) {
> + scatterwalk_start(&dd->in_walk,
> + dd->in_sg);
> + src = sg_virt(dd->in_sg) +
> + _calc_walked(in);
> + }
> + } else {
> + src++;
> + }
> + }
> +
> + /* Clear IRQ status */
> + status &= ~DES_REG_IRQ_DATA_IN;
> + omap_des_write(dd, DES_REG_IRQ_STATUS(dd), status);
> +
> + /* Enable DATA_OUT interrupt */
> + omap_des_write(dd, DES_REG_IRQ_ENABLE(dd), 0x4);
> +
> + } else if (status & DES_REG_IRQ_DATA_OUT) {
> + omap_des_write(dd, DES_REG_IRQ_ENABLE(dd), 0x0);
> +
> + BUG_ON(!dd->out_sg);
> +
> + BUG_ON(_calc_walked(out) > dd->out_sg->length);
> +
> + dst = sg_virt(dd->out_sg) + _calc_walked(out);
> +
> + for (i = 0; i < DES_BLOCK_WORDS; i++) {
> + *dst = omap_des_read(dd, DES_REG_DATA_N(dd, i));
> + scatterwalk_advance(&dd->out_walk, 4);
> + if (dd->out_sg->length == _calc_walked(out)) {
> + dd->out_sg = scatterwalk_sg_next(dd->out_sg);
> + if (dd->out_sg) {
> + scatterwalk_start(&dd->out_walk,
> + dd->out_sg);
> + dst = sg_virt(dd->out_sg) +
> + _calc_walked(out);
> + }
> + } else {
> + dst++;
> + }
> + }
> +
> + dd->total -= DES_BLOCK_SIZE;
> +
> + BUG_ON(dd->total < 0);
> +
> + /* Clear IRQ status */
> + status &= ~DES_REG_IRQ_DATA_OUT;
> + omap_des_write(dd, DES_REG_IRQ_STATUS(dd), status);
> +
> + if (!dd->total)
> + /* All bytes read! */
> + tasklet_schedule(&dd->done_task);
> + else
> + /* Enable DATA_IN interrupt for next block */
> + omap_des_write(dd, DES_REG_IRQ_ENABLE(dd), 0x2);
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static const struct of_device_id omap_des_of_match[] = {
> + {
> + .compatible = "ti,omap4-des",
> + .data = &omap_des_pdata_omap4,
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, omap_des_of_match);
> +
> +static int omap_des_get_res_of(struct omap_des_dev *dd,
> + struct device *dev, struct resource *res)
> +{
> + struct device_node *node = dev->of_node;
> + const struct of_device_id *match;
> + int err = 0;
> +
> + match = of_match_device(of_match_ptr(omap_des_of_match), dev);
> + if (!match) {
> + dev_err(dev, "no compatible OF match\n");
> + err = -EINVAL;
> + goto err;
> + }
> +
> + err = of_address_to_resource(node, 0, res);
Do you really need to do this? DT should have already populated
a resource for you based on the 'reg' property.
> + if (err < 0) {
> + dev_err(dev, "can't translate OF node address\n");
> + err = -EINVAL;
> + goto err;
> + }
> +
> + dd->dma_out = -1; /* Dummy value that's unused */
> + dd->dma_in = -1; /* Dummy value that's unused */
> +
> + dd->pdata = match->data;
> +
> +err:
> + return err;
> +}
> +#else
> +static const struct of_device_id omap_des_of_match[] = {
> + {},
> +};
you don't need this if you use of_match_ptr()
> +
> +static int omap_des_get_res_of(struct omap_des_dev *dd,
> + struct device *dev, struct resource *res)
> +{
> + return -EINVAL;
> +}
> +#endif
> +
> +static int omap_des_get_res_pdev(struct omap_des_dev *dd,
> + struct platform_device *pdev, struct resource *res)
> +{
> + struct device *dev = &pdev->dev;
> + struct resource *r;
> + int err = 0;
> +
> + /* Get the base address */
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!r) {
> + dev_err(dev, "no MEM resource info\n");
> + err = -ENODEV;
> + goto err;
> + }
> + memcpy(res, r, sizeof(*res));
I don't think you need any of this. Regardless of a DT or a
non-DT platform, you should be able to do a platform_get_resource()
for mem resources.
> +
> + /* Get the DMA out channel */
> + r = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> + if (!r) {
> + dev_err(dev, "no DMA out resource info\n");
> + err = -ENODEV;
> + goto err;
> + }
> + dd->dma_out = r->start;
> +
> + /* Get the DMA in channel */
> + r = platform_get_resource(pdev, IORESOURCE_DMA, 1);
> + if (!r) {
> + dev_err(dev, "no DMA in resource info\n");
> + err = -ENODEV;
> + goto err;
> + }
> + dd->dma_in = r->start;
> +
> + /* non-DT devices get pdata from pdev */
> + dd->pdata = pdev->dev.platform_data;
> +
> +err:
> + return err;
> +}
> +
> +static int omap_des_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct omap_des_dev *dd;
> + struct crypto_alg *algp;
> + struct resource res;
> + int err = -ENOMEM, i, j, irq = -1;
> + u32 reg;
> +
> + dd = devm_kzalloc(dev, sizeof(struct omap_des_dev), GFP_KERNEL);
> + if (dd == NULL) {
> + dev_err(dev, "unable to alloc data struct.\n");
> + goto err_data;
> + }
> + dd->dev = dev;
> + platform_set_drvdata(pdev, dd);
> +
> + spin_lock_init(&dd->lock);
> + crypto_init_queue(&dd->queue, OMAP_DES_QUEUE_LENGTH);
> +
> + err = (dev->of_node) ? omap_des_get_res_of(dd, dev, &res) :
> + omap_des_get_res_pdev(dd, pdev, &res);
> + if (err)
> + goto err_res;
> +
> + dd->io_base = devm_request_and_ioremap(dev, &res);
> + if (!dd->io_base) {
> + dev_err(dev, "can't ioremap\n");
> + err = -ENOMEM;
> + goto err_res;
> + }
> + dd->phys_base = res.start;
> +
> + pm_runtime_enable(dev);
> + pm_runtime_get_sync(dev);
> +
> + omap_des_dma_stop(dd);
> +
> + reg = omap_des_read(dd, DES_REG_REV(dd));
> +
> + pm_runtime_put_sync(dev);
> +
> + dev_info(dev, "OMAP DES hw accel rev: %u.%u\n",
> + (reg & dd->pdata->major_mask) >> dd->pdata->major_shift,
> + (reg & dd->pdata->minor_mask) >> dd->pdata->minor_shift);
> +
> + tasklet_init(&dd->done_task, omap_des_done_task, (unsigned long)dd);
> + tasklet_init(&dd->queue_task, omap_des_queue_task, (unsigned long)dd);
> +
> + err = omap_des_dma_init(dd);
> + if (err && DES_REG_IRQ_STATUS(dd) && DES_REG_IRQ_ENABLE(dd)) {
> + dd->pio_only = 1;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(dev, "can't get IRQ resource\n");
> + goto err_irq;
> + }
> +
> + err = devm_request_irq(dev, irq, omap_des_irq, 0,
> + dev_name(dev), dd);
> + if (err) {
> + dev_err(dev, "Unable to grab omap-des IRQ\n");
> + goto err_irq;
> + }
> + }
> +
> +
> + INIT_LIST_HEAD(&dd->list);
> + spin_lock(&list_lock);
> + list_add_tail(&dd->list, &dev_list);
> + spin_unlock(&list_lock);
> +
> + for (i = 0; i < dd->pdata->algs_info_size; i++) {
> + for (j = 0; j < dd->pdata->algs_info[i].size; j++) {
> + algp = &dd->pdata->algs_info[i].algs_list[j];
> +
> + pr_debug("reg alg: %s\n", algp->cra_name);
> + INIT_LIST_HEAD(&algp->cra_list);
> +
> + err = crypto_register_alg(algp);
> + if (err)
> + goto err_algs;
> +
> + dd->pdata->algs_info[i].registered++;
> + }
> + }
> +
> + return 0;
> +err_algs:
> + for (i = dd->pdata->algs_info_size - 1; i >= 0; i--)
> + for (j = dd->pdata->algs_info[i].registered - 1; j >= 0; j--)
> + crypto_unregister_alg(
> + &dd->pdata->algs_info[i].algs_list[j]);
> + if (!dd->pio_only)
> + omap_des_dma_cleanup(dd);
> +err_irq:
> + tasklet_kill(&dd->done_task);
> + tasklet_kill(&dd->queue_task);
> + pm_runtime_disable(dev);
> +err_res:
> + dd = NULL;
> +err_data:
> + dev_err(dev, "initialization failed.\n");
> + return err;
> +}
> +
> +static int omap_des_remove(struct platform_device *pdev)
> +{
> + struct omap_des_dev *dd = platform_get_drvdata(pdev);
> + int i, j;
> +
> + if (!dd)
> + return -ENODEV;
> +
> + spin_lock(&list_lock);
> + list_del(&dd->list);
> + spin_unlock(&list_lock);
> +
> + for (i = dd->pdata->algs_info_size - 1; i >= 0; i--)
> + for (j = dd->pdata->algs_info[i].registered - 1; j >= 0; j--)
> + crypto_unregister_alg(
> + &dd->pdata->algs_info[i].algs_list[j]);
> +
> + tasklet_kill(&dd->done_task);
> + tasklet_kill(&dd->queue_task);
> + omap_des_dma_cleanup(dd);
> + pm_runtime_disable(dd->dev);
> + dd = NULL;
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int omap_des_suspend(struct device *dev)
> +{
> + pm_runtime_put_sync(dev);
I know you seemed to have taken this from the omap-aes driver, but this
does not seem correct. Your system suspend callbacks shouldn't
really be using pm_runtime apis.
On OMAPs this is handled by the pm_domain level callbacks, in case
your device is not runtime suspended during system suspend.
> + return 0;
> +}
> +
> +static int omap_des_resume(struct device *dev)
> +{
> + pm_runtime_get_sync(dev);
Same here.
Btw, has the omap-aes or this driver been tested with system
suspend on any platfoms?
> + return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops omap_des_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(omap_des_suspend, omap_des_resume)
> +};
> +
> +static struct platform_driver omap_des_driver = {
> + .probe = omap_des_probe,
> + .remove = omap_des_remove,
> + .driver = {
> + .name = "omap-des",
> + .owner = THIS_MODULE,
> + .pm = &omap_des_pm_ops,
> + .of_match_table = omap_des_of_match,
You could use of_match_ptr() here and avoid having the empty
omap_des_of_match defined.
> + },
> +};
> +
> +module_platform_driver(omap_des_driver);
> +
> +MODULE_DESCRIPTION("OMAP DES hw acceleration support.");
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("Joel Fernandes <joelf@ti.com>");
>
^ permalink raw reply
* Re: [PATCH 1/3] crypto: omap-des: Add omap-des driver for OMAP4/AM43xx
From: Joel Fernandes @ 2013-08-29 23:31 UTC (permalink / raw)
To: Joel Fernandes
Cc: Herbert Xu, David S. Miller, Mark Greer, Tony Lindgren,
Lokesh Vutla, Linux OMAP List, Linux ARM Kernel List,
Linux Kernel Mailing List, Linux Crypto Mailing List
In-Reply-To: <1377818873-21174-2-git-send-email-joelf@ti.com>
On 08/29/2013 06:27 PM, Joel Fernandes wrote:
> Add omap-des driver with platform data for OMAP4. Support added for DES
> ECB and CBC modes.
>
> Where possible, code is reused from omap-aes driver with changes made for
> adjusting key size, block size, removing non-existent encryption modes
> and adding support for OMAP4 platform data and offsets.
>
> Tests have been conducted with the CRYPTO test manager, and functionality
> is verified at different page length alignments.
>
> Signed-off-by: Joel Fernandes <joelf@ti.com>
> ---
> drivers/crypto/omap-des.c | 1192 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 1192 insertions(+)
> create mode 100644 drivers/crypto/omap-des.c
>
> diff --git a/drivers/crypto/omap-des.c b/drivers/crypto/omap-des.c
> new file mode 100644
> index 0000000..6a9a25f
> --- /dev/null
> +++ b/drivers/crypto/omap-des.c
> @@ -0,0 +1,1192 @@
> +/*
> + * Cryptographic API.
> + *
> + * Support for OMAP DES and Triple DES HW acceleration.
> + *
> + * Copyright (c) 2012 Texas Instruments Incorporated
> + * Author: Joel Fernandes <joelf@ti.com>
Have to change this to 2013. Will do so if there are no other comments.
Regards,
-Joel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox