[PATCH] crypto: dont return PTR_ERR() of wrong pointer
Fix a PTR_ERR() return of the wrong pointer Signed-off-by: Roel Kluin roel.kl...@gmail.com --- drivers/crypto/mv_cesa.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c index 7d279e5..c99305a 100644 --- a/drivers/crypto/mv_cesa.c +++ b/drivers/crypto/mv_cesa.c @@ -857,7 +857,7 @@ static int mv_cra_hash_init(struct crypto_tfm *tfm, const char *base_hash_name, printk(KERN_WARNING MV_CESA Base driver '%s' could not be loaded!\n, base_hash_name); - err = PTR_ERR(fallback_tfm); + err = PTR_ERR(base_hash); goto err_bad_base; } } -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] compiler: prevent dead store elimination
Due to optimization a call to memset() may be removed as a dead store when
the buffer is not used after its value is overwritten. The new function
secure_bzero() ensures a section of memory is padded with zeroes.
To prevent a memset() from being eliminated, the compiler must belive
that the memory area is used after the memset(). Also, every byte in
the memory area must be used. If only the first byte is used, via e.g.
asm( :: m(*(char*)p)), then the compiler _will_ skip scrubbing
bytes beyond the first.
The trick is to define a local type of the same size as the memory
area, and use it for the m operand in a dummy asm():
static inline void fake_memory_use(void *p, size_t n)
{
struct __scrub { char c[n]; }
asm( : : m(*(struct __scrub *)p));
}
This looks strange, n being a variable, but has been confirmed to
work with gcc-3.2.3 up to gcc-4.4.3.
Many thanks to Mikael Pettersson and Andi Kleen.
Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
I used a trick to prevent the assembly for Itanium, defining
fake_memory_use in instrinsics.h and checking its definition
in string.c, but maybe there's a better way to do this?
To provide a memory clobber for Itanium it appears we could use
the pragma directive to set the optimization level [1]. Only when
optimization level is greater than 3, dead store removal occurs[2].
But this will be ugly:
inline void secure_bzero(void *p, size_t n)
{
#ifdef _ASM_IA64_INTRINSICS_H
#pragma OPT_LEVEL 2
memset(p, 0, n);
#pragma OPT_LEVEL INITIAL
#else
memset(p, 0, n);
fake_memory_use(p, n);
#endif /* _ASM_IA64_INTRINSICS_H */
}
Do we want this?
Thanks, Roel
[1] http://docs.hp.com/en/B3901-90030/ch03s04.html
[2]
http://h21007.www2.hp.com/portal/download/files/unprot/Itanium/OptimizingApps-ItaniumV9-1.pdf
(page 6)
arch/ia64/include/asm/intrinsics.h |3 +++
include/linux/string.h |1 +
lib/string.c | 27 +++
3 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/ia64/include/asm/intrinsics.h
b/arch/ia64/include/asm/intrinsics.h
index 111ed52..51d8035 100644
--- a/arch/ia64/include/asm/intrinsics.h
+++ b/arch/ia64/include/asm/intrinsics.h
@@ -241,6 +241,9 @@ extern long ia64_cmpxchg_called_with_bad_pointer (void);
IA64_INTRINSIC_API(intrin_local_irq_restore)
#define ia64_set_rr0_to_rr4IA64_INTRINSIC_API(set_rr0_to_rr4)
+/* FIXME: define fake_memory_use as a memory clobber for Itanium */
+#define fake_memory_use
+
#endif /* !__ASSEMBLY__ */
#endif /* _ASM_IA64_INTRINSICS_H */
diff --git a/include/linux/string.h b/include/linux/string.h
index a716ee2..be68efb 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -118,6 +118,7 @@ extern void * memchr(const void *,int,__kernel_size_t);
extern char *kstrdup(const char *s, gfp_t gfp);
extern char *kstrndup(const char *s, size_t len, gfp_t gfp);
extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
+extern void secure_bzero(void *, size_t);
extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
extern void argv_free(char **argv);
diff --git a/lib/string.c b/lib/string.c
index a1cdcfc..cbc32d4 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -559,6 +559,33 @@ void *memset(void *s, int c, size_t count)
EXPORT_SYMBOL(memset);
#endif
+#ifndef fake_memory_use
+/*
+ * Dead store elimination is an optimization that may remove a write to a
+ * buffer that is not used anymore. To prevent this, e.g. for security
+ * reasons, the compiler must believe that every byte in this memory area
+ * is used after the last write.
+ */
+static inline void fake_memory_use(void *p, size_t n)
+{
+ struct __scrub { char c[n]; }
+ asm( : : m(*(struct __scrub *)p));
+}
+#endif /* fake_memory_use */
+
+/**
+ * secure_bzero - Call memset to fill a region of memory with zeroes and
+ * ensure this memset is not removed due to dead store elimination.
+ * @p: Pointer to the start of the area.
+ * @n: The size of the area.
+ */
+inline void secure_bzero(void *p, size_t n)
+{
+ memset(p, 0, n);
+ fake_memory_use(p, n);
+}
+EXPORT_SYMBOL(secure_bzero);
+
#ifndef __HAVE_ARCH_MEMCPY
/**
* memcpy - Copy one area of memory to another
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] compiler: prevent dead store elimination
Due to optimization A call to memset() may be removed as a dead store when
the buffer is not used after its value is overwritten. The new function
secure_bzero() ensures a section of memory is padded with zeroes.
From the GCC manual, section 5.37:
If your assembler instructions access memory in an unpredictable
fashion, add `memory' to the list of clobbered registers. This will
cause GCC to not keep memory values cached in registers across the
assembler instruction and not optimize stores or loads to that memory.
Every byte in the [p,p+n[ range must be used. If you only use the
first byte, via e.g. asm( :: m(*(char*)p)), then the compiler
_will_ skip scrubbing bytes beyond the first. This works with
gcc-3.2.3 up to gcc-4.4.3.
Thanks to Mikael Pettersson for this information.
Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
You forgot to credit Mikael
Not intended, I first put it in the changelog but then decided to thank all
involved in my comments. Do credits belong in the changelog?
+#define ARRAY_PREVENT_DSE(p, n) \
Maybe it's just me, but the name is ugly.
Ok, changed it to ARRAY_KEEP_STORE() or do you have a suggestion?
+do {\
+struct __scrub { char c[n]; }; \
Better typeof(*p)[n]
I guess you meant `struct __scrub { typeof(*p) c[n]; };' ?
In the current implementation p is a void pointer. Is it ok to declare an array
of voids? In kernel it compiles, in my testcase it produces an error.
+++ b/include/linux/compiler-intel.h
+#define ARRAY_PREVENT_DSE(p, n)
Who says the Intel compiler doesn't need this?
There was a comment in include/linux/compiler-intel.h that it's not supported.
I'm sure it does dead store elimination too and it understands
gcc asm syntax.
Ok, should I put it in include/linux/compiler.h or where?
+void secure_bzero(void *p, size_t n)
+{
+memset(p, 0, n);
+ARRAY_PREVENT_DSE(p, n);
I think that's a candidate for a inline
You mean the secure_bzero() function, right?
Thanks for comments, Roel
include/linux/compiler.h | 11 +++
include/linux/string.h |1 +
lib/string.c | 13 +
3 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 188fcae..2f14199 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -302,4 +302,15 @@ void ftrace_likely_update(struct ftrace_branch_data *f,
int val, int expect);
*/
#define ACCESS_ONCE(x) (*(volatile typeof(x) *)(x))
+/*
+ * Dead store elimination is an optimization that may remove a write to a
+ * buffer that is not used anymore. Use ARRAY_KEEP_STORE after a write when
+ * the scrub is required for security reasons.
+ */
+#define ARRAY_KEEP_STORE(p, n) \
+ do {\
+ struct __scrub { typeof(*p) c[n]; };\
+ asm( : : m(*(struct __scrub *)p)); \
+ } while (0)
+
#endif /* __LINUX_COMPILER_H */
diff --git a/include/linux/string.h b/include/linux/string.h
index a716ee2..36213e6 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -118,6 +118,7 @@ extern void * memchr(const void *,int,__kernel_size_t);
extern char *kstrdup(const char *s, gfp_t gfp);
extern char *kstrndup(const char *s, size_t len, gfp_t gfp);
extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
+extern void secure_bzero(void *, __kernel_size_t);
extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
extern void argv_free(char **argv);
diff --git a/lib/string.c b/lib/string.c
index a1cdcfc..5576161 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -559,6 +559,19 @@ void *memset(void *s, int c, size_t count)
EXPORT_SYMBOL(memset);
#endif
+/**
+ * secure_bzero - Call memset to fill a region of memory with zeroes and
+ * ensure this memset is not removed due to dead store elimination.
+ * @p: Pointer to the start of the area.
+ * @n: The size of the area.
+ */
+inline void secure_bzero(void *p, size_t n)
+{
+ memset(p, 0, n);
+ ARRAY_KEEP_STORE(p, n);
+}
+EXPORT_SYMBOL(secure_bzero);
+
#ifndef __HAVE_ARCH_MEMCPY
/**
* memcpy - Copy one area of memory to another
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v1] compiler: prevent dead store elimination
Due to optimization A call to memset() may be removed as a dead store when
the buffer is not used after its value is overwritten. The new function
secure_bzero() ensures a section of memory is padded with zeroes.
From the GCC manual, section 5.37:
If your assembler instructions access memory in an unpredictable
fashion, add `memory' to the list of clobbered registers. This will
cause GCC to not keep memory values cached in registers across the
assembler instruction and not optimize stores or loads to that memory.
Every byte in the [p,p+n[ range must be used. If you only use the
first byte, via e.g. asm( :: m(*(char*)p)), then the compiler
_will_ skip scrubbing bytes beyond the first. This works with
gcc-3.2.3 up to gcc-4.4.3.
Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
include/linux/compiler-gcc.h | 11 +++
include/linux/compiler-intel.h |2 ++
include/linux/string.h |1 +
lib/string.c | 13 +
4 files changed, 27 insertions(+), 0 deletions(-)
Thanks all for the required information, checkpatch.pl and compile
tested. In my (non-kernel) testcase this prevents dead store elimination.
Comments?
Roel
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 73dcf80..0799938 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -12,6 +12,17 @@
#define barrier() __asm__ __volatile__(: : :memory)
/*
+ * Dead store elimination (DSE) is an optimization that may remove a write to
+ * a buffer that is not used anymore. Use ARRAY_PREVENT_DSE after a write when
+ * the scrub is required for security reasons.
+ */
+#define ARRAY_PREVENT_DSE(p, n)\
+ do {\
+ struct __scrub { char c[n]; }; \
+ asm( : : m(*(struct __scrub *)p)); \
+ } while (0)
+
+/*
* This macro obfuscates arithmetic on a variable address so that gcc
* shouldn't recognize the original var, and make assumptions about it.
*
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index d8e636e..e8e11f3 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -14,9 +14,11 @@
* It uses intrinsics to do the equivalent things.
*/
#undef barrier
+#undef ARRAY_PREVENT_DSE
#undef RELOC_HIDE
#define barrier() __memory_barrier()
+#define ARRAY_PREVENT_DSE(p, n)
#define RELOC_HIDE(ptr, off) \
({ unsigned long __ptr; \
diff --git a/include/linux/string.h b/include/linux/string.h
index a716ee2..36213e6 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -118,6 +118,7 @@ extern void * memchr(const void *,int,__kernel_size_t);
extern char *kstrdup(const char *s, gfp_t gfp);
extern char *kstrndup(const char *s, size_t len, gfp_t gfp);
extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
+extern void secure_bzero(void *, __kernel_size_t);
extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
extern void argv_free(char **argv);
diff --git a/lib/string.c b/lib/string.c
index a1cdcfc..588ac31 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -559,6 +559,19 @@ void *memset(void *s, int c, size_t count)
EXPORT_SYMBOL(memset);
#endif
+/**
+ * secure_bzero - Call memset to fill a region of memory with zeroes and
+ * ensure this memset is not removed due to dead store elimination.
+ * @p: Pointer to the start of the area.
+ * @n: The size of the area.
+ */
+void secure_bzero(void *p, size_t n)
+{
+ memset(p, 0, n);
+ ARRAY_PREVENT_DSE(p, n);
+}
+EXPORT_SYMBOL(secure_bzero);
+
#ifndef __HAVE_ARCH_MEMCPY
/**
* memcpy - Copy one area of memory to another
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sha: prevent removal of memset as dead store in sha1_update()
Also from that document:
If you know how large the accessed memory is, you can add it as input or
output but if this is not known, you should add memory. As an example, if
you access ten bytes of a string, you can use a memory input like:
{m( ({ struct { char x[10]; } *p = (void *)ptr ; *p; }) )}.
does this mean we could use something like:
#define SECURE_BZERO(x) do {\
memset(x, 0, sizeof(x));\
asm( : :m( ({ struct { char __y[ARRAY_SIZE(x)]; } *__z =\
(void *)x ; *__z; }) ));\
} while(0)
or rather for not just char arrays:
#define SECURE_BZERO(x) do {\
memset(x, 0, sizeof(x));\
asm( :: m ( ({ \
struct {\
typeof(x[0]) __y[ARRAY_SIZE(x)];\
} *__z = (void *)x; \
*__z; \
}) )); \
} while(0)
This appears to work in my testcase:
---
#include stdio.h
#include string.h
#include stdlib.h
#define SECURE 1
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
#define SECURE_BZERO(x) do {\
memset(x, 0, sizeof(x));\
asm( :: m ( ({ \
struct {\
typeof(x[0]) __y[ARRAY_SIZE(x)];\
} *__z = (void *)x; \
*__z; \
}) )); \
} while(0)
void foo()
{
char password[] = secret;
password[0]='S';
printf (Don't show again: %s\n, password);
#if SECURE == 1
SECURE_BZERO(password);
#else
memset(password, 0, sizeof(password));
#endif
}
void foo1()
{
int nrs[] = {1,1,2,3,4,5,6,7};
nrs[0] = 0;
int i = 8;
printf (Don't show again:\n);
while (i--)
printf (%u\n, nrs[i]);
#if SECURE == 1
SECURE_BZERO(nrs);
#else
memset(nrs, 0, sizeof(nrs));
#endif
}
int main(int argc, char* argv[])
{
foo();
int i;
char foo3[] = ;
char* bar = foo3[0];
for (i = -50; i 50; i++)
printf (%c., bar[i]);
printf(\n\n);
foo1();
int foo4 = 20;
int* ber = foo4;
for (i = -50; i 50; i++)
printf (%u_, ber[i]);
printf(\n);
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Is kernel optimized with dead store removal?
According to http://cwe.mitre.org/data/slices/2000.html#14 due to optimization A call to memset() can be removed as a dead store when the buffer is not used after its value is overwritten. Does this optimization also occur during compilation of the Linux kernel? Then I think I may have found some vulnerabilities. One is sha1_update() where memset(temp, 0, sizeof(temp)); may be removed. Roel -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] geode: Fix cip/blk confusion
a crypto_cipher cip member was set where a crypto_cipher blk members
should have been.
Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
I think my previous patch missed one
blk to cip conversion in geode_setkey_cip():
Please send an incremental patch. Thanks!
Ok, here's the missing part
diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 03e71b1..c7a5a43 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -141,7 +141,7 @@ static int geode_setkey_cip(struct crypto_tfm *tfm, const
u8 *key,
ret = crypto_cipher_setkey(op-fallback.cip, key, len);
if (ret) {
tfm-crt_flags = ~CRYPTO_TFM_RES_MASK;
- tfm-crt_flags |= (op-fallback.blk-base.crt_flags
CRYPTO_TFM_RES_MASK);
+ tfm-crt_flags |= (op-fallback.cip-base.crt_flags
CRYPTO_TFM_RES_MASK);
}
return ret;
}
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] geode: Fix cip/blk confusion
crypto_cipher cip members were set where crypto_cipher blk members should
have been, also the PTR_ERR of the wrong pointer was returned.
Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
This was already discussed in december/januari but I still cannot find it in
mainline, was it lost?
diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
index 6be4503..58f4673 100644
--- a/arch/s390/crypto/aes_s390.c
+++ b/arch/s390/crypto/aes_s390.c
@@ -78,14 +78,14 @@ static int setkey_fallback_cip(struct crypto_tfm *tfm,
const u8 *in_key,
struct s390_aes_ctx *sctx = crypto_tfm_ctx(tfm);
int ret;
- sctx-fallback.blk-base.crt_flags = ~CRYPTO_TFM_REQ_MASK;
- sctx-fallback.blk-base.crt_flags |= (tfm-crt_flags
+ sctx-fallback.cip-base.crt_flags = ~CRYPTO_TFM_REQ_MASK;
+ sctx-fallback.cip-base.crt_flags |= (tfm-crt_flags
CRYPTO_TFM_REQ_MASK);
ret = crypto_cipher_setkey(sctx-fallback.cip, in_key, key_len);
if (ret) {
tfm-crt_flags = ~CRYPTO_TFM_RES_MASK;
- tfm-crt_flags |= (sctx-fallback.blk-base.crt_flags
+ tfm-crt_flags |= (sctx-fallback.cip-base.crt_flags
CRYPTO_TFM_RES_MASK);
}
return ret;
diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 4801162..c7a5a43 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -135,13 +135,13 @@ static int geode_setkey_cip(struct crypto_tfm *tfm, const
u8 *key,
/*
* The requested key size is not supported by HW, do a fallback
*/
- op-fallback.blk-base.crt_flags = ~CRYPTO_TFM_REQ_MASK;
- op-fallback.blk-base.crt_flags |= (tfm-crt_flags
CRYPTO_TFM_REQ_MASK);
+ op-fallback.cip-base.crt_flags = ~CRYPTO_TFM_REQ_MASK;
+ op-fallback.cip-base.crt_flags |= (tfm-crt_flags
CRYPTO_TFM_REQ_MASK);
ret = crypto_cipher_setkey(op-fallback.cip, key, len);
if (ret) {
tfm-crt_flags = ~CRYPTO_TFM_RES_MASK;
- tfm-crt_flags |= (op-fallback.blk-base.crt_flags
CRYPTO_TFM_RES_MASK);
+ tfm-crt_flags |= (op-fallback.cip-base.crt_flags
CRYPTO_TFM_RES_MASK);
}
return ret;
}
@@ -263,7 +263,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm)
if (IS_ERR(op-fallback.cip)) {
printk(KERN_ERR Error allocating fallback algo %s\n, name);
- return PTR_ERR(op-fallback.blk);
+ return PTR_ERR(op-fallback.cip);
}
return 0;
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] geode: Fix cip/blk confusion
On Fri, Jan 29, 2010 at 3:51 PM, Sebastian Andrzej Siewior
sebast...@breakpoint.cc wrote:
* Roel Kluin | 2010-01-29 14:32:56 [+0100]:
This was already discussed in december/januari but I still cannot find it in
mainline, was it lost?
Isn't this patch [0] and [1] in Herbert's tree? If so Herbert is
probably going to merge in the next merge window because it is not
urgend enough.
[0]
http://git.kernel.org/?p=linux/kernel/git/herbert/cryptodev-2.6.git;a=commit;h=faad98f29606d9d3c6bddae7c88693be37d2fb43
[1]
http://git.kernel.org/?p=linux/kernel/git/herbert/cryptodev-2.6.git;a=commit;h=d7ac769068df87ca8c7f72d99cf67ead16739f18
Yes that are the patches, however, I think my previous patch missed one
blk to cip conversion in geode_setkey_cip():
ret = crypto_cipher_setkey(op-fallback.cip, key, len);
if (ret) {
tfm-crt_flags = ~CRYPTO_TFM_RES_MASK;
- tfm-crt_flags |= (op-fallback.blk-base.crt_flags
CRYPTO_TFM_RES_MASK);
+ tfm-crt_flags |= (op-fallback.cip-base.crt_flags
CRYPTO_TFM_RES_MASK);
}
return ret;
}
Roel
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] geode: PTR_ERR return of wrong pointer in fallback_init_cip()
Return the PTR_ERR of the correct pointer.
Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
drivers/crypto/geode-aes.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 4801162..12cf864 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -263,7 +263,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm)
if (IS_ERR(op-fallback.cip)) {
printk(KERN_ERR Error allocating fallback algo %s\n, name);
- return PTR_ERR(op-fallback.blk);
+ return PTR_ERR(op-fallback.cip);
}
return 0;
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] geode: PTR_ERR return of wrong pointer in fallback_init_cip()
Return the PTR_ERR of the correct pointer.
Signed-off-by: Roel Kluin roel.kl...@gmail.com
---
This is correct however you missed one spot a few lines above that one.
Sergey Mironov sent a patch a while ago unfortunatelly a mangled one and
he hasn't resent it yet.
Could you please look at [0] for the missing spot? If you had fun fixing
that one, I've a made a similar mistake in s390's driver [1]. If not
please say so.
Yes, I found that one too, although I sent a patch in a seperate thread.
You can ignore that and ack this if you like it.
I did not find the additional mistakes, though, so a new patch below.
[0] http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg03883.html
[1] http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg03888.html
Sebastian
Thanks,
Roel
arch/s390/crypto/aes_s390.c |8
drivers/crypto/geode-aes.c |6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
index 6118890..58f4673 100644
--- a/arch/s390/crypto/aes_s390.c
+++ b/arch/s390/crypto/aes_s390.c
@@ -78,14 +78,14 @@ static int setkey_fallback_cip(struct crypto_tfm *tfm,
const u8 *in_key,
struct s390_aes_ctx *sctx = crypto_tfm_ctx(tfm);
int ret;
- sctx-fallback.blk-base.crt_flags = ~CRYPTO_TFM_REQ_MASK;
- sctx-fallback.blk-base.crt_flags |= (tfm-crt_flags
+ sctx-fallback.cip-base.crt_flags = ~CRYPTO_TFM_REQ_MASK;
+ sctx-fallback.cip-base.crt_flags |= (tfm-crt_flags
CRYPTO_TFM_REQ_MASK);
ret = crypto_cipher_setkey(sctx-fallback.cip, in_key, key_len);
if (ret) {
tfm-crt_flags = ~CRYPTO_TFM_RES_MASK;
- tfm-crt_flags |= (sctx-fallback.blk-base.crt_flags
+ tfm-crt_flags |= (sctx-fallback.cip-base.crt_flags
CRYPTO_TFM_RES_MASK);
}
return ret;
@@ -174,7 +174,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm)
if (IS_ERR(sctx-fallback.cip)) {
pr_err(Allocating AES fallback algorithm %s failed\n,
name);
- return PTR_ERR(sctx-fallback.blk);
+ return PTR_ERR(sctx-fallback.cip);
}
return 0;
diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 4801162..03e71b1 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -135,8 +135,8 @@ static int geode_setkey_cip(struct crypto_tfm *tfm, const
u8 *key,
/*
* The requested key size is not supported by HW, do a fallback
*/
- op-fallback.blk-base.crt_flags = ~CRYPTO_TFM_REQ_MASK;
- op-fallback.blk-base.crt_flags |= (tfm-crt_flags
CRYPTO_TFM_REQ_MASK);
+ op-fallback.cip-base.crt_flags = ~CRYPTO_TFM_REQ_MASK;
+ op-fallback.cip-base.crt_flags |= (tfm-crt_flags
CRYPTO_TFM_REQ_MASK);
ret = crypto_cipher_setkey(op-fallback.cip, key, len);
if (ret) {
@@ -263,7 +263,7 @@ static int fallback_init_cip(struct crypto_tfm *tfm)
if (IS_ERR(op-fallback.cip)) {
printk(KERN_ERR Error allocating fallback algo %s\n, name);
- return PTR_ERR(op-fallback.blk);
+ return PTR_ERR(op-fallback.cip);
}
return 0;
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: Fix test in get_prng_bytes()
Op 12-10-09 16:07, Herbert Xu schreef: On Mon, Oct 12, 2009 at 09:51:42AM -0400, Neil Horman wrote: . Or should this test be removed? diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c index 3aa6e38..9162456 100644 --- a/crypto/ansi_cprng.c +++ b/crypto/ansi_cprng.c @@ -192,7 +192,7 @@ static int get_prng_bytes(char *buf, size_t nbytes, struct prng_context *ctx) int err; - if (nbytes 0) + if ((ssize_t)nbytes 0) return -EINVAL; spin_lock_bh(ctx-prng_lock); No, you're quite right, its a harmless, but unneeded check. Herbert, could you pull this into cryptodev please? Thank you. Hmm, if it's unneeded why don't we just kill it instead? In that case: --8--8- size_t nbytes cannot be less than 0 and the test was redundant. Acked-by: Neil Horman nhor...@tuxdriver.com Signed-off-by: Roel Kluin roel.kl...@gmail.com --- diff --git a/crypto/ansi_cprng.c b/crypto/ansi_cprng.c index 3aa6e38..47995ae 100644 --- a/crypto/ansi_cprng.c +++ b/crypto/ansi_cprng.c @@ -192,9 +192,6 @@ static int get_prng_bytes(char *buf, size_t nbytes, struct prng_context *ctx) int err; - if (nbytes 0) - return -EINVAL; - spin_lock_bh(ctx-prng_lock); err = -EINVAL; -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
