Re: Backport e666d4e9ceec crypto: vmx - Use skcipher for ctr fallback
On Fri, Aug 24, 2018 at 09:51:41AM +0200, Petr Vorel wrote: > Hi Herbert, > > > On Thu, Aug 23, 2018 at 05:31:01PM +0200, Petr Vorel wrote: > > > Hi, > > > > I wonder, it it makes sense to backport commit > > > e666d4e9ceec crypto: vmx - Use skcipher for ctr fallback > > > to v 4.14 stable kernel. > > > I'm using it in 4.12+. > > > > These commits (somehow similar) has been backported to 4.10.2: > > > 5839f555fa57 crypto: vmx - Use skcipher for xts fallback > > > c96d0a1c47ab crypto: vmx - Use skcipher for cbc fallback > > > I think so. Here is the patch which should be applied for all > > kernels since 4.10 up to and including 4.14: > > Thanks a lot for confirmation and preparing the patch! The patch didn't apply, but I fixed it :) Now queued up. greg k-h
[PATCH 3/4] crc-t10dif: Allow current transform to be inspected in sysfs
Add a way to print the currently active CRC algorithm in:
/sys/module/crc_t10dif/parameters/transform
Signed-off-by: Martin K. Petersen
---
lib/crc-t10dif.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c
index 72076a902df5..21c9b35a656f 100644
--- a/lib/crc-t10dif.c
+++ b/lib/crc-t10dif.c
@@ -107,6 +107,17 @@ static void __exit crc_t10dif_mod_fini(void)
module_init(crc_t10dif_mod_init);
module_exit(crc_t10dif_mod_fini);
+static int crc_t10dif_transform_show(char *buffer, const struct kernel_param
*kp)
+{
+ if (static_key_false(&crct10dif_fallback))
+ return sprintf(buffer, "fallback\n");
+
+ return sprintf(buffer, "%s\n",
+ crypto_tfm_alg_driver_name(crypto_shash_tfm(crct10dif_tfm)));
+}
+
+module_param_call(transform, NULL, crc_t10dif_transform_show, NULL, 0644);
+
MODULE_DESCRIPTION("T10 DIF CRC calculation");
MODULE_LICENSE("GPL");
MODULE_SOFTDEP("pre: crct10dif");
--
2.17.1
[PATCH 4/4] block: Integrity profile init function to trigger module loads
The T10 CRC library function is built into the kernel and therefore
registered early. The hardware-accelerated CRC helpers are typically
loaded as modules and only become available later in the boot
sequence. A separate patch modifies the T10 CRC library to subscribe
to notifications from crypto and permits switching from the
table-based algorithm to a hardware accelerated ditto once the
relevant module is loaded.
However, since the dependency for "crc10dif" is already satisfied,
nothing is going to cause the hardware-accelerated kernel modules to
get loaded. Introduce an init_fn in the integrity profile that can be
called to trigger a load of modules providing the T10 CRC calculation
capability. This function will ony get called when a new integrity
profile is registered during device discovery.
Signed-off-by: Martin K. Petersen
---
block/blk-integrity.c | 5 +
block/t10-pi.c | 10 ++
include/linux/blkdev.h | 2 ++
3 files changed, 17 insertions(+)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 6121611e1316..5cacae9a2dc2 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include "blk.h"
@@ -391,6 +392,7 @@ static blk_status_t blk_integrity_nop_fn(struct
blk_integrity_iter *iter)
static const struct blk_integrity_profile nop_profile = {
.name = "nop",
+ .init_fn = NULL,
.generate_fn = blk_integrity_nop_fn,
.verify_fn = blk_integrity_nop_fn,
};
@@ -418,6 +420,9 @@ void blk_integrity_register(struct gendisk *disk, struct
blk_integrity *template
bi->tuple_size = template->tuple_size;
bi->tag_size = template->tag_size;
+ if (bi->profile->init_fn)
+ bi->profile->init_fn();
+
disk->queue->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
}
EXPORT_SYMBOL(blk_integrity_register);
diff --git a/block/t10-pi.c b/block/t10-pi.c
index a98db384048f..b83278f9163a 100644
--- a/block/t10-pi.c
+++ b/block/t10-pi.c
@@ -24,6 +24,7 @@
#include
#include
#include
+#include
#include
typedef __be16 (csum_fn) (void *, unsigned int);
@@ -157,8 +158,14 @@ static blk_status_t t10_pi_type3_verify_ip(struct
blk_integrity_iter *iter)
return t10_pi_verify(iter, t10_pi_ip_fn, 3);
}
+static void t10_pi_crc_init(void)
+{
+ request_module_nowait(CRC_T10DIF_STRING);
+}
+
const struct blk_integrity_profile t10_pi_type1_crc = {
.name = "T10-DIF-TYPE1-CRC",
+ .init_fn= t10_pi_crc_init,
.generate_fn= t10_pi_type1_generate_crc,
.verify_fn = t10_pi_type1_verify_crc,
};
@@ -166,6 +173,7 @@ EXPORT_SYMBOL(t10_pi_type1_crc);
const struct blk_integrity_profile t10_pi_type1_ip = {
.name = "T10-DIF-TYPE1-IP",
+ .init_fn= NULL,
.generate_fn= t10_pi_type1_generate_ip,
.verify_fn = t10_pi_type1_verify_ip,
};
@@ -173,6 +181,7 @@ EXPORT_SYMBOL(t10_pi_type1_ip);
const struct blk_integrity_profile t10_pi_type3_crc = {
.name = "T10-DIF-TYPE3-CRC",
+ .init_fn= t10_pi_crc_init,
.generate_fn= t10_pi_type3_generate_crc,
.verify_fn = t10_pi_type3_verify_crc,
};
@@ -180,6 +189,7 @@ EXPORT_SYMBOL(t10_pi_type3_crc);
const struct blk_integrity_profile t10_pi_type3_ip = {
.name = "T10-DIF-TYPE3-IP",
+ .init_fn= NULL,
.generate_fn= t10_pi_type3_generate_ip,
.verify_fn = t10_pi_type3_verify_ip,
};
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 79226ca8f80f..a43c02e4f43d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1806,10 +1806,12 @@ struct blk_integrity_iter {
};
typedef blk_status_t (integrity_processing_fn) (struct blk_integrity_iter *);
+typedef void (integrity_init_fn) (void);
struct blk_integrity_profile {
integrity_processing_fn *generate_fn;
integrity_processing_fn *verify_fn;
+ integrity_init_fn *init_fn;
const char *name;
};
--
2.17.1
[PATCH 2/4] crc-t10dif: Pick better transform if one becomes available
T10 CRC library is linked into the kernel thanks to block and SCSI. The
crypto accelerators are typically loaded later as modules and are
therefore not available when the T10 CRC library is initialized.
Use the crypto notifier facility to trigger a switch to a better algorithm
if one becomes available after the initial hash has been registered. Use
RCU to protect the original transform while the new one is being set up.
Suggested-by: Ard Biesheuvel
Signed-off-by: Martin K. Petersen
---
include/linux/crc-t10dif.h | 1 +
lib/crc-t10dif.c | 46 --
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/include/linux/crc-t10dif.h b/include/linux/crc-t10dif.h
index 1fe0cfcdea30..6bb0c0bf357b 100644
--- a/include/linux/crc-t10dif.h
+++ b/include/linux/crc-t10dif.h
@@ -6,6 +6,7 @@
#define CRC_T10DIF_DIGEST_SIZE 2
#define CRC_T10DIF_BLOCK_SIZE 1
+#define CRC_T10DIF_STRING "crct10dif"
extern __u16 crc_t10dif_generic(__u16 crc, const unsigned char *buffer,
size_t len);
diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c
index 1ad33e555805..72076a902df5 100644
--- a/lib/crc-t10dif.c
+++ b/lib/crc-t10dif.c
@@ -14,10 +14,47 @@
#include
#include
#include
+#include
#include
+#include
-static struct crypto_shash *crct10dif_tfm;
+static struct crypto_shash __rcu *crct10dif_tfm;
static struct static_key crct10dif_fallback __read_mostly;
+DEFINE_SPINLOCK(crc_t10dif_mutex);
+
+static int crc_t10dif_rehash(struct notifier_block *self, unsigned long val,
void *data)
+{
+ struct crypto_alg *alg = data;
+ struct crypto_shash *new, *old;
+
+ if (val != CRYPTO_MSG_ALG_LOADED ||
+ static_key_false(&crct10dif_fallback) ||
+ strncmp(alg->cra_name, CRC_T10DIF_STRING,
strlen(CRC_T10DIF_STRING)))
+ return 0;
+
+ spin_lock(&crc_t10dif_mutex);
+ old = rcu_dereference_protected(crct10dif_tfm,
+ lockdep_is_held(&crc_t10dif_mutex));
+ if (!old) {
+ spin_unlock(&crc_t10dif_mutex);
+ return 0;
+ }
+ new = crypto_alloc_shash("crct10dif", 0, 0);
+ if (IS_ERR(new)) {
+ spin_unlock(&crc_t10dif_mutex);
+ return 0;
+ }
+ rcu_assign_pointer(crct10dif_tfm, new);
+ spin_unlock(&crc_t10dif_mutex);
+
+ synchronize_rcu();
+ crypto_free_shash(old);
+ return 0;
+}
+
+static struct notifier_block crc_t10dif_nb = {
+ .notifier_call = crc_t10dif_rehash,
+};
__u16 crc_t10dif_update(__u16 crc, const unsigned char *buffer, size_t len)
{
@@ -30,11 +67,14 @@ __u16 crc_t10dif_update(__u16 crc, const unsigned char
*buffer, size_t len)
if (static_key_false(&crct10dif_fallback))
return crc_t10dif_generic(crc, buffer, len);
- desc.shash.tfm = crct10dif_tfm;
+ rcu_read_lock();
+ desc.shash.tfm = rcu_dereference(crct10dif_tfm);
desc.shash.flags = 0;
*(__u16 *)desc.ctx = crc;
err = crypto_shash_update(&desc.shash, buffer, len);
+ rcu_read_unlock();
+
BUG_ON(err);
return *(__u16 *)desc.ctx;
@@ -49,6 +89,7 @@ EXPORT_SYMBOL(crc_t10dif);
static int __init crc_t10dif_mod_init(void)
{
+ crypto_register_notifier(&crc_t10dif_nb);
crct10dif_tfm = crypto_alloc_shash("crct10dif", 0, 0);
if (IS_ERR(crct10dif_tfm)) {
static_key_slow_inc(&crct10dif_fallback);
@@ -59,6 +100,7 @@ static int __init crc_t10dif_mod_init(void)
static void __exit crc_t10dif_mod_fini(void)
{
+ crypto_unregister_notifier(&crc_t10dif_nb);
crypto_free_shash(crct10dif_tfm);
}
--
2.17.1
[PATCH 1/4] crypto: Introduce notifier for new crypto algorithms
Introduce a facility that can be used to receive a notification
callback when a new algorithm becomes available. This can be used by
existing crypto registrations to trigger a switch from a software-only
algorithm to a hardware-accelerated version.
A new CRYPTO_MSG_ALG_LOADED state is introduced to the existing crypto
notification chain, and the register/unregister functions are exported
so they can be called by subsystems outside of crypto.
Signed-off-by: Martin K. Petersen
Suggested-by: Herbert Xu
---
crypto/algapi.c | 7 +++
crypto/algboss.c| 2 ++
crypto/internal.h | 8
include/crypto/algapi.h | 10 ++
4 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/crypto/algapi.c b/crypto/algapi.c
index c0755cf4f53f..87abad3cc322 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -361,15 +361,12 @@ static void crypto_wait_for_test(struct crypto_larval
*larval)
err = crypto_probing_notify(CRYPTO_MSG_ALG_REGISTER, larval->adult);
if (err != NOTIFY_STOP) {
if (WARN_ON(err != NOTIFY_DONE))
- goto out;
+ return;
crypto_alg_tested(larval->alg.cra_driver_name, 0);
}
err = wait_for_completion_killable(&larval->completion);
WARN_ON(err);
-
-out:
- crypto_larval_kill(&larval->alg);
}
int crypto_register_alg(struct crypto_alg *alg)
@@ -390,6 +387,8 @@ int crypto_register_alg(struct crypto_alg *alg)
return PTR_ERR(larval);
crypto_wait_for_test(larval);
+ crypto_probing_notify(CRYPTO_MSG_ALG_LOADED, larval);
+ crypto_larval_kill(&larval->alg);
return 0;
}
EXPORT_SYMBOL_GPL(crypto_register_alg);
diff --git a/crypto/algboss.c b/crypto/algboss.c
index 5e6df2a087fa..527b44d0af21 100644
--- a/crypto/algboss.c
+++ b/crypto/algboss.c
@@ -274,6 +274,8 @@ static int cryptomgr_notify(struct notifier_block *this,
unsigned long msg,
return cryptomgr_schedule_probe(data);
case CRYPTO_MSG_ALG_REGISTER:
return cryptomgr_schedule_test(data);
+ case CRYPTO_MSG_ALG_LOADED:
+ break;
}
return NOTIFY_DONE;
diff --git a/crypto/internal.h b/crypto/internal.h
index 9a3f39939fba..ef769b5e8ad3 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -26,12 +26,6 @@
#include
#include
-/* Crypto notification events. */
-enum {
- CRYPTO_MSG_ALG_REQUEST,
- CRYPTO_MSG_ALG_REGISTER,
-};
-
struct crypto_instance;
struct crypto_template;
@@ -90,8 +84,6 @@ struct crypto_alg *crypto_find_alg(const char *alg_name,
void *crypto_alloc_tfm(const char *alg_name,
const struct crypto_type *frontend, u32 type, u32 mask);
-int crypto_register_notifier(struct notifier_block *nb);
-int crypto_unregister_notifier(struct notifier_block *nb);
int crypto_probing_notify(unsigned long val, void *v);
unsigned int crypto_alg_extsize(struct crypto_alg *alg);
diff --git a/include/crypto/algapi.h b/include/crypto/algapi.h
index bd5e8ccf1687..807501a4a754 100644
--- a/include/crypto/algapi.h
+++ b/include/crypto/algapi.h
@@ -425,4 +425,14 @@ static inline void crypto_yield(u32 flags)
#endif
}
+int crypto_register_notifier(struct notifier_block *nb);
+int crypto_unregister_notifier(struct notifier_block *nb);
+
+/* Crypto notification events. */
+enum {
+ CRYPTO_MSG_ALG_REQUEST,
+ CRYPTO_MSG_ALG_REGISTER,
+ CRYPTO_MSG_ALG_LOADED,
+};
+
#endif /* _CRYPTO_ALGAPI_H */
--
2.17.1
Re: [PATCH] Performance Improvement in CRC16 Calculations.
Herbert, > I don't think this is safe unless you do some kind of locking > which would slow down the data path. The easiest fix would be > to keep the old tfm around forever, or use RCU if RCU read locking > is acceptable to your use-case. You're right. There's a small race there. Patch series coming... -- Martin K. Petersen Oracle Linux Engineering
Re: [PATCH v2 02/17] zinc: introduce minimal cryptography library
On Sat, Aug 25, 2018 at 10:40:06AM -0600, Jason A. Donenfeld wrote: > Hey Eric, > > On Fri, Aug 24, 2018 at 11:29:52PM -0700, Eric Biggers wrote: > > I thought you were going to wrap lines at 80 characters? It's hard to read > > the > > extremely long lines, and they encourage deep nesting. > > I somehow noted this for the WireGuard side of things but assumed I > didn't need to do so for Zinc. Hah, such wishful thinking. I'll have > this wrapped at 80 for v3. Hi Jason The coding style document applies to all code in the tree. Plus some out of tree stuff, like iproute2 if, you need user space patches for that. I've been running checkpatch on just the network parts. But i expect at some point, somebody will run it on the crypto stuff as well, and ask you to fix up some of the errors and warning. There is also a general trend that you won't get in detail human reviews until the automated review tools indicate the code is O.K., Andrew
Re: [PATCH v2 02/17] zinc: introduce minimal cryptography library
Pressed send too fast. On Sat, Aug 25, 2018 at 11:06 AM Jason A. Donenfeld wrote: > - https://www.wireguard.com/papers/wireguard-formal-verification.pdf > - https://www.wireguard.com/papers/dowling-paterson-computational-2018.pdf > - https://www.wireguard.com/formal-verification/ - https://www.wireguard.com/papers/lipp-computational-2018.pdf - https://eprint.iacr.org/2018/766.pdf
Re: [PATCH v2 02/17] zinc: introduce minimal cryptography library
Hello Ard, On Sat, Aug 25, 2018 at 11:17:42AM +0100, Ard Biesheuvel wrote: > Do we really need a new crypto API for WireGuard? Surely, you yourself > are not affected by these concerns, and I don't anticipate an > explosion of new crypto use cases in the kernel that would justify > maintaining two parallel crypto stacks. Yes, we really do. And the new API won't be useful for WireGuard but also for the majority of cryptography users within the kernel, which will gradually be ported to use the new API. > Also, I take it this means that WireGuard will only work with your > routines? We don't usually impose that kind of policy in the kernel: > on my arm64 systems, AES/GCM runs at 2.3 cycles per byte, which is a > good enough reason to prefer it over a ChaCha20/Poly1305 based AEAD, > even if your code is faster than the current code (which is debatable, > as you know) WireGuard is a protocol that specifies ChaPoly, and I am proposing an implementation of that protocol. The protocol explicitly does not specify other ciphers. Please read the whitepaper for extensive discussion and feel free to come on over to wiregu...@lists.zx2c4.com if you really want to hardcore bikeshed on protocol particulars. The short answer, however, is: no WireGuard won't be adding cipher negotiation, and no, this isn't considered a good feature, and no, you won't have any luck changing that. I am certain, though, that pushing this point will be a terrific means of steering this implementation thread far off course, as various folks with their various opinions feel compelled to jump in about their thoughts on cipher negotiation. Yawn. Besides, your claim about "impos[ing] that kind of policy in the kernel" is bogus: with the exception of IPsec, dm-crypt, and AF_ALG, basically all users of cryptography I can find are using a very particular set of ciphers. For example, Bluetooth uses a particular elliptic curve that has been specified by the Bluetooth people, and the kernel therefore implements a driver that does computations over that elliptic curve. Nobody on LKML is clamoring for that driver to also support Curve448 or something, since that's not what Bluetooth specifies or how Bluetooth works. So too, that's not what WireGuard specifies or how WireGuard works. > It also means that users of the crypto API will not benefit from your > better code. I'm sure you agree that it is a bad idea to force those > same crypto-impaired programmers to port their code to a new API. As mentioned, I think we'll certainly be gradually porting existing crypto API users over to the new API where appropriate. As always, this is a gradual thing, and won't ever come all at once in the first patchset, but I'm pretty confident we can get there somewhat quickly, especially as new primitives become available in the new API. > I also suggested that we work with Andy Polyakov (as I have in the > past) to make the changes required for the kernel in the OpenSSL > upstream. That way, we can take the .pl files unmodified, and benefit > from the maintenance and review upstream. Dumping 10,000s of lines of > generated assembler in the kernel tree like that is really > unacceptable IMO. I'm happy to do that, and indeed I do enjoy working with Andy. However, I don't think your characterization of the current situation is at all accurate. Rather, those .S files have been extensively worked with and crafted. They're far from being merely generated, and they've been pretty heavily reviewed. > I'm not sure what the relevance of > formally verified implementations is, though, since it only proves > that the code is true to the algorithm, not that the algorithm is > secure. Or am I missing something? You are indeed missing something. When the code is not true to the algorithm, that is very bad. When the code is true to the algorithm, that is very good. Formally verified implementations intend to increase our confidence in the latter. Meanwhile on the algorithm side of thing, the WireGuard protocol has a number of proofs that the protocol is secure: - https://www.wireguard.com/papers/wireguard-formal-verification.pdf - https://www.wireguard.com/papers/dowling-paterson-computational-2018.pdf - https://www.wireguard.com/formal-verification/ Before you get too excited though, keep in mind that this is a proof of the protocol, and not of each primitive, like, say, "ChaCha20" or "Curve25519." However, academic literature is full of all sorts of curious and fascinating security analyses of the various primitives if this is something you're interested in. Jason
Re: [PATCH v2 02/17] zinc: introduce minimal cryptography library
Hey Eric, On Fri, Aug 24, 2018 at 11:29:52PM -0700, Eric Biggers wrote: > I thought you were going to wrap lines at 80 characters? It's hard to read > the > extremely long lines, and they encourage deep nesting. I somehow noted this for the WireGuard side of things but assumed I didn't need to do so for Zinc. Hah, such wishful thinking. I'll have this wrapped at 80 for v3. > As I said before, I still think you need to switch the crypto API ChaCha20 and > Poly1305 over to use the new implementations. It's not okay to have two > completely different sets of ChaCha20 and Poly1305 implementations just > because > you want a different API, so you might as well get started on it... The thing > is that before you try it, it's not clear what problems will come up that > require changes to the design. So, this really ought to be addressed > up-front. It was my hope that this entire series could enter the tree through Dave's net-next, and that I wouldn't have to touch anything in crypto/ or touch any of Herbert's stuff at all in anyway. However, if you want, I can start to play with that in another branch for a separate patchset, and of course I'd really value your feedback on that and on doing it right. > It's also not clearly explained whether/why/how the new ChaCha20 and Poly1305 > implementations are better than the existing ones. The patch adding the ARM > and > ARM64 ChaCha, for example, just says who wrote them, with no mention of why > the > particular implementations were chosen. I can expand on that, sure. One primary advantage that I do touch on on the big introductory commit message, though, is that sharing code means that we benefit from the eyeballs and fuzzing hours spent on OpenSSL, and I think this general advantage is extremely compelling. > You've also documented a lot of stuff in commit messages which will be lost as > it isn't being added to the source itself. There are various examples of > this, > such as information about where the various implementations came from, what > you > changed, why a particular implementation isn't used on Skylake or whatever, > etc. > Can you please make sure that any important information is in comments, e.g. > at > the top of the files? Good idea. I'll do that for v3. > I still think the "Zinc" name is confusing and misleading, and people are > going > to forget that it means "crypto". lib/crypto/ would be more intuitive. > But I don't care *that* much myself, and you should see what others think... I'd like to keep it. It also enables a natural path for a corresponding userspace library that shares all of the same code, and one that I think will be compelling to attract academics and developers to contributing to these efforts. Plus, can't I name what I made? > It seems you still don't explicitly clarify anywhere in the source itself that > the copyright holders of the code from OpenSSL have relicensed it under GPLv2. > I only see a GPLv2 license slapped on the files, yet no such license is > presence > in the OpenSSL originals, at least in the one I checked. The SPDX headers for those came out of a discussion on how to encode CRYPTOGAMS into SPDX from the SPDX mailing list several months ago. > If you did receive > explicit permission, then you should include an explicit clarification in each > file like the one in arch/arm/crypto/sha1-armv4-large.S. That's a good idea. > As Ard and I discussed recently on my patch > "crypto: arm/poly1305 - add NEON accelerated Poly1305 implementation" > which proposed adding the exact same poly1305-arm.S file, for all the OpenSSL > assembly it would probably be better to include the .pl file and generate the > .S > file as part of the build process. For one, there is semantic information > like > register names in the .pl script that is lost in the .S file, thereby making > the > .S file less readable. But on the other hand, the .S files have been modified and changed to fit kernel constraints and conventions. They're very much no longer merely "generated". This is most apparent with the x86_64 files, but is present too with the ARM files. We're still, I believe, bug-for-bug similar to the originals -- keeping with the point of going with Andy's implementations in the first place -- and we've been able to pretty trivially track OpenSSL changes -- but nonetheless important tweaks have been done to make this suitable to kernel space. > There are still some alignment bugs where integers are loaded from byte arrays > without using the unaligned access macros, e.g. in chacha20_init(), > hchacha20_generic(), and fe_frombytes_impl(). I found these grepping for > le32_to_cpu. Interestingly, that last one is in "formally verified" code :-) Thanks. I'll do another pass at these for v3. Jason
Re: [PATCH v2 02/17] zinc: introduce minimal cryptography library
> It seems you still don't explicitly clarify anywhere in the source itself that > the copyright holders of the code from OpenSSL have relicensed it under GPLv2. > I only see a GPLv2 license slapped on the files, yet no such license is > presence > in the OpenSSL originals, at least in the one I checked. If you did receive > explicit permission, then you should include an explicit clarification in each > file like the one in arch/arm/crypto/sha1-armv4-large.S. Better yet, get the copyright holders to publicly send a signed-off-by: or acked-by: so it is clear they agree to this. Andrew
TRADING ACCOUNT
Dear sir , I KELLY ALAN purchasing and sales manager of CFM INTERNATIONAL .Our Company specialised in Supplying computer hardware and Electronic .We want to extend our supplier list because of concurrency in prices on the international market. We are seeking a supplier with whom we can to have partnered long-term in order to have competitive prices . we are interested to buy the products you sell and want to place an order with you in big quantities. Can you give us payment facilities ( 14 , 30 or 60 days payment terms ) ? What is the procedure for our account opening and credit line application ? Cordially KELLY ALAN CFM INTERNATIONAL 2 BOULEVARD DU GAL MARTIAL VALIN 75015 PARIS REG N° 302 527 700 VAT N° FR90 302527700 TEL +33171025367 FAX +33177759149 https://www.cfmaeroengines.com
Re: [PATCH 1/1] crypto: cavium/nitrox - fix for command corruption in queue full case with backlog submissions.
On Wed, Aug 22, 2018 at 12:40:52PM +0530, Srikanth Jampala wrote: > Earlier used to post the current command without checking queue full > after backlog submissions. So, post the current command only after > confirming the space in queue after backlog submissions. > > Maintain host write index instead of reading device registers > to get the next free slot to post the command. > > Return -ENOSPC in queue full case. > > Signed-off-by: Srikanth Jampala > Reviewed-by: Gadam Sreerama > Tested-by: Jha, Chandan Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2] crypto: vmx - Fix sleep-in-atomic bugs
On Wed, Aug 22, 2018 at 08:26:31AM +0200, Ondrej Mosnacek wrote:
> This patch fixes sleep-in-atomic bugs in AES-CBC and AES-XTS VMX
> implementations. The problem is that the blkcipher_* functions should
> not be called in atomic context.
>
> The bugs can be reproduced via the AF_ALG interface by trying to
> encrypt/decrypt sufficiently large buffers (at least 64 KiB) using the
> VMX implementations of 'cbc(aes)' or 'xts(aes)'. Such operations then
> trigger BUG in crypto_yield():
>
> [ 891.863680] BUG: sleeping function called from invalid context at
> include/crypto/algapi.h:424
> [ 891.864622] in_atomic(): 1, irqs_disabled(): 0, pid: 12347, name: kcapi-enc
> [ 891.864739] 1 lock held by kcapi-enc/12347:
> [ 891.864811] #0: f5d42c46 (sk_lock-AF_ALG){+.+.}, at:
> skcipher_recvmsg+0x50/0x530
> [ 891.865076] CPU: 5 PID: 12347 Comm: kcapi-enc Not tainted
> 4.19.0-0.rc0.git3.1.fc30.ppc64le #1
> [ 891.865251] Call Trace:
> [ 891.865340] [c003387578c0] [c0d67ea4] dump_stack+0xe8/0x164
> (unreliable)
> [ 891.865511] [c00338757910] [c0172a58]
> ___might_sleep+0x2f8/0x310
> [ 891.865679] [c00338757990] [c06bff74]
> blkcipher_walk_done+0x374/0x4a0
> [ 891.865825] [c003387579e0] [d7e73e70]
> p8_aes_cbc_encrypt+0x1c8/0x260 [vmx_crypto]
> [ 891.865993] [c00338757ad0] [c06c0ee0]
> skcipher_encrypt_blkcipher+0x60/0x80
> [ 891.866128] [c00338757b10] [c06ec504]
> skcipher_recvmsg+0x424/0x530
> [ 891.866283] [c00338757bd0] [c0b00654] sock_recvmsg+0x74/0xa0
> [ 891.866403] [c00338757c10] [c0b00f64] ___sys_recvmsg+0xf4/0x2f0
> [ 891.866515] [c00338757d90] [c0b02bb8] __sys_recvmsg+0x68/0xe0
> [ 891.866631] [c00338757e30] [c000bbe4] system_call+0x5c/0x70
>
> Fixes: 8c755ace357c ("crypto: vmx - Adding CBC routines for VMX module")
> Fixes: c07f5d3da643 ("crypto: vmx - Adding support for XTS")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Ondrej Mosnacek
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: aesni - Use unaligned loads from gcm_context_data
On Wed, Aug 15, 2018 at 10:29:42AM -0700, Dave Watson wrote:
> A regression was reported bisecting to 1476db2d12
> "Move HashKey computation from stack to gcm_context". That diff
> moved HashKey computation from the stack, which was explicitly aligned
> in the asm, to a struct provided from the C code, depending on
> AESNI_ALIGN_ATTR for alignment. It appears some compilers may not
> align this struct correctly, resulting in a crash on the movdqa
> instruction when attempting to encrypt or decrypt data.
>
> Fix by using unaligned loads for the HashKeys. On modern
> hardware there is no perf difference between the unaligned and
> aligned loads. All other accesses to gcm_context_data already use
> unaligned loads.
>
> Reported-by: Mauro Rossi
> Fixes: 1476db2d12 ("Move HashKey computation from stack to gcm_context")
> Signed-off-by: Dave Watson
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: arm64/aes-gcm-ce - fix scatterwalk API violation
On Mon, Aug 20, 2018 at 04:58:34PM +0200, Ard Biesheuvel wrote:
> Commit 71e52c278c54 ("crypto: arm64/aes-ce-gcm - operate on
> two input blocks at a time") modified the granularity at which
> the AES/GCM code processes its input to allow subsequent changes
> to be applied that improve performance by using aggregation to
> process multiple input blocks at once.
>
> For this reason, it doubled the algorithm's 'chunksize' property
> to 2 x AES_BLOCK_SIZE, but retained the non-SIMD fallback path that
> processes a single block at a time. In some cases, this violates the
> skcipher scatterwalk API, by calling skcipher_walk_done() with a
> non-zero residue value for a chunk that is expected to be handled
> in its entirety. This results in a WARN_ON() to be hit by the TLS
> self test code, but is likely to break other user cases as well.
> Unfortunately, none of the current test cases exercises this exact
> code path at the moment.
>
> Fixes: 71e52c278c54 ("crypto: arm64/aes-ce-gcm - operate on two ...")
> Reported-by: Vakul Garg
> Signed-off-by: Ard Biesheuvel
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: arm64/sm4-ce - check for the right CPU feature bit
On Tue, Aug 07, 2018 at 11:18:36PM +0200, Ard Biesheuvel wrote: > ARMv8.2 specifies special instructions for the SM3 cryptographic hash > and the SM4 symmetric cipher. While it is unlikely that a core would > implement one and not the other, we should only use SM4 instructions > if the SM4 CPU feature bit is set, and we currently check the SM3 > feature bit instead. So fix that. > > Signed-off-by: Ard Biesheuvel Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] chtls: fix null dereference chtls_free_uld()
On Fri, Aug 10, 2018 at 06:27:41PM +0530, Ganesh Goudar wrote: > call chtls_free_uld() only for the initialized cdev, > this fixes NULL dereference in chtls_free_uld() > > Signed-off-by: Ganesh Goudar > Signed-off-by: Atul Gupta Patch applied. Thanks. -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: caam/qi - fix error path in xts setkey
On Mon, Aug 06, 2018 at 03:29:39PM +0300, Horia Geantă wrote:
> xts setkey callback returns 0 on some error paths.
> Fix this by returning -EINVAL.
>
> Cc: # 4.12+
> Fixes: b189817cf789 ("crypto: caam/qi - add ablkcipher and authenc
> algorithms")
> Signed-off-by: Horia Geantă
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: caam - fix DMA mapping direction for RSA forms 2 & 3
On Mon, Aug 06, 2018 at 03:29:55PM +0300, Horia Geantă wrote:
> Crypto engine needs some temporary locations in external memory for
> running RSA decrypt forms 2 and 3 (CRT).
> These are named "tmp1" and "tmp2" in the PDB.
>
> Update DMA mapping direction of tmp1 and tmp2 from TO_DEVICE to
> BIDIRECTIONAL, since engine needs r/w access.
>
> Cc: # 4.13+
> Fixes: 52e26d77b8b3 ("crypto: caam - add support for RSA key form 2")
> Fixes: 4a651b122adb ("crypto: caam - add support for RSA key form 3")
> Signed-off-by: Horia Geantă
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: caam/jr - fix descriptor DMA unmapping
On Mon, Aug 06, 2018 at 03:29:09PM +0300, Horia Geantă wrote:
> Descriptor address needs to be swapped to CPU endianness before being
> DMA unmapped.
>
> Cc: # 4.8+
> Fixes: 261ea058f016 ("crypto: caam - handle core endianness != caam
> endianness")
> Reported-by: Laurentiu Tudor
> Signed-off-by: Horia Geantă
Patch applied. Thanks.
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] chtls_cm.c: fix missing return value check of alloc_skb()
Jiecheng Wu wrote: > Function chtls_close_conn() defined in > drivers/crypto/chelsio/chtls/chtls_cm.c calls alloc_skb() to allocate memory > for struct sk_buff which is dereferenced immediately. As alloc_skb() may > return NULL on failure, this code piece may cause NULL pointer dereference > bug. Please add a sign-off. You can refer to Documentation/process/submitting-patches.rst Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [RESEND PATCH 2/2] crypto: caam - Support deferred probing in JR dependent drivers
On Tue, Aug 07, 2018 at 12:34:57PM +, Horia Geanta wrote:
> On 8/7/2018 11:00 AM, Marcin Niestroj wrote:
> > It is possible, that caam_jr_alloc() is called before JR devices are
> > probed. Return -EPROBE_DEFER in drivers that rely on JR devices, so
> > they are probed at later stage.
> >
> These drivers don't have a probe() callback.
> Returning -EPROBE_DEFER in module's init() (caamrng) or crypto algorithm's
> init() (caamalg etc.) callbacks is not going to help, they won't be called
> later.
>
> Does adding request_module("caam_jr") in module's init() solve the issue?
If everything is built as a module this should always work because
a module isn't available until after its init function has finished.
So the only problem here appears to be when everything is built-in
in which case the init functions are run in random order.
So perhaps move caam_jr's module_init function to an earlier level
(i.e., earlier than device_initcall)?
Cheers,
--
Email: Herbert Xu
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v2 02/17] zinc: introduce minimal cryptography library
Hi Jason, On 24 August 2018 at 22:38, Jason A. Donenfeld wrote: > Zinc stands for "Zinc Is Neat Crypto" or "Zinc as IN Crypto" or maybe > just "Zx2c4's INsane Cryptolib." It's also short, easy to type, and > plays nicely with the recent trend of naming crypto libraries after > elements. The guiding principle is "don't overdo it". It's less of a > library and more of a directory tree for organizing well-curated direct > implementations of cryptography primitives. > > Zinc is a new cryptography API that is much more minimal and lower-level > than the current one. It intends to complement it and provide a basis > upon which the current crypto API might build, as the provider of > software implementations of cryptographic primitives. It is motivated by > three primary observations in crypto API design: > > * Highly composable "cipher modes" and related abstractions from > 90s cryptographers did not turn out to be as terrific an idea as > hoped, leading to a host of API misuse problems. > > * Most programmers are afraid of crypto code, and so prefer to > integrate it into libraries in a highly abstracted manner, so as to > shield themselves from implementation details. Cryptographers, on > the other hand, prefer simple direct implementations, which they're > able to verify for high assurance and optimize in accordance with > their expertise. > > * Overly abstracted and flexible cryptography APIs lead to a host of > dangerous problems and performance issues. The kernel is in the > business usually not of coming up with new uses of crypto, but > rather implementing various constructions, which means it essentially > needs a library of primitives, not a highly abstracted enterprise-ready > pluggable system, with a few particular exceptions. > Do we really need a new crypto API for WireGuard? Surely, you yourself are not affected by these concerns, and I don't anticipate an explosion of new crypto use cases in the kernel that would justify maintaining two parallel crypto stacks. Also, I take it this means that WireGuard will only work with your routines? We don't usually impose that kind of policy in the kernel: on my arm64 systems, AES/GCM runs at 2.3 cycles per byte, which is a good enough reason to prefer it over a ChaCha20/Poly1305 based AEAD, even if your code is faster than the current code (which is debatable, as you know) It also means that users of the crypto API will not benefit from your better code. I'm sure you agree that it is a bad idea to force those same crypto-impaired programmers to port their code to a new API. I also suggested that we work with Andy Polyakov (as I have in the past) to make the changes required for the kernel in the OpenSSL upstream. That way, we can take the .pl files unmodified, and benefit from the maintenance and review upstream. Dumping 10,000s of lines of generated assembler in the kernel tree like that is really unacceptable IMO. I think you will have to engage with the kernel community to identify the problems with the current crypto API, and get them fixed. I think it makes a lot of sense to have crypto primitives in lib/ (and arch-specific accelerated versions in arch//lib), and layer many of the current crypto API drivers on top of that. Also, having more test cases is useful as well. I'm not sure what the relevance of formally verified implementations is, though, since it only proves that the code is true to the algorithm, not that the algorithm is secure. Or am I missing something? Upstreaming your code is a lot easier if you don't cater for your own needs only but also for the needs of others. Please work with us to fix the problems in the current crypto API before parachuting in a new one. > This last observation has seen itself play out several times over and > over again within the kernel: > > * The perennial move of actual primitives away from crypto/ and into > lib/, so that users can actually call these functions directly with > no overhead and without lots of allocations, function pointers, > string specifier parsing, and general clunkiness. For example: > sha256, chacha20, siphash, sha1, and so forth live in lib/ rather > than in crypto/. Zinc intends to stop the cluttering of lib/ and > introduce these direct primitives into their proper place, lib/zinc/. > > * An abundance of misuse bugs with the present crypto API that have > been very unpleasant to clean up. > > * A hesitance to even use cryptography, because of the overhead and > headaches involved in accessing the routines. > > Zinc goes in a rather different direction. Rather than providing a > thoroughly designed and abstracted API, Zinc gives you simple functions, > which implement some primitive, or some particular and specific > construction of primitives. It is not dynamic in the least, though one > could imagine implementing a complex dynamic dispatch mechanism (such as > the current crypto API
