* Re: [PATCHv3 03/11] crypto: omap-sham: implement context export/import APIs
From: Tero Kristo @ 2016-09-01 7:46 UTC (permalink / raw)
To: Herbert Xu
Cc: lokeshvutla, davem, linux-crypto, tony, linux-omap,
linux-arm-kernel
In-Reply-To: <20160901073113.GA2232@gondor.apana.org.au>
On 01/09/16 10:31, Herbert Xu wrote:
> On Thu, Sep 01, 2016 at 10:28:47AM +0300, Tero Kristo wrote:
>>
>> Yeah, the flush should do the trick now. Kind of a chicken-egg
>> problem here. :P How do you see the situation with the above
>> explanation?
>
> The export function is not allowed to sleep so you must not wait
> for the hardware to complete in it.
>
> If you need to wait, then you must use the completion mechanism.
I don't think export is allowed to return -EINPROGRESS either? At least
currently the kernel pieces using this functionality won't work if I do
that.
If thats the case, I need to think of something else to handle this...
-Tero
^ permalink raw reply
* Re: AF_ALG zero-size hash fails
From: Herbert Xu @ 2016-09-01 9:16 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: noloader, linux-crypto
In-Reply-To: <20160809094525.GA6529@gondor.apana.org.au>
On Tue, Aug 09, 2016 at 05:45:25PM +0800, Herbert Xu wrote:
>
> > tested with the Freescale CAAM driver with SHA1 and MD5 hashes, and
> > the ARM SHA1 shash implementation. Should there at least be a single
> > write to the socket (of zero size) in this case, or should the kernel
> > return the correct hash on the first read without a preceding
> > write/send?
>
> It's an oversight in the algif_hash code. I'll get it fixed.
---8<---
Subject: crypto: algif_hash - Handle NULL hashes correctly
Right now attempting to read an empty hash simply returns zeroed
bytes, this patch corrects this by calling the digest function
using an empty input.
Reported-by: Russell King - ARM Linux <linux@armlinux.org.uk>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 68a5cea..2d8466f 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -39,6 +39,37 @@ struct algif_hash_tfm {
bool has_key;
};
+static int hash_alloc_result(struct sock *sk, struct hash_ctx *ctx)
+{
+ unsigned ds;
+
+ if (ctx->result)
+ return 0;
+
+ ds = crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req));
+
+ ctx->result = sock_kmalloc(sk, ds, GFP_KERNEL);
+ if (!ctx->result)
+ return -ENOMEM;
+
+ memset(ctx->result, 0, ds);
+
+ return 0;
+}
+
+static void hash_free_result(struct sock *sk, struct hash_ctx *ctx)
+{
+ unsigned ds;
+
+ if (!ctx->result)
+ return;
+
+ ds = crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req));
+
+ sock_kzfree_s(sk, ctx->result, ds);
+ ctx->result = NULL;
+}
+
static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
size_t ignored)
{
@@ -54,6 +85,9 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
lock_sock(sk);
if (!ctx->more) {
+ if ((msg->msg_flags & MSG_MORE))
+ hash_free_result(sk, ctx);
+
err = af_alg_wait_for_completion(crypto_ahash_init(&ctx->req),
&ctx->completion);
if (err)
@@ -90,6 +124,10 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
ctx->more = msg->msg_flags & MSG_MORE;
if (!ctx->more) {
+ err = hash_alloc_result(sk, ctx);
+ if (err)
+ goto unlock;
+
ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0);
err = af_alg_wait_for_completion(crypto_ahash_final(&ctx->req),
&ctx->completion);
@@ -116,6 +154,13 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
sg_init_table(ctx->sgl.sg, 1);
sg_set_page(ctx->sgl.sg, page, size, offset);
+ if (!(flags & MSG_MORE)) {
+ err = hash_alloc_result(sk, ctx);
+ if (err)
+ goto unlock;
+ } else if (!ctx->more)
+ hash_free_result(sk, ctx);
+
ahash_request_set_crypt(&ctx->req, ctx->sgl.sg, ctx->result, size);
if (!(flags & MSG_MORE)) {
@@ -153,6 +198,7 @@ static int hash_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
struct alg_sock *ask = alg_sk(sk);
struct hash_ctx *ctx = ask->private;
unsigned ds = crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req));
+ bool result;
int err;
if (len > ds)
@@ -161,17 +207,29 @@ static int hash_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
msg->msg_flags |= MSG_TRUNC;
lock_sock(sk);
+ result = ctx->result;
+ err = hash_alloc_result(sk, ctx);
+ if (err)
+ goto unlock;
+
+ ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0);
+
if (ctx->more) {
ctx->more = 0;
- ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0);
err = af_alg_wait_for_completion(crypto_ahash_final(&ctx->req),
&ctx->completion);
if (err)
goto unlock;
+ } else if (!result) {
+ err = af_alg_wait_for_completion(
+ crypto_ahash_digest(&ctx->req),
+ &ctx->completion);
}
err = memcpy_to_msg(msg, ctx->result, len);
+ hash_free_result(sk, ctx);
+
unlock:
release_sock(sk);
@@ -394,8 +452,7 @@ static void hash_sock_destruct(struct sock *sk)
struct alg_sock *ask = alg_sk(sk);
struct hash_ctx *ctx = ask->private;
- sock_kzfree_s(sk, ctx->result,
- crypto_ahash_digestsize(crypto_ahash_reqtfm(&ctx->req)));
+ hash_free_result(sk, ctx);
sock_kfree_s(sk, ctx, ctx->len);
af_alg_release_parent(sk);
}
@@ -407,20 +464,12 @@ static int hash_accept_parent_nokey(void *private, struct sock *sk)
struct algif_hash_tfm *tfm = private;
struct crypto_ahash *hash = tfm->hash;
unsigned len = sizeof(*ctx) + crypto_ahash_reqsize(hash);
- unsigned ds = crypto_ahash_digestsize(hash);
ctx = sock_kmalloc(sk, len, GFP_KERNEL);
if (!ctx)
return -ENOMEM;
- ctx->result = sock_kmalloc(sk, ds, GFP_KERNEL);
- if (!ctx->result) {
- sock_kfree_s(sk, ctx, len);
- return -ENOMEM;
- }
-
- memset(ctx->result, 0, ds);
-
+ ctx->result = NULL;
ctx->len = len;
ctx->more = 0;
af_alg_init_completion(&ctx->completion);
--
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 related
* Re: [RFC PATCH] crypto: caam - convert from ablkcipher -> skcipher
From: Herbert Xu @ 2016-09-01 10:12 UTC (permalink / raw)
To: Horia Geantă; +Cc: linux-crypto, David S. Miller
In-Reply-To: <1472479884-1887-1-git-send-email-horia.geanta@nxp.com>
On Mon, Aug 29, 2016 at 05:11:24PM +0300, Horia Geantă wrote:
> (a)blkcipher is being deprecated in favcur of skcipher.
> The main difference is that IV generation is moved out
> of crypto algorithms.
>
> Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
Thanks! Please hold on though because I'm not quite done with
the driver-side skcipher interface yet.
> but I am not sure block ciphers with IV generation - for e.g.
> seqiv(rfc3686(ctr(aes))) - make any sense and/or are useful.
This should be removed completely as IV generation is now done
at the AEAD level only.
Cheers,
--
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 1/2] crypto: arm/ghash-ce - add missing async import/export
From: Ard Biesheuvel @ 2016-09-01 13:19 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto@vger.kernel.org
In-Reply-To: <20160831144135.GB26616@gondor.apana.org.au>
On 31 August 2016 at 15:41, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Mon, Aug 29, 2016 at 12:19:53PM +0100, Ard Biesheuvel wrote:
>> Since commit 8996eafdcbad ("crypto: ahash - ensure statesize is non-zero"),
>> all ahash drivers are required to implement import()/export(), and must have
>> a non-zero statesize. Fix this for the ARM Crypto Extensions GHASH
>> implementation.
>>
>> Fixes: 8996eafdcbad ("crypto: ahash - ensure statesize is non-zero")
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> arch/arm/crypto/ghash-ce-glue.c | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/arch/arm/crypto/ghash-ce-glue.c b/arch/arm/crypto/ghash-ce-glue.c
>> index 1568cb5cd870..212aaa715fdb 100644
>> --- a/arch/arm/crypto/ghash-ce-glue.c
>> +++ b/arch/arm/crypto/ghash-ce-glue.c
>> @@ -220,6 +220,29 @@ static int ghash_async_digest(struct ahash_request *req)
>> }
>> }
>>
>> +static int ghash_async_import(struct ahash_request *req, const void *in)
>> +{
>> + struct ahash_request *cryptd_req = ahash_request_ctx(req);
>> + struct shash_desc *desc = cryptd_shash_desc(cryptd_req);
>> + struct ghash_desc_ctx *dctx = shash_desc_ctx(desc);
>> +
>> + ghash_async_init(req);
>
> Is this really needed?
>
Actually, yes, and more. I could not find in the documentation whether
the .import() hook requires .init() to be called first, but based on
the test case in testmgr.c, it appears that is not the case.
This means that the stuff that occurs in .init() to establish the
relation between this desc and its child transform needs to be copied
here as well. In fact, the only way I could make this work is by
adding it to cryptd's import() routines as well, otherwise the test
crashes reliably.
>> + *dctx = *(const struct ghash_desc_ctx *)in;
>
> I'd prefer to call the underlying shash import/export functions
> like we do in cryptd.
>
OK, I can change that
Thanks,
Ard.
^ permalink raw reply
* [PATCH v2 0/3] crypto: arm and cryptd fixes
From: Ard Biesheuvel @ 2016-09-01 13:25 UTC (permalink / raw)
To: linux-crypto, herbert; +Cc: Ard Biesheuvel
Patch #1 fixes a trivial code generation issue on ARM.
Patch #2 and #3 fix the broken GHASH on ARM using the v8 Crypto Extensions
pmull.64 instructions. The problem seems to be that it is allowed to call
.import() without .init() (at least, that is what the test cases do), but
this means that the initialization to tie the shash_desc's to their child
transforms needs to execute in the .import() context as well.
Perhaps Herbert could shed some light on this? Thanks.
In any case, these patches are both necessary (and sufficient) to get GHASH
on ARM working again, including the boot time selftests
Ard Biesheuvel (3):
crypto: arm/sha1-neon - add support for building in Thumb2 mode
crypto: arm/ghash-ce - add missing async import/export
crypto: cryptd - initialize child shash_desc on import
arch/arm/crypto/ghash-ce-glue.c | 24 ++++++++++++++++++++
arch/arm/crypto/sha1-armv7-neon.S | 1 -
crypto/cryptd.c | 9 ++++++--
3 files changed, 31 insertions(+), 3 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v2 1/3] crypto: arm/sha1-neon - add support for building in Thumb2 mode
From: Ard Biesheuvel @ 2016-09-01 13:25 UTC (permalink / raw)
To: linux-crypto, herbert; +Cc: Ard Biesheuvel
In-Reply-To: <1472736343-15305-1-git-send-email-ard.biesheuvel@linaro.org>
The ARMv7 NEON module is explicitly built in ARM mode, which is not
supported by the Thumb2 kernel. So remove the explicit override, and
leave it up to the build environment to decide whether the core SHA1
routines are assembled as ARM or as Thumb2 code.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm/crypto/sha1-armv7-neon.S | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/arm/crypto/sha1-armv7-neon.S b/arch/arm/crypto/sha1-armv7-neon.S
index dcd01f3f0bb0..2468fade49cf 100644
--- a/arch/arm/crypto/sha1-armv7-neon.S
+++ b/arch/arm/crypto/sha1-armv7-neon.S
@@ -12,7 +12,6 @@
#include <asm/assembler.h>
.syntax unified
-.code 32
.fpu neon
.text
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/3] crypto: arm/ghash-ce - add missing async import/export
From: Ard Biesheuvel @ 2016-09-01 13:25 UTC (permalink / raw)
To: linux-crypto, herbert; +Cc: Ard Biesheuvel
In-Reply-To: <1472736343-15305-1-git-send-email-ard.biesheuvel@linaro.org>
Since commit 8996eafdcbad ("crypto: ahash - ensure statesize is non-zero"),
all ahash drivers are required to implement import()/export(), and must have
a non-zero statesize. Fix this for the ARM Crypto Extensions GHASH
implementation.
Fixes: 8996eafdcbad ("crypto: ahash - ensure statesize is non-zero")
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
arch/arm/crypto/ghash-ce-glue.c | 24 ++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm/crypto/ghash-ce-glue.c b/arch/arm/crypto/ghash-ce-glue.c
index 1568cb5cd870..b88364aa149a 100644
--- a/arch/arm/crypto/ghash-ce-glue.c
+++ b/arch/arm/crypto/ghash-ce-glue.c
@@ -220,6 +220,27 @@ static int ghash_async_digest(struct ahash_request *req)
}
}
+static int ghash_async_import(struct ahash_request *req, const void *in)
+{
+ struct ahash_request *cryptd_req = ahash_request_ctx(req);
+ struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+ struct ghash_async_ctx *ctx = crypto_ahash_ctx(tfm);
+ struct shash_desc *desc = cryptd_shash_desc(cryptd_req);
+
+ desc->tfm = cryptd_ahash_child(ctx->cryptd_tfm);
+ desc->flags = req->base.flags;
+
+ return crypto_shash_import(desc, in);
+}
+
+static int ghash_async_export(struct ahash_request *req, void *out)
+{
+ struct ahash_request *cryptd_req = ahash_request_ctx(req);
+ struct shash_desc *desc = cryptd_shash_desc(cryptd_req);
+
+ return crypto_shash_export(desc, out);
+}
+
static int ghash_async_setkey(struct crypto_ahash *tfm, const u8 *key,
unsigned int keylen)
{
@@ -268,7 +289,10 @@ static struct ahash_alg ghash_async_alg = {
.final = ghash_async_final,
.setkey = ghash_async_setkey,
.digest = ghash_async_digest,
+ .import = ghash_async_import,
+ .export = ghash_async_export,
.halg.digestsize = GHASH_DIGEST_SIZE,
+ .halg.statesize = sizeof(struct ghash_desc_ctx),
.halg.base = {
.cra_name = "ghash",
.cra_driver_name = "ghash-ce",
--
2.7.4
^ permalink raw reply related
* [PATCH v2 3/3] crypto: cryptd - initialize child shash_desc on import
From: Ard Biesheuvel @ 2016-09-01 13:25 UTC (permalink / raw)
To: linux-crypto, herbert; +Cc: Ard Biesheuvel
In-Reply-To: <1472736343-15305-1-git-send-email-ard.biesheuvel@linaro.org>
When calling .import() on a cryptd ahash_request, the structure members
that describe the child transform in the shash_desc need to be initialized
like they are when calling .init()
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
crypto/cryptd.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/crypto/cryptd.c b/crypto/cryptd.c
index cf8037a87b2d..510a25f6ee37 100644
--- a/crypto/cryptd.c
+++ b/crypto/cryptd.c
@@ -631,9 +631,14 @@ static int cryptd_hash_export(struct ahash_request *req, void *out)
static int cryptd_hash_import(struct ahash_request *req, const void *in)
{
- struct cryptd_hash_request_ctx *rctx = ahash_request_ctx(req);
+ struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+ struct cryptd_hash_ctx *ctx = crypto_ahash_ctx(tfm);
+ struct shash_desc *desc = cryptd_shash_desc(req);
+
+ desc->tfm = ctx->child;
+ desc->flags = req->base.flags;
- return crypto_shash_import(&rctx->desc, in);
+ return crypto_shash_import(desc, in);
}
static int cryptd_create_hash(struct crypto_template *tmpl, struct rtattr **tb,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 1/2] crypto: arm/ghash-ce - add missing async import/export
From: Ard Biesheuvel @ 2016-09-01 15:14 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto@vger.kernel.org
In-Reply-To: <CAKv+Gu8tr3kq9gYpJjNeKvt-PDsnBUb1FZqxOeNBfuF+J6hCzA@mail.gmail.com>
On 1 September 2016 at 14:19, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 31 August 2016 at 15:41, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>> On Mon, Aug 29, 2016 at 12:19:53PM +0100, Ard Biesheuvel wrote:
>>> Since commit 8996eafdcbad ("crypto: ahash - ensure statesize is non-zero"),
>>> all ahash drivers are required to implement import()/export(), and must have
>>> a non-zero statesize. Fix this for the ARM Crypto Extensions GHASH
>>> implementation.
>>>
>>> Fixes: 8996eafdcbad ("crypto: ahash - ensure statesize is non-zero")
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> arch/arm/crypto/ghash-ce-glue.c | 26 ++++++++++++++++++++++++++
>>> 1 file changed, 26 insertions(+)
>>>
>>> diff --git a/arch/arm/crypto/ghash-ce-glue.c b/arch/arm/crypto/ghash-ce-glue.c
>>> index 1568cb5cd870..212aaa715fdb 100644
>>> --- a/arch/arm/crypto/ghash-ce-glue.c
>>> +++ b/arch/arm/crypto/ghash-ce-glue.c
>>> @@ -220,6 +220,29 @@ static int ghash_async_digest(struct ahash_request *req)
>>> }
>>> }
>>>
>>> +static int ghash_async_import(struct ahash_request *req, const void *in)
>>> +{
>>> + struct ahash_request *cryptd_req = ahash_request_ctx(req);
>>> + struct shash_desc *desc = cryptd_shash_desc(cryptd_req);
>>> + struct ghash_desc_ctx *dctx = shash_desc_ctx(desc);
>>> +
>>> + ghash_async_init(req);
>>
>> Is this really needed?
>>
>
> Actually, yes, and more. I could not find in the documentation whether
> the .import() hook requires .init() to be called first, but based on
> the test case in testmgr.c, it appears that is not the case.
>
> This means that the stuff that occurs in .init() to establish the
> relation between this desc and its child transform needs to be copied
> here as well. In fact, the only way I could make this work is by
> adding it to cryptd's import() routines as well, otherwise the test
> crashes reliably.
>
>>> + *dctx = *(const struct ghash_desc_ctx *)in;
>>
>> I'd prefer to call the underlying shash import/export functions
>> like we do in cryptd.
>>
>
I managed to track down why this issue seems specific to ARM (i.e.,
why it does not affect the x86 clmul-ni driver as well)
The culprit appears to be that the .cra_name of the internal shash is
"ghash", (and not "__ghash" like in the x86 case) which causes the
test code to run the test on not only the public ahash, but also on
the internal cryptd() encapsulated shash, and also on the internal
shash itself.
However, that does not answer the question whether .init() must be
called before .import() [which the test code does not do]. If not,
then please disregard my v2, and I will followup with a patch that
renames ghash to __ghash (but .import() will still require the .init()
bits as well). Given that these internal shashes/ahashes are in fact
callable, and calling .import() will result in a crash, I suppose
duplicating some of the init() code in .import() makes sense
regardless.
^ permalink raw reply
* Re: CONFIG_FIPS without module loading support?
From: NTU @ 2016-09-02 2:57 UTC (permalink / raw)
Cc: linux-crypto
In-Reply-To: <2366601.movXpHLGKJ@tauon.atsec.com>
Ok, can we merge that in?
Alec Ari
^ permalink raw reply
* bcachefs: Encryption (Posting for review)
From: Kent Overstreet @ 2016-09-02 4:41 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, linux-crypto, linux-bcache
Encryption in bcachefs is done and working and I just finished documenting the
design - so now, it needs more eyeballs and vetting before letting users play
with it.
I'd appreciate help circulating this around to people who'd be qualified to
review it, too.
Also, bcachefs development is still moving along but it still needs funding: if
you're a fan of bcache or fancypants new filesystems, please chip in -
https://www.patreon.com/bcachefs
https://bcache.evilpiepirate.org/Bcachefs/
-------------
The master copy of this document is at:
https://bcache.evilpiepirate.org/Encryption/
# bcache/bcachefs encryption design:
This document is intended for review by cryptographers and other experience
implementers of cryptography code, before the design is frozen. Everything
described in this document has been implemented and tested.
The code may be found at
[[https://evilpiepirate.org/git/linux-bcache.git/log/?h=bcache-encryption]]
Feedback and comments should be sent to Kent Overstreet,
kent.overstreet@gmail.com.
## Intro:
Bcachefs provides whole-filesystem encryption, using ChaCha20/Poly1305.
Encryption may be enabled when creating a filesystem, or encryption may be
enabled on an existing filesystem (TODO: implement interface for enabling
encryption on an existing filesystem - kernel code exists).
Example:
$ bcache format --encrypted /dev/sda
(Enter passphrase when prompted)
$ bcache unlock /dev/sda
(Enter passphrase again)
Then mount as normal:
$ mount /dev/sda /mnt
## Goals:
Bcachefs encryption is meant to be a clean slate design that prioritizes
security and robustness, and is meant to defend against a wider variety of
yadversarial models than is typical in existing filesystem level or block level
encryption.
## Filesystem vs. directory encryption
We do not currently offer per directory encryption; instead, we take an "encrypt
everything" approach.
Rationale:
With per directory encryption, preventing metadata from leaking is highly
problematic. For example, file sizes - file sizes are required for fsck, so
they would have to be stored unencrypted - or failing that some complicated way
of deferring fsck for that part of the filesystem until the key has been
provided. There would be additional complications around filenames, xattrs,
extents (and inline extents), etc. - not necessarily insurmountable, but they
would definitely lead to a more complicated, more fragile design.
With whole filesystem encryption, it’s much easier to say what is and isn’t
encrypted, we can encrypt at the granularity of an entire metadata write (a
journal entry, a btree node) and it's much easier to show that the contents -
everything after the header for that particular metadata write - will not leak.
### Algorithms
By virtue of working within a copy on write filesystem with provisions for ZFS
style checksums (that is, checksums with the pointers, not the data), we’re
able to use a modern AEAD style construction. We use ChaCha20 and Poly1305. We
use the cyphers directly instead of using the kernel AEAD library (and thus
means there's a bit more in the design that needs auditing).
The current design uses the same key for both ChaCha20 and Poly1305, but my
recent rereading of the Poly1305-AES paper seems to imply that the Poly1305 key
shouldn't be used for anything else. Guidance from actual cryptographers would
be appreciated here; the ChaCha20/Poly1305 AEAD RFC appears to be silent on the
matter.
Note that ChaCha20 is a stream cypher. This means that it’s critical that we use
a cryptographic MAC (which would be highly desirable anyways), and also avoiding
nonce reuse is critical. Getting nonces right is where most of the trickiness is
involved in bcachefs’s encryption.
The current algorithm choices are not hard coded. Bcachefs already has
selectable checksum types, and every individual data and metadata write has a
field that describes the checksum algorithm that was used. On disk, encrypted
data is represented as a new checksum type - so we now have [none, crc32c,
crc64, chacha20/poly1305] as possible methods for data to be
checksummed/encrypted. If in the future we add new encryption algorithms, users
will be able to switch to the new algorithm on existing encrypted filesystems;
new data will be written with the new algorithm and old data will be read with
the old algorithm until it is rewritten.
### Key derivation, master key
Userspace tooling takes the user's passphrase and derives an encryption key with
scrypt. This key is made available to the kernel (via the Linux kernel's keyring
service) prior to mounting the filesystem.
On filesystem mount, the userspace provided key is used to decrypt the master
key, which is stored in the superblock - also with ChaCha20. The master key is
encrypted with an 8 byte header, so that we can tell if the correct key was
supplied.
### Metadata
Except for the superblock, no metadata in bcache/bcachefs is updated in place -
everything is more or less log structured. Only the superblock is stored
unencrypted; other metadata is stored with an unencrypted header and encrypted
contents.
The superblock contains:
* Label and UUIDs identifying the filesystem
* A list of component devices (for multi-device filesystems), and information
on their size, geometry, status (active/failed), last used timestamp
* Filesystem options
* The location of the journal
For the rest of the metadata, the unencrypted portion contains:
* 128 bit checksum/MAC field
* Magic number - identifies a given structure as btree/journal/allocation
information, for that filesystem
* Version number (of on disk format), flags (including checksum/encryption
type).
* Sequence numbers: journal entries have an ascending 64 bit sequence number,
btree node entries have a random 64 bit sequence number identifying them as
belonging to that node. Btree nodes also have a field containing the sequence
number of the most recent journal entry they contain updates from; this is
stored unencrypted so it can be used as part of the nonce.
* Size of the btree node entry/journal entry, in u64s
Btree node layout information is encrypted; an attacker could tell that a given
location on disk was a btree node, but the part of the header that indicates
what range of the keyspace, or which btree ID (extents/dirents/xattrs/etc.), or
which level of the btree is all encrypted.
#### Metadata nonces
* Journal entries use their sequence number - which is unique for a given
filesystem. When metadata is being replicated and we're doing multiple
journal writes with the same sequence number - and thus nonce - we really are
writing the same data (we only checksum once, not once per write).
* Btree nodes concatenate a few things for the nonce:
- A 64 bit random integer, which is generated per btree node (but btree nodes
are log structured, so entries within a given btree node share the same
integer).
- A journal sequence number. For btree node writes done at around the same
point in time, this field can be identical in unrelated btree node writes -
but only for btree nodes writes done relatively close in time, so the
journal sequence number plus the previous random integer should be more
than sufficient entropy.
- And lastly the offset within the btree node, so that btree node entries
sharing the same random integer are guaranteed a different nonce.
* Allocation information (struct prio_set):
bcache/bcachefs doesn't have allocation information persisted like other
filesystems, but this is our closest equivalent - this structure mainly
stores generation numbers that correspond to extent pointers.
Allocation information uses a dedicated randomly generated 96 bit nonce
field.
### Data
Data writes have no unencrypted header: checksums/MACs, nonces, etc. are stored
with the pointers, ZFS style.
Bcache/bcachefs is extent based, not block based: pointers point to variable
sized chunks of data, and we store one checksum/MAC per extent, not per block: a
checksum or MAC might cover up to 64k (extents that aren't checksummed or
compressed may be larger). Nonces are thus also per extent, not per block.
Currently, the Poly1305 MAC is truncated to 64 bits - due to a desire not to
inflate our metadata any more than necessary. Guidance from cryptographers is
requested as to whether this is a reasonable option; do note that the MAC is not
stored with the data, but is itself stored encrypted elsewhere in the btree. We
do already have different fields for storing 4 byte checksums and 8 byte
checksums; it will be a trivial matter to add a field allowing 16 byte checksums
to be stored, and we will add that anyways - so this isn't a pressing design
issue, this is just a matter of what the defaults should be and what we should
tell users.
#### Extent nonces
We don't wish to simply add a random 96 bit nonce to every extent - that would
inflate our metadata size by a very significant amount. Instead, keys (of which
extents are a subset) have a 96 bit version number field; when encryption is
enabled, we ensure that version numbers are enabled and every new extent gets a
new, unique version number.
However, extents may be partially overwritten or split, and then to defragment
we may have to rewrite those partially overwritten extents elsewhere. We cannot
simply pick a new version number when we rewrite an extent - that would break
semantics other uses of version numbers expect.
When we rewrite an extent, we only write the currently live portions of the
extent - we don't rewrite the parts that were overwritten. We can't write it out
with the same nonce as the original extent.
If we concatenated the version number with the offset within the file, and the
extent's current size - that would work, except that it would break fcollapse(),
which moves extents to a new position within a file. We are forced to add some
additional state to extents.
We could add a small counter that is incremented every time the size of an
extent is reduced (and the data it points to changes); we can easily bound the
size of the counter we need by the maximum size of a checksummed extent. But
this approach fails when extents are split.
What can work is if we add a field for "offset from the start of the original
extent to the start of the current extent" - updating that field whenever we
trim the front of an extent.
If we have that, then we could simply skip ahead in the keystream to where the
currently live data lived in the original extent - there's no problem with nonce
reuse if you're encrypting exactly the same data. Except - that fails with
compression, since if we take an extent, drop the first 4k, and compress it,
that won't give the same data as if we compress it and then drop the first 4k of
the compressed data.
The approach almost works though, if we take that offset and use it as part of
our nonce: what we want to do is construct a function that will output the same
nonce iff two extents (fragments of the same original extent) really are the
same data.
Offset into the original extent works in the absence of compression - two
fragments with the same offset but different sizes will be equal in their common
prefix, ignoring compression. We can handle compression if we also include both
the current size, and the current compression function - offset + current size
uniquely determines the uncompressed data, so, offset + current size +
compression function will uniquely determine the compressed output.
#### Nonce reuse on startup
After recovery, we must ensure we don't reuse existing version numbers - we must
ensure that newly allocated version numbers are strictly greater than any
version number that has every been used before.
The problem here is that we use the version number to write the data before
adding the extent with that version number to the btree: after unclean shutdown,
there will have been version numbers used to write data for which we have no
record in the btree.
The rigorous solution to this is to add a field (likely to the journal header)
that indicates version numbers smaller than that field may have been used.
However, we don't do that yet - it's not completely trivial since it'll add
another potential dependency in the IO path that needs some analysis.
The current solution implemented by the code is to scan every existing version
number (as part of an existing pass), and set the next version number to
allocate to be 64k greater than the highest existing version number that was
found.
^ permalink raw reply
* [PATCH] crypto: ccp - add missing release in ccp_dmaengine_register
From: Quentin Lambert @ 2016-09-02 9:48 UTC (permalink / raw)
To: Tom Lendacky, Gary Hook, Herbert Xu, David S. Miller,
linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
ccp_dmaengine_register used to return with an error code before
releasing all resource. This patch adds a jump to the appropriate label
ensuring that the resources are properly released before returning.
This issue was found with Hector.
Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
drivers/crypto/ccp/ccp-dmaengine.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
--- a/drivers/crypto/ccp/ccp-dmaengine.c
+++ b/drivers/crypto/ccp/ccp-dmaengine.c
@@ -650,8 +650,11 @@ int ccp_dmaengine_register(struct ccp_de
dma_desc_cache_name = devm_kasprintf(ccp->dev, GFP_KERNEL,
"%s-dmaengine-desc-cache",
ccp->name);
- if (!dma_cmd_cache_name)
- return -ENOMEM;
+ if (!dma_cmd_cache_name) {
+ ret = -ENOMEM;
+ goto err_cache;
+ }
+
ccp->dma_desc_cache = kmem_cache_create(dma_desc_cache_name,
sizeof(struct ccp_dma_desc),
sizeof(void *),
^ permalink raw reply
* Re: [PATCH v2 3/4] hw_random: jz4780-rng: Add RNG node to jz4780.dtsi
From: Paul Burton @ 2016-09-02 12:47 UTC (permalink / raw)
To: PrasannaKumar Muralidharan
Cc: mpm, herbert, robh+dt, mark.rutland, ralf, gregkh,
boris.brezillon, harvey.hunt, prarit, f.fainelli,
joshua.henderson, narmstrong, linus.walleij, linux-crypto,
devicetree, linux-mips
In-Reply-To: <1472321697-3094-4-git-send-email-prasannatsmkumar@gmail.com>
On 27/08/16 19:14, PrasannaKumar Muralidharan wrote:
> This patch adds RNG node to jz4780.dtsi.
>
> Signed-off-by: PrasannaKumar Muralidharan <prasannatsmkumar@gmail.com>
> ---
> arch/mips/boot/dts/ingenic/jz4780.dtsi | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/boot/dts/ingenic/jz4780.dtsi b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> index b868b42..f11d139 100644
> --- a/arch/mips/boot/dts/ingenic/jz4780.dtsi
> +++ b/arch/mips/boot/dts/ingenic/jz4780.dtsi
> @@ -36,7 +36,7 @@
>
> cgu: jz4780-cgu@10000000 {
> compatible = "ingenic,jz4780-cgu";
> - reg = <0x10000000 0x100>;
> + reg = <0x10000000 0xD8>;
Hi PrasannaKumar,
I don't like this change. The RNG registers are documented as a part of
the same hardware block as the clock & power stuff which the CGU driver
handles, and indeed in the M200 SoC there is a power-related register
after the RNG registers. So shortening the range covered by the CGU
driver is not the right way to go.
Perhaps you could instead have the CGU driver make use of the syscon
infrastructure to expose a regmap which your RNG driver could pick up & use?
Thanks,
Paul
>
> clocks = <&ext>, <&rtc>;
> clock-names = "ext", "rtc";
> @@ -44,6 +44,11 @@
> #clock-cells = <1>;
> };
>
> + rng: jz4780-rng@100000D8 {
> + compatible = "ingenic,jz4780-rng";
> + reg = <0x100000D8 0x8>;
> + };
> +
> uart0: serial@10030000 {
> compatible = "ingenic,jz4780-uart";
> reg = <0x10030000 0x100>;
>
^ permalink raw reply
* Re: [PATCH v2 3/4] hw_random: jz4780-rng: Add RNG node to jz4780.dtsi
From: PrasannaKumar Muralidharan @ 2016-09-02 12:57 UTC (permalink / raw)
To: Paul Burton
Cc: mpm-VDJrAJ4Gl5ZBDgjK7y7TUQ, Herbert Xu,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
Ralf Baechle, Greg KH,
boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
harvey.hunt-1AXoQHu6uovQT0dZR+AlfA, prarit-H+wXaHxf7aLQT0dZR+AlfA,
Florian Fainelli, joshua.henderson-UWL1GkI3JZL3oGB3hsPCZA,
narmstrong-rdvid1DuHRBWk0Htik3J/w, Linus Walleij,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-mips-6z/3iImG2C8G8FEW9MqTrA
In-Reply-To: <4a7fb1cb-e0d4-31b7-7016-35adb63a659d-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
> I don't like this change. The RNG registers are documented as a part of
> the same hardware block as the clock & power stuff which the CGU driver
> handles, and indeed in the M200 SoC there is a power-related register
> after the RNG registers. So shortening the range covered by the CGU
> driver is not the right way to go.
Could not find M200 SoC PM in ingenic's website or ftp. So did not notice this.
> Perhaps you could instead have the CGU driver make use of the syscon
> infrastructure to expose a regmap which your RNG driver could pick up & use?
I will see how to use syscon and provide an updated patch.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: CONFIG_FIPS without module loading support?
From: Herbert Xu @ 2016-09-02 14:22 UTC (permalink / raw)
To: NTU; +Cc: linux-crypto
In-Reply-To: <CAM5Ud7Pw84GcjpooX_5VfBuRTgU8+HMKkM6GBnD1z=JEw0PkXw@mail.gmail.com>
NTU <neotheuser@gmail.com> wrote:
> Ok, can we merge that in?
We can if you send us a patch :)
--
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
* Who will copy the AAD data to dest. buffer
From: Harsh Jain @ 2016-09-02 14:35 UTC (permalink / raw)
To: Herbert Xu, linux-crypto, Stephan Mueller
Hi Herbert,
Is copy of AAD data to destination buffer when dst != src is mandatory
requirements for crypto drivers or we can skip this copy. Actually I
am bit confused, In following link Stephen had mentioned caller will
memcpy the AAD to destination buffer but authenc.c also copies the AAD
to dest. buffer.
http://www.chronox.de/libkcapi/html/ch02s02.html
Secondly When AAD data remains unchanged in AEAD encryption/decryption
operations. Why we copy the same data to destination buffer?
Thanks & Regards
Harsh Jain
^ permalink raw reply
* [PATCH 0/2] add omitted release in qat_common
From: Quentin Lambert @ 2016-09-02 14:37 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel,
Quentin Lambert
Cc: kernel-janitors
The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.
---
drivers/crypto/qat/qat_common/qat_uclo.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
---
^ permalink raw reply
* [PATCH 1/2] crypto: qat - introduces a variable to handle error codes
From: Quentin Lambert @ 2016-09-02 14:37 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902143759.31125-1-lambert.quentin@gmail.com>
Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.
Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
drivers/crypto/qat/qat_common/qat_uclo.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
{
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;
+ int ret;
obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
&obj_handle->str_table)) {
pr_err("QAT: UOF doesn't have effective images\n");
+ ret = -EFAULT;
goto out_err;
}
obj_handle->uimage_num =
qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
- if (!obj_handle->uimage_num)
+ if (!obj_handle->uimage_num) {
+ ret = -EFAULT;
goto out_err;
+ }
if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
pr_err("QAT: Bad object\n");
+ ret = -EFAULT;
goto out_check_uof_aemask_err;
}
qat_uclo_init_uword_num(handle);
qat_uclo_map_initmem_table(&obj_handle->encap_uof_obj,
&obj_handle->init_mem_tab);
- if (qat_uclo_set_ae_mode(handle))
+ if (qat_uclo_set_ae_mode(handle)) {
+ ret = -EFAULT;
goto out_check_uof_aemask_err;
+ }
return 0;
out_check_uof_aemask_err:
for (ae = 0; ae < obj_handle->uimage_num; ae++)
kfree(obj_handle->ae_uimage[ae].page);
out_err:
kfree(obj_handle->uword_buf);
- return -EFAULT;
+ return ret;
}
static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,
^ permalink raw reply
* [PATCH 2/2] crypto: qat - fix resource release omissions
From: Quentin Lambert @ 2016-09-02 14:37 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902143759.31125-1-lambert.quentin@gmail.com>
This issue was found with Hector.
Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
drivers/crypto/qat/qat_common/qat_uclo.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
(PID_MINOR_REV & handle->hal_handle->revision_id);
if (qat_uclo_check_uof_compat(obj_handle)) {
pr_err("QAT: UOF incompatible\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_err;
}
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||
^ permalink raw reply
* Re: Who will copy the AAD data to dest. buffer
From: Herbert Xu @ 2016-09-02 14:42 UTC (permalink / raw)
To: Harsh Jain; +Cc: linux-crypto, Stephan Mueller
In-Reply-To: <CAFXBA=m-Ued+9EGmfwa210cm6HZGxN3P-g_Fsapzcw+a3FKdZw@mail.gmail.com>
On Fri, Sep 02, 2016 at 08:05:04PM +0530, Harsh Jain wrote:
> Hi Herbert,
>
> Is copy of AAD data to destination buffer when dst != src is mandatory
> requirements for crypto drivers or we can skip this copy. Actually I
> am bit confused, In following link Stephen had mentioned caller will
> memcpy the AAD to destination buffer but authenc.c also copies the AAD
> to dest. buffer.
>
> http://www.chronox.de/libkcapi/html/ch02s02.html
It has to be copied if src != dst.
> Secondly When AAD data remains unchanged in AEAD encryption/decryption
> operations. Why we copy the same data to destination buffer?
This greatly simplifies the implementation of the AEAD algorithms
because we can throw away src and use the dst only. For example,
authenc hashes the AAD and ciphertext. If we didn't force the
copy it would have to hash them separately, meaning the use of
the slow init/update/final interface. With the copy it can use
the digest interface.
Cheers,
--
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
* [PATCH 0/2][RESEND] add omitted release in qat_common
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel,
Quentin Lambert
Cc: kernel-janitors
In-Reply-To: <20160902143759.31125-1-lambert.quentin@gmail.com>
The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.
---
drivers/crypto/qat/qat_common/qat_uclo.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
---
^ permalink raw reply
* [PATCH 2/2] crypto: qat - fix resource release omissions
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902144400.31216-1-lambert.quentin@gmail.com>
In certain cases qat_uclo_parse_uof_obj used to return with an error code
before releasing all resources. This patch add a jump to the appropriate label
ensuring that the resources are properly released before returning.
This issue was found with Hector.
Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
drivers/crypto/qat/qat_common/qat_uclo.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -981,7 +981,8 @@ static int qat_uclo_parse_uof_obj(struct
(PID_MINOR_REV & handle->hal_handle->revision_id);
if (qat_uclo_check_uof_compat(obj_handle)) {
pr_err("QAT: UOF incompatible\n");
- return -EINVAL;
+ ret = -EINVAL;
+ goto out_err;
}
obj_handle->ustore_phy_size = ICP_QAT_UCLO_MAX_USTORE;
if (!obj_handle->obj_hdr->file_buff ||
^ permalink raw reply
* [PATCH 1/2][RESEND] crypto: qat - introduces a variable to handle error codes
From: Quentin Lambert @ 2016-09-02 14:43 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902144400.31216-1-lambert.quentin@gmail.com>
Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.
Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
drivers/crypto/qat/qat_common/qat_uclo.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
{
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;
+ int ret;
obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
&obj_handle->str_table)) {
pr_err("QAT: UOF doesn't have effective images\n");
+ ret = -EFAULT;
goto out_err;
}
obj_handle->uimage_num =
qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
- if (!obj_handle->uimage_num)
+ if (!obj_handle->uimage_num) {
+ ret = -EFAULT;
goto out_err;
+ }
if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
pr_err("QAT: Bad object\n");
+ ret = -EFAULT;
goto out_check_uof_aemask_err;
}
qat_uclo_init_uword_num(handle);
qat_uclo_map_initmem_table(&obj_handle->encap_uof_obj,
&obj_handle->init_mem_tab);
- if (qat_uclo_set_ae_mode(handle))
+ if (qat_uclo_set_ae_mode(handle)) {
+ ret = -EFAULT;
goto out_check_uof_aemask_err;
+ }
return 0;
out_check_uof_aemask_err:
for (ae = 0; ae < obj_handle->uimage_num; ae++)
kfree(obj_handle->ae_uimage[ae].page);
out_err:
kfree(obj_handle->uword_buf);
- return -EFAULT;
+ return ret;
}
static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,
^ permalink raw reply
* [PATCH v2 0/2] add omitted release in qat_common
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel,
Quentin Lambert
Cc: kernel-janitors
In-Reply-To: <20160902144400.31216-1-lambert.quentin@gmail.com>
The first patch introduces a variable to handle different error codes and be
able to reuse the same clean up code. The second add an omitted release by
jumping to the clean code having set the returned value to the proper error
code.
-changes since v1
I failed to send the first version properly and it was missing a proper commit
message, just ignore it.
---
drivers/crypto/qat/qat_common/qat_uclo.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
---
^ permalink raw reply
* [PATCH v2 1/2] crypto: qat - introduces a variable to handle error codes
From: Quentin Lambert @ 2016-09-02 14:47 UTC (permalink / raw)
To: Giovanni Cabiddu, Salvatore Benedetto, Herbert Xu,
David S. Miller, qat-linux, linux-crypto, linux-kernel
Cc: kernel-janitors, Quentin Lambert
In-Reply-To: <20160902144753.31334-1-lambert.quentin@gmail.com>
Most error code used to jump to a label that lead to a "return -EFAULT"
statement. This patch introduces a variable that stores the error code
so that other error branches can use the same label to exit.
Signed-off-by: Quentin Lambert <lambert.quentin@gmail.com>
---
drivers/crypto/qat/qat_common/qat_uclo.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
--- a/drivers/crypto/qat/qat_common/qat_uclo.c
+++ b/drivers/crypto/qat/qat_common/qat_uclo.c
@@ -966,6 +966,7 @@ static int qat_uclo_parse_uof_obj(struct
{
struct icp_qat_uclo_objhandle *obj_handle = handle->obj_handle;
unsigned int ae;
+ int ret;
obj_handle->uword_buf = kcalloc(UWORD_CPYBUF_SIZE, sizeof(uint64_t),
GFP_KERNEL);
@@ -987,29 +988,35 @@ static int qat_uclo_parse_uof_obj(struct
!qat_uclo_map_str_table(obj_handle->obj_hdr, ICP_QAT_UOF_STRT,
&obj_handle->str_table)) {
pr_err("QAT: UOF doesn't have effective images\n");
+ ret = -EFAULT;
goto out_err;
}
obj_handle->uimage_num =
qat_uclo_map_uimage(obj_handle, obj_handle->ae_uimage,
ICP_QAT_UCLO_MAX_AE * ICP_QAT_UCLO_MAX_CTX);
- if (!obj_handle->uimage_num)
+ if (!obj_handle->uimage_num) {
+ ret = -EFAULT;
goto out_err;
+ }
if (qat_uclo_map_ae(handle, handle->hal_handle->ae_max_num)) {
pr_err("QAT: Bad object\n");
+ ret = -EFAULT;
goto out_check_uof_aemask_err;
}
qat_uclo_init_uword_num(handle);
qat_uclo_map_initmem_table(&obj_handle->encap_uof_obj,
&obj_handle->init_mem_tab);
- if (qat_uclo_set_ae_mode(handle))
+ if (qat_uclo_set_ae_mode(handle)) {
+ ret = -EFAULT;
goto out_check_uof_aemask_err;
+ }
return 0;
out_check_uof_aemask_err:
for (ae = 0; ae < obj_handle->uimage_num; ae++)
kfree(obj_handle->ae_uimage[ae].page);
out_err:
kfree(obj_handle->uword_buf);
- return -EFAULT;
+ return ret;
}
static int qat_uclo_map_suof_file_hdr(struct icp_qat_fw_loader_handle *handle,
^ 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