* Re: Is the asynchronous hash crypto API asynchronous?
From: Stephan Müller @ 2017-01-09 10:23 UTC (permalink / raw)
To: Gilad Ben-Yossef; +Cc: linux-crypto, Herbert Xu, David Miller
In-Reply-To: <CAOtvUMd=EHaFq=AtMePzN9fsoMLszZ0ruAu8KaaP=W=8J7nGUg@mail.gmail.com>
Am Sonntag, 8. Januar 2017, 15:45:37 CET schrieb Gilad Ben-Yossef:
Hi Gilad,
> ahash_request_set_callback(req, 0, NULL, NULL);
>
> Would anyone be kind enough to enlighten me?
The documentation got out of sync with the real world. I will file a patch for
that shortly.
The ahash API works identically to the async skcipher API for which you find
an example in the api-samples.rst. There you see that with the set_callback, a
function is registered that is triggered upon completion of the operation.
Thus, use the callback example you find for skcipher for your ahash operation
and you get an async operation.
Ciao
Stephan
^ permalink raw reply
* Outlook Security Team Micorosof
From: Devaraj Veerasamy, Dr @ 2017-01-09 11:21 UTC (permalink / raw)
To: "\"NO-REPLY@WEBMAIL.NET""
MICROSOFT OUTLOOK anmälan
Din e-rutan konto behöver vara verifiera nu för oegentligheter finns i din e-box-konto eller kommer att blockera. Klicka här<https://mrswangjuan17.wixsite.com/webaccess2017> för att verifiera din e-postkonto och fil i ditt korrekta användarnamn och lösenord omedelbart
Outlook Security Team Micorosof
Tack.
Copyright © 2017 MIcrosoft OUtlook . Inc . All rights reserved.
^ permalink raw reply
* Re: Is the asynchronous hash crypto API asynchronous?
From: Gilad Ben-Yossef @ 2017-01-09 12:08 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto, Herbert Xu, David Miller
In-Reply-To: <3639236.s9QSuQoh7d@positron.chronox.de>
Hello Stephen,
Before getting to business I wish to offer my thanks for hosting the
kernel crypto documentation on your web site at chronox.de. It has
proven very useful to me :-)
On Mon, Jan 9, 2017 at 12:23 PM, Stephan Müller <smueller@chronox.de> wrote:
> Am Sonntag, 8. Januar 2017, 15:45:37 CET schrieb Gilad Ben-Yossef:
>
> Hi Gilad,
>
>> ahash_request_set_callback(req, 0, NULL, NULL);
>
>>
>> Would anyone be kind enough to enlighten me?
>
> The documentation got out of sync with the real world. I will file a patch for
> that shortly.
>
> The ahash API works identically to the async skcipher API for which you find
> an example in the api-samples.rst. There you see that with the set_callback, a
> function is registered that is triggered upon completion of the operation.
>
> Thus, use the callback example you find for skcipher for your ahash operation
> and you get an async operation.
Thank you very much for pointing out the skcipher example. It is very helpful.
However, I suspect there is something a miss here beyond documentation:
There is (quite a lot of) kernel code calling
ahash_request_set_callback() with a NULL callback and consequently not
performing any synchronization in the completion of the hashing
operations.
See for example crypt_iv_essiv_init() in drivers/md/dm-crypt.c
Other similar call sites can be found at block/drbd/drbd_worker.c,
net/ppp/ppp_mppe.c, net/wireless/intersil/orinoco/mic.c,
scsi/iscsi_tcp.c, target/iscsi/iscsi_target_login.c
As far as I could tell the code in crypto/ahash.c does not take any
special consideration of the case where a NULL call back function has
been set and at least one of the underlying ahash algorithm provider
will crash if used like this.
This seems broken to me. I would be very happy to offer to fix the
broken call sites, if you can only confirm my understanding that
indeed cases which register a NULL callback are broken and it is not
just a misunderstanding on my part.
Many thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
-- Jean-Baptiste Queru
^ permalink raw reply
* Re: Is the asynchronous hash crypto API asynchronous?
From: Stephan Müller @ 2017-01-09 15:25 UTC (permalink / raw)
To: Gilad Ben-Yossef; +Cc: linux-crypto, Herbert Xu, David Miller
In-Reply-To: <CAOtvUMfheVHLH6y96+YjScZOYvBJegF6GwuBynR1LEgFt-O4=w@mail.gmail.com>
Am Montag, 9. Januar 2017, 14:08:23 CET schrieb Gilad Ben-Yossef:
Hi Gilad,
> Hello Stephen,
>
> Before getting to business I wish to offer my thanks for hosting the
> kernel crypto documentation on your web site at chronox.de. It has
> proven very useful to me :-)
I am glad that it is helpful.
>
> On Mon, Jan 9, 2017 at 12:23 PM, Stephan Müller <smueller@chronox.de> wrote:
> > Am Sonntag, 8. Januar 2017, 15:45:37 CET schrieb Gilad Ben-Yossef:
> >
> > Hi Gilad,
> >
> >> ahash_request_set_callback(req, 0, NULL, NULL);
> >>
> >> Would anyone be kind enough to enlighten me?
> >
> > The documentation got out of sync with the real world. I will file a patch
> > for that shortly.
> >
> > The ahash API works identically to the async skcipher API for which you
> > find an example in the api-samples.rst. There you see that with the
> > set_callback, a function is registered that is triggered upon completion
> > of the operation.
> >
> > Thus, use the callback example you find for skcipher for your ahash
> > operation and you get an async operation.
>
> Thank you very much for pointing out the skcipher example. It is very
> helpful.
>
> However, I suspect there is something a miss here beyond documentation:
>
> There is (quite a lot of) kernel code calling
> ahash_request_set_callback() with a NULL callback and consequently not
> performing any synchronization in the completion of the hashing
> operations.
I think this is legacy, although I cannot say for sure. As of now, there is
hardly any real ahash implementation out there. The only one I am aware of is
the sha1-mb/sha256-mb/sha512-mb for x86. There, the kernel would even crash if
there is a NULL as callback, if I read the code correctly.
For all other implementations, they are synchronous even though you use the
ahash API. I.e. all of those implementations would not trigger the callback
function.
>
> See for example crypt_iv_essiv_init() in drivers/md/dm-crypt.c
> Other similar call sites can be found at block/drbd/drbd_worker.c,
> net/ppp/ppp_mppe.c, net/wireless/intersil/orinoco/mic.c,
> scsi/iscsi_tcp.c, target/iscsi/iscsi_target_login.c
>
> As far as I could tell the code in crypto/ahash.c does not take any
> special consideration of the case where a NULL call back function has
> been set and at least one of the underlying ahash algorithm provider
> will crash if used like this.
I do not think that the ahash.c code crashes, but look into the sha1-mb
implementation:
...
req = cast_mcryptd_ctx_to_req(req_ctx);
if (irqs_disabled())
req_ctx->complete(&req->base, ret);
else {
local_bh_disable();
req_ctx->complete(&req->base, ret);
local_bh_enable();
...
Here you see the invocation of complete without a check.
>
> This seems broken to me. I would be very happy to offer to fix the
> broken call sites, if you can only confirm my understanding that
> indeed cases which register a NULL callback are broken and it is not
> just a misunderstanding on my part.
IMHO, the use of a NULL callback works, but should definitely converted to a
real callback function.
>
> Many thanks,
> Gilad
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v2 11/12] crypto: atmel-authenc: add support to authenc(hmac(shaX),Y(aes)) modes
From: Cyrille Pitchen @ 2017-01-09 18:24 UTC (permalink / raw)
To: Stephan Müller
Cc: herbert, davem, nicolas.ferre, linux-crypto, linux-kernel,
linux-arm-kernel
In-Reply-To: <1548507.knvAQkH9bK@tauon.atsec.com>
Hi Stephan,
Le 23/12/2016 à 12:34, Stephan Müller a écrit :
> Am Donnerstag, 22. Dezember 2016, 17:38:00 CET schrieb Cyrille Pitchen:
>
> Hi Cyrille,
>
>> This patchs allows to combine the AES and SHA hardware accelerators on
>> some Atmel SoCs. Doing so, AES blocks are only written to/read from the
>> AES hardware. Those blocks are also transferred from the AES to the SHA
>> accelerator internally, without additionnal accesses to the system busses.
>>
>> Hence, the AES and SHA accelerators work in parallel to process all the
>> data blocks, instead of serializing the process by (de)crypting those
>> blocks first then authenticating them after like the generic
>> crypto/authenc.c driver does.
>>
>> Of course, both the AES and SHA hardware accelerators need to be available
>> before we can start to process the data blocks. Hence we use their crypto
>> request queue to synchronize both drivers.
>>
>> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
>> ---
>> drivers/crypto/Kconfig | 12 +
>> drivers/crypto/atmel-aes-regs.h | 16 ++
>> drivers/crypto/atmel-aes.c | 471
>> +++++++++++++++++++++++++++++++++++++++- drivers/crypto/atmel-authenc.h |
>> 64 ++++++
>> drivers/crypto/atmel-sha-regs.h | 14 ++
>> drivers/crypto/atmel-sha.c | 344 +++++++++++++++++++++++++++--
>> 6 files changed, 906 insertions(+), 15 deletions(-)
>> create mode 100644 drivers/crypto/atmel-authenc.h
>>
>> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
>> index 79564785ae30..719a868d8ea1 100644
>> --- a/drivers/crypto/Kconfig
>> +++ b/drivers/crypto/Kconfig
>> @@ -415,6 +415,18 @@ config CRYPTO_DEV_BFIN_CRC
>> Newer Blackfin processors have CRC hardware. Select this if you
>> want to use the Blackfin CRC module.
>>
>> +config CRYPTO_DEV_ATMEL_AUTHENC
>> + tristate "Support for Atmel IPSEC/SSL hw accelerator"
>> + depends on (ARCH_AT91 && HAS_DMA) || COMPILE_TEST
>> + select CRYPTO_AUTHENC
>> + select CRYPTO_DEV_ATMEL_AES
>> + select CRYPTO_DEV_ATMEL_SHA
>> + help
>> + Some Atmel processors can combine the AES and SHA hw accelerators
>> + to enhance support of IPSEC/SSL.
>> + Select this if you want to use the Atmel modules for
>> + authenc(hmac(shaX),Y(cbc)) algorithms.
>> +
>> config CRYPTO_DEV_ATMEL_AES
>> tristate "Support for Atmel AES hw accelerator"
>> depends on HAS_DMA
>> diff --git a/drivers/crypto/atmel-aes-regs.h
>> b/drivers/crypto/atmel-aes-regs.h index 0ec04407b533..7694679802b3 100644
>> --- a/drivers/crypto/atmel-aes-regs.h
>> +++ b/drivers/crypto/atmel-aes-regs.h
>> @@ -68,6 +68,22 @@
>> #define AES_CTRR 0x98
>> #define AES_GCMHR(x) (0x9c + ((x) * 0x04))
>>
>> +#define AES_EMR 0xb0
>> +#define AES_EMR_APEN BIT(0) /* Auto Padding Enable */
>> +#define AES_EMR_APM BIT(1) /* Auto Padding Mode */
>> +#define AES_EMR_APM_IPSEC 0x0
>> +#define AES_EMR_APM_SSL BIT(1)
>> +#define AES_EMR_PLIPEN BIT(4) /* PLIP Enable */
>> +#define AES_EMR_PLIPD BIT(5) /* PLIP Decipher */
>> +#define AES_EMR_PADLEN_MASK (0xFu << 8)
>> +#define AES_EMR_PADLEN_OFFSET 8
>> +#define AES_EMR_PADLEN(padlen) (((padlen) << AES_EMR_PADLEN_OFFSET) &\
>> + AES_EMR_PADLEN_MASK)
>> +#define AES_EMR_NHEAD_MASK (0xFu << 16)
>> +#define AES_EMR_NHEAD_OFFSET 16
>> +#define AES_EMR_NHEAD(nhead) (((nhead) << AES_EMR_NHEAD_OFFSET) &\
>> + AES_EMR_NHEAD_MASK)
>> +
>> #define AES_TWR(x) (0xc0 + ((x) * 0x04))
>> #define AES_ALPHAR(x) (0xd0 + ((x) * 0x04))
>>
>> diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
>> index 9fd2f63b8bc0..3c651e0c3113 100644
>> --- a/drivers/crypto/atmel-aes.c
>> +++ b/drivers/crypto/atmel-aes.c
>> @@ -41,6 +41,7 @@
>> #include <linux/platform_data/crypto-atmel.h>
>> #include <dt-bindings/dma/at91.h>
>> #include "atmel-aes-regs.h"
>> +#include "atmel-authenc.h"
>>
>> #define ATMEL_AES_PRIORITY 300
>>
>> @@ -78,6 +79,7 @@
>> #define AES_FLAGS_INIT BIT(2)
>> #define AES_FLAGS_BUSY BIT(3)
>> #define AES_FLAGS_DUMP_REG BIT(4)
>> +#define AES_FLAGS_OWN_SHA BIT(5)
>>
>> #define AES_FLAGS_PERSISTENT (AES_FLAGS_INIT | AES_FLAGS_BUSY)
>>
>> @@ -92,6 +94,7 @@ struct atmel_aes_caps {
>> bool has_ctr32;
>> bool has_gcm;
>> bool has_xts;
>> + bool has_authenc;
>> u32 max_burst_size;
>> };
>>
>> @@ -144,10 +147,31 @@ struct atmel_aes_xts_ctx {
>> u32 key2[AES_KEYSIZE_256 / sizeof(u32)];
>> };
>>
>> +#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
>> +struct atmel_aes_authenc_ctx {
>> + struct atmel_aes_base_ctx base;
>> + struct atmel_sha_authenc_ctx *auth;
>> +};
>> +#endif
>> +
>> struct atmel_aes_reqctx {
>> unsigned long mode;
>> };
>>
>> +#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
>> +struct atmel_aes_authenc_reqctx {
>> + struct atmel_aes_reqctx base;
>> +
>> + struct scatterlist src[2];
>> + struct scatterlist dst[2];
>> + size_t textlen;
>> + u32 digest[SHA512_DIGEST_SIZE / sizeof(u32)];
>> +
>> + /* auth_req MUST be place last. */
>> + struct ahash_request auth_req;
>> +};
>> +#endif
>> +
>> struct atmel_aes_dma {
>> struct dma_chan *chan;
>> struct scatterlist *sg;
>> @@ -291,6 +315,9 @@ static const char *atmel_aes_reg_name(u32 offset, char
>> *tmp, size_t sz) snprintf(tmp, sz, "GCMHR[%u]", (offset - AES_GCMHR(0)) >>
>> 2);
>> break;
>>
>> + case AES_EMR:
>> + return "EMR";
>> +
>> case AES_TWR(0):
>> case AES_TWR(1):
>> case AES_TWR(2):
>> @@ -463,8 +490,16 @@ static inline bool atmel_aes_is_encrypt(const struct
>> atmel_aes_dev *dd) return (dd->flags & AES_FLAGS_ENCRYPT);
>> }
>>
>> +#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
>> +static void atmel_aes_authenc_complete(struct atmel_aes_dev *dd, int err);
>> +#endif
>> +
>> static inline int atmel_aes_complete(struct atmel_aes_dev *dd, int err)
>> {
>> +#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
>> + atmel_aes_authenc_complete(dd, err);
>> +#endif
>> +
>> clk_disable(dd->iclk);
>> dd->flags &= ~AES_FLAGS_BUSY;
>>
>> @@ -1931,6 +1966,407 @@ static struct crypto_alg aes_xts_alg = {
>> }
>> };
>>
>> +#ifdef CONFIG_CRYPTO_DEV_ATMEL_AUTHENC
>> +/* authenc aead functions */
>> +
>> +static int atmel_aes_authenc_start(struct atmel_aes_dev *dd);
>> +static int atmel_aes_authenc_init(struct atmel_aes_dev *dd, int err,
>> + bool is_async);
>> +static int atmel_aes_authenc_transfer(struct atmel_aes_dev *dd, int err,
>> + bool is_async);
>> +static int atmel_aes_authenc_digest(struct atmel_aes_dev *dd);
>> +static int atmel_aes_authenc_final(struct atmel_aes_dev *dd, int err,
>> + bool is_async);
>> +
>> +static void atmel_aes_authenc_complete(struct atmel_aes_dev *dd, int err)
>> +{
>> + struct aead_request *req = aead_request_cast(dd->areq);
>> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
>> +
>> + if (err && (dd->flags & AES_FLAGS_OWN_SHA))
>> + atmel_sha_authenc_abort(&rctx->auth_req);
>> + dd->flags &= ~AES_FLAGS_OWN_SHA;
>> +}
>> +
>> +static int atmel_aes_authenc_copy_assoc(struct aead_request *req)
>> +{
>> + size_t buflen, assoclen = req->assoclen;
>> + off_t skip = 0;
>> + u8 buf[256];
>> +
>> + while (assoclen) {
>> + buflen = min_t(size_t, assoclen, sizeof(buf));
>> +
>> + if (sg_pcopy_to_buffer(req->src, sg_nents(req->src),
>> + buf, buflen, skip) != buflen)
>> + return -EINVAL;
>> +
>> + if (sg_pcopy_from_buffer(req->dst, sg_nents(req->dst),
>> + buf, buflen, skip) != buflen)
>> + return -EINVAL;
>> +
>> + skip += buflen;
>> + assoclen -= buflen;
>> + }
>
> This seems to be a very expansive operation. Wouldn't it be easier, leaner and
> with one less memcpy to use the approach of crypto_authenc_copy_assoc?
>
> Instead of copying crypto_authenc_copy_assoc, what about carving the logic in
> crypto/authenc.c out into a generic aead helper code as we need to add that to
> other AEAD implementations?
Before writing this function, I checked how the crypto/authenc.c driver
handles the copy of the associated data, hence crypto_authenc_copy_assoc().
I have to admit I didn't perform any benchmark to compare the two
implementation but I just tried to understand how
crypto_authenc_copy_assoc() works. At the first look, this function seems
very simple but I guess all the black magic is hidden by the call of
crypto_skcipher_encrypt() on the default null transform, which is
implemented using the ecb(cipher_null) algorithm.
When I wrote my function I thought that this ecb(cipher_null) algorithm was
implemented by combining crypto_ecb_crypt() from crypto/ecb.c with
null_crypt() from crypto/crypto_null.c. Hence I thought there would be much
function call overhead to copy only few bytes but now checking again I
realize that the ecb(cipher_null) algorithm is directly implemented by
skcipher_null_crypt() still from crypto/crypto_null.c. So yes, maybe you're
right: it could be better to reuse what was done in
crypto_authenc_copy_assoc() from crypto/authenc.c.
This way we could need twice less memcpy() hence I agree with you.
>> +
>> + return 0;
>> +}
>> +
>> +static int atmel_aes_authenc_start(struct atmel_aes_dev *dd)
>> +{
>> + struct aead_request *req = aead_request_cast(dd->areq);
>> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
>> + struct crypto_aead *tfm = crypto_aead_reqtfm(req);
>> + struct atmel_aes_authenc_ctx *ctx = crypto_aead_ctx(tfm);
>> + int err;
>> +
>> + atmel_aes_set_mode(dd, &rctx->base);
>> +
>> + err = atmel_aes_hw_init(dd);
>> + if (err)
>> + return atmel_aes_complete(dd, err);
>> +
>> + return atmel_sha_authenc_schedule(&rctx->auth_req, ctx->auth,
>> + atmel_aes_authenc_init, dd);
>> +}
>> +
>> +static int atmel_aes_authenc_init(struct atmel_aes_dev *dd, int err,
>> + bool is_async)
>> +{
>> + struct aead_request *req = aead_request_cast(dd->areq);
>> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
>> +
>> + if (is_async)
>> + dd->is_async = true;
>> + if (err)
>> + return atmel_aes_complete(dd, err);
>> +
>> + /* If here, we've got the ownership of the SHA device. */
>> + dd->flags |= AES_FLAGS_OWN_SHA;
>> +
>> + /* Configure the SHA device. */
>> + return atmel_sha_authenc_init(&rctx->auth_req,
>> + req->src, req->assoclen,
>> + rctx->textlen,
>> + atmel_aes_authenc_transfer, dd);
>> +}
>> +
>> +static int atmel_aes_authenc_transfer(struct atmel_aes_dev *dd, int err,
>> + bool is_async)
>> +{
>> + struct aead_request *req = aead_request_cast(dd->areq);
>> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
>> + bool enc = atmel_aes_is_encrypt(dd);
>> + struct scatterlist *src, *dst;
>> + u32 iv[AES_BLOCK_SIZE / sizeof(u32)];
>> + u32 emr;
>> +
>> + if (is_async)
>> + dd->is_async = true;
>> + if (err)
>> + return atmel_aes_complete(dd, err);
>> +
>> + /* Prepare src and dst scatter-lists to transfer cipher/plain texts. */
>> + src = scatterwalk_ffwd(rctx->src, req->src, req->assoclen);
>> + dst = src;
>> +
>> + if (req->src != req->dst) {
>> + err = atmel_aes_authenc_copy_assoc(req);
>> + if (err)
>> + return atmel_aes_complete(dd, err);
>> +
>> + dst = scatterwalk_ffwd(rctx->dst, req->dst, req->assoclen);
>> + }
>> +
>> + /* Configure the AES device. */
>> + memcpy(iv, req->iv, sizeof(iv));
>> +
>> + /*
>> + * Here we always set the 2nd parameter of atmel_aes_write_ctrl() to
>> + * 'true' even if the data transfer is actually performed by the CPU (so
>> + * not by the DMA) because we must force the AES_MR_SMOD bitfield to the
>> + * value AES_MR_SMOD_IDATAR0. Indeed, both AES_MR_SMOD and SHA_MR_SMOD
>> + * must be set to *_MR_SMOD_IDATAR0.
>> + */
>> + atmel_aes_write_ctrl(dd, true, iv);
>> + emr = AES_EMR_PLIPEN;
>> + if (!enc)
>> + emr |= AES_EMR_PLIPD;
>> + atmel_aes_write(dd, AES_EMR, emr);
>> +
>> + /* Transfer data. */
>> + return atmel_aes_dma_start(dd, src, dst, rctx->textlen,
>> + atmel_aes_authenc_digest);
>> +}
>> +
>> +static int atmel_aes_authenc_digest(struct atmel_aes_dev *dd)
>> +{
>> + struct aead_request *req = aead_request_cast(dd->areq);
>> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
>> +
>> + /* atmel_sha_authenc_final() releases the SHA device. */
>> + dd->flags &= ~AES_FLAGS_OWN_SHA;
>> + return atmel_sha_authenc_final(&rctx->auth_req,
>> + rctx->digest, sizeof(rctx->digest),
>> + atmel_aes_authenc_final, dd);
>> +}
>> +
>> +static int atmel_aes_authenc_final(struct atmel_aes_dev *dd, int err,
>> + bool is_async)
>> +{
>> + struct aead_request *req = aead_request_cast(dd->areq);
>> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
>> + struct crypto_aead *tfm = crypto_aead_reqtfm(req);
>> + bool enc = atmel_aes_is_encrypt(dd);
>> + u32 idigest[SHA512_DIGEST_SIZE / sizeof(u32)], *odigest = rctx->digest;
>> + u32 offs, authsize;
>> +
>> + if (is_async)
>> + dd->is_async = true;
>> + if (err)
>> + goto complete;
>> +
>> + offs = req->assoclen + rctx->textlen;
>> + authsize = crypto_aead_authsize(tfm);
>> + if (enc) {
>> + scatterwalk_map_and_copy(odigest, req->dst, offs, authsize, 1);
>> + } else {
>> + scatterwalk_map_and_copy(idigest, req->src, offs, authsize, 0);
>> + if (crypto_memneq(idigest, odigest, authsize))
>> + err = -EBADMSG;
>> + }
>> +
>> +complete:
>> + return atmel_aes_complete(dd, err);
>> +}
>> +
>> +static int atmel_aes_authenc_setkey(struct crypto_aead *tfm, const u8 *key,
>> + unsigned int keylen)
>> +{
>> + struct atmel_aes_authenc_ctx *ctx = crypto_aead_ctx(tfm);
>> + struct crypto_authenc_keys keys;
>> + u32 flags;
>> + int err;
>> +
>> + if (crypto_authenc_extractkeys(&keys, key, keylen) != 0)
>> + goto badkey;
>> +
>> + if (keys.enckeylen > sizeof(ctx->base.key))
>> + goto badkey;
>> +
>> + /* Save auth key. */
>> + flags = crypto_aead_get_flags(tfm);
>> + err = atmel_sha_authenc_setkey(ctx->auth,
>> + keys.authkey, keys.authkeylen,
>> + &flags);
>> + crypto_aead_set_flags(tfm, flags & CRYPTO_TFM_RES_MASK);
>> + if (err)
>> + return err;
>> +
>> + /* Save enc key. */
>> + ctx->base.keylen = keys.enckeylen;
>> + memcpy(ctx->base.key, keys.enckey, keys.enckeylen);
>
> memzero_explicit(keys) please
good point :)
>> +
>> + return 0;
>> +
>> +badkey:
>> + crypto_aead_set_flags(tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
>> + return -EINVAL;
>> +}
>> +
>> +static int atmel_aes_authenc_init_tfm(struct crypto_aead *tfm,
>> + unsigned long auth_mode)
>> +{
>> + struct atmel_aes_authenc_ctx *ctx = crypto_aead_ctx(tfm);
>> + unsigned int auth_reqsize = atmel_sha_authenc_get_reqsize();
>> +
>> + ctx->auth = atmel_sha_authenc_spawn(auth_mode);
>> + if (IS_ERR(ctx->auth))
>> + return PTR_ERR(ctx->auth);
>> +
>> + crypto_aead_set_reqsize(tfm, (sizeof(struct atmel_aes_authenc_reqctx) +
>> + auth_reqsize));
>> + ctx->base.start = atmel_aes_authenc_start;
>> +
>> + return 0;
>> +}
>> +
>> +static int atmel_aes_authenc_hmac_sha1_init_tfm(struct crypto_aead *tfm)
>> +{
>> + return atmel_aes_authenc_init_tfm(tfm, SHA_FLAGS_HMAC_SHA1);
>> +}
>> +
>> +static int atmel_aes_authenc_hmac_sha224_init_tfm(struct crypto_aead *tfm)
>> +{
>> + return atmel_aes_authenc_init_tfm(tfm, SHA_FLAGS_HMAC_SHA224);
>> +}
>> +
>> +static int atmel_aes_authenc_hmac_sha256_init_tfm(struct crypto_aead *tfm)
>> +{
>> + return atmel_aes_authenc_init_tfm(tfm, SHA_FLAGS_HMAC_SHA256);
>> +}
>> +
>> +static int atmel_aes_authenc_hmac_sha384_init_tfm(struct crypto_aead *tfm)
>> +{
>> + return atmel_aes_authenc_init_tfm(tfm, SHA_FLAGS_HMAC_SHA384);
>> +}
>> +
>> +static int atmel_aes_authenc_hmac_sha512_init_tfm(struct crypto_aead *tfm)
>> +{
>> + return atmel_aes_authenc_init_tfm(tfm, SHA_FLAGS_HMAC_SHA512);
>> +}
>> +
>> +static void atmel_aes_authenc_exit_tfm(struct crypto_aead *tfm)
>> +{
>> + struct atmel_aes_authenc_ctx *ctx = crypto_aead_ctx(tfm);
>> +
>> + atmel_sha_authenc_free(ctx->auth);
>> +}
>> +
>> +static int atmel_aes_authenc_crypt(struct aead_request *req,
>> + unsigned long mode)
>> +{
>> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
>> + struct crypto_aead *tfm = crypto_aead_reqtfm(req);
>> + struct atmel_aes_base_ctx *ctx = crypto_aead_ctx(tfm);
>> + u32 authsize = crypto_aead_authsize(tfm);
>> + bool enc = (mode & AES_FLAGS_ENCRYPT);
>> + struct atmel_aes_dev *dd;
>> +
>> + /* Compute text length. */
>> + rctx->textlen = req->cryptlen - (enc ? 0 : authsize);
>
> Is there somewhere a check that authsize is always < req->cryptlen (at least
> it escaped me)? Note, this logic will be exposed to user space which may do
> funky things.
I thought those 2 sizes were always set by the kernel only but I admit I
didn't check my assumption. If you tell me they could be set directly from
the userspace, yes I agree with you, I need to add a test.
>> +
>> + /*
>> + * Currently, empty messages are not supported yet:
>> + * the SHA auto-padding can be used only on non-empty messages.
>> + * Hence a special case needs to be implemented for empty message.
>> + */
>> + if (!rctx->textlen && !req->assoclen)
>> + return -EINVAL;
>> +
>> + rctx->base.mode = mode;
>> + ctx->block_size = AES_BLOCK_SIZE;
>> +
>> + dd = atmel_aes_find_dev(ctx);
>> + if (!dd)
>> + return -ENODEV;
>> +
>> + return atmel_aes_handle_queue(dd, &req->base);
>
> Ciao
> Stephan
>
thanks for your review! :)
Best regards,
Cyrille
^ permalink raw reply
* Re: [PATCH v2 11/12] crypto: atmel-authenc: add support to authenc(hmac(shaX),Y(aes)) modes
From: Stephan Müller @ 2017-01-09 18:34 UTC (permalink / raw)
To: Cyrille Pitchen
Cc: herbert, davem, nicolas.ferre, linux-crypto, linux-kernel,
linux-arm-kernel
In-Reply-To: <6301d79c-f1c5-d86c-823c-dfdbb5100e74@atmel.com>
Am Montag, 9. Januar 2017, 19:24:12 CET schrieb Cyrille Pitchen:
Hi Cyrille,
> >> +static int atmel_aes_authenc_copy_assoc(struct aead_request *req)
> >> +{
> >> + size_t buflen, assoclen = req->assoclen;
> >> + off_t skip = 0;
> >> + u8 buf[256];
> >> +
> >> + while (assoclen) {
> >> + buflen = min_t(size_t, assoclen, sizeof(buf));
> >> +
> >> + if (sg_pcopy_to_buffer(req->src, sg_nents(req->src),
> >> + buf, buflen, skip) != buflen)
> >> + return -EINVAL;
> >> +
> >> + if (sg_pcopy_from_buffer(req->dst, sg_nents(req->dst),
> >> + buf, buflen, skip) != buflen)
> >> + return -EINVAL;
> >> +
> >> + skip += buflen;
> >> + assoclen -= buflen;
> >> + }
> >
> > This seems to be a very expansive operation. Wouldn't it be easier, leaner
> > and with one less memcpy to use the approach of
> > crypto_authenc_copy_assoc?
> >
> > Instead of copying crypto_authenc_copy_assoc, what about carving the logic
> > in crypto/authenc.c out into a generic aead helper code as we need to add
> > that to other AEAD implementations?
>
> Before writing this function, I checked how the crypto/authenc.c driver
> handles the copy of the associated data, hence crypto_authenc_copy_assoc().
>
> I have to admit I didn't perform any benchmark to compare the two
> implementation but I just tried to understand how
> crypto_authenc_copy_assoc() works. At the first look, this function seems
> very simple but I guess all the black magic is hidden by the call of
> crypto_skcipher_encrypt() on the default null transform, which is
> implemented using the ecb(cipher_null) algorithm.
The magic in the null cipher is that it not only performs a memcpy, but
iterates through the SGL and performs a memcpy on each part of the source/
destination SGL.
I will release a patch set later today -- the coding is completed, but testing
is yet under way. That patch now allows you to make only one function call
without special init/deinit code.
>
> When I wrote my function I thought that this ecb(cipher_null) algorithm was
> implemented by combining crypto_ecb_crypt() from crypto/ecb.c with
> null_crypt() from crypto/crypto_null.c. Hence I thought there would be much
> function call overhead to copy only few bytes but now checking again I
> realize that the ecb(cipher_null) algorithm is directly implemented by
> skcipher_null_crypt() still from crypto/crypto_null.c. So yes, maybe you're
> right: it could be better to reuse what was done in
> crypto_authenc_copy_assoc() from crypto/authenc.c.
>
> This way we could need twice less memcpy() hence I agree with you.
In addition to the additional memcpy, the patch I want to air shortly (and
which I hope is going to be accepted) should reduce the complexity of your
code in this corner.
...
> >> +static int atmel_aes_authenc_crypt(struct aead_request *req,
> >> + unsigned long mode)
> >> +{
> >> + struct atmel_aes_authenc_reqctx *rctx = aead_request_ctx(req);
> >> + struct crypto_aead *tfm = crypto_aead_reqtfm(req);
> >> + struct atmel_aes_base_ctx *ctx = crypto_aead_ctx(tfm);
> >> + u32 authsize = crypto_aead_authsize(tfm);
> >> + bool enc = (mode & AES_FLAGS_ENCRYPT);
> >> + struct atmel_aes_dev *dd;
> >> +
> >> + /* Compute text length. */
> >> + rctx->textlen = req->cryptlen - (enc ? 0 : authsize);
> >
> > Is there somewhere a check that authsize is always < req->cryptlen (at
> > least it escaped me)? Note, this logic will be exposed to user space
> > which may do funky things.
>
> I thought those 2 sizes were always set by the kernel only but I admit I
> didn't check my assumption. If you tell me they could be set directly from
> the userspace, yes I agree with you, I need to add a test.
Then I would like to ask you adding that check -- as this check is cheap, it
should not affect performance.
Ciao
Stephan
^ permalink raw reply
* [PATCH 13/13] crypto: qat - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:41 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
drivers/crypto/qat/qat_common/qat_algs.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/crypto/qat/qat_common/qat_algs.c b/drivers/crypto/qat/qat_common/qat_algs.c
index 20f35df..6576328 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -865,6 +865,10 @@ static int qat_alg_aead_enc(struct aead_request *areq)
uint8_t *iv = areq->iv;
int ret, ctr = 0;
+ ret = crypto_aead_copy_ad(areq);
+ if (ret)
+ return ret;
+
ret = qat_alg_sgl_to_bufl(ctx->inst, areq->src, areq->dst, qat_req);
if (unlikely(ret))
return ret;
--
2.9.3
^ permalink raw reply related
* [PATCH 12/13] crypto: nx - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:40 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
drivers/crypto/nx/nx-aes-ccm.c | 4 ++++
drivers/crypto/nx/nx-aes-gcm.c | 10 ++++++++++
2 files changed, 14 insertions(+)
diff --git a/drivers/crypto/nx/nx-aes-ccm.c b/drivers/crypto/nx/nx-aes-ccm.c
index 7038f36..ee570bf 100644
--- a/drivers/crypto/nx/nx-aes-ccm.c
+++ b/drivers/crypto/nx/nx-aes-ccm.c
@@ -428,6 +428,10 @@ static int ccm_nx_encrypt(struct aead_request *req,
unsigned int processed = 0, to_process;
int rc = -1;
+ rc = crypto_aead_copy_ad(req);
+ if (rc)
+ return rc;
+
spin_lock_irqsave(&nx_ctx->lock, irq_flags);
rc = generate_pat(desc->info, req, nx_ctx, authsize, nbytes, assoclen,
diff --git a/drivers/crypto/nx/nx-aes-gcm.c b/drivers/crypto/nx/nx-aes-gcm.c
index abd465f..0cc0533 100644
--- a/drivers/crypto/nx/nx-aes-gcm.c
+++ b/drivers/crypto/nx/nx-aes-gcm.c
@@ -432,9 +432,14 @@ static int gcm_aes_nx_encrypt(struct aead_request *req)
{
struct nx_gcm_rctx *rctx = aead_request_ctx(req);
char *iv = rctx->iv;
+ int err;
memcpy(iv, req->iv, 12);
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
return gcm_aes_nx_crypt(req, 1, req->assoclen);
}
@@ -455,6 +460,7 @@ static int gcm4106_aes_nx_encrypt(struct aead_request *req)
struct nx_gcm_rctx *rctx = aead_request_ctx(req);
char *iv = rctx->iv;
char *nonce = nx_ctx->priv.gcm.nonce;
+ int err;
memcpy(iv, nonce, NX_GCM4106_NONCE_LEN);
memcpy(iv + NX_GCM4106_NONCE_LEN, req->iv, 8);
@@ -462,6 +468,10 @@ static int gcm4106_aes_nx_encrypt(struct aead_request *req)
if (req->assoclen < 8)
return -EINVAL;
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
return gcm_aes_nx_crypt(req, 1, req->assoclen - 8);
}
--
2.9.3
^ permalink raw reply related
* [PATCH 11/13] crypto: chelsio - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:40 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
drivers/crypto/chelsio/chcr_algo.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/crypto/chelsio/chcr_algo.c b/drivers/crypto/chelsio/chcr_algo.c
index 2ed1e24..b3283c0 100644
--- a/drivers/crypto/chelsio/chcr_algo.c
+++ b/drivers/crypto/chelsio/chcr_algo.c
@@ -2398,6 +2398,11 @@ static int chcr_aead_encrypt(struct aead_request *req)
{
struct crypto_aead *tfm = crypto_aead_reqtfm(req);
struct chcr_aead_reqctx *reqctx = aead_request_ctx(req);
+ int err;
+
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
reqctx->verify = VERIFY_HW;
--
2.9.3
^ permalink raw reply related
* [PATCH 10/13] crypto: caam - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:39 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
drivers/crypto/caam/caamalg.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 662fe94..30ad943 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -1433,6 +1433,10 @@ static int gcm_encrypt(struct aead_request *req)
u32 *desc;
int ret = 0;
+ ret = crypto_aead_copy_ad(req);
+ if (ret)
+ return ret;
+
/* allocate extended descriptor */
edesc = aead_edesc_alloc(req, GCM_DESC_JOB_IO_LEN, &all_contig, true);
if (IS_ERR(edesc))
@@ -1476,6 +1480,10 @@ static int aead_encrypt(struct aead_request *req)
u32 *desc;
int ret = 0;
+ ret = crypto_aead_copy_ad(req);
+ if (ret)
+ return ret;
+
/* allocate extended descriptor */
edesc = aead_edesc_alloc(req, AUTHENC_DESC_JOB_IO_LEN,
&all_contig, true);
--
2.9.3
^ permalink raw reply related
* [PATCH 09/13] crypto: atmel - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:39 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
drivers/crypto/atmel-aes.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/crypto/atmel-aes.c b/drivers/crypto/atmel-aes.c
index 0e3d0d6..48ecf72 100644
--- a/drivers/crypto/atmel-aes.c
+++ b/drivers/crypto/atmel-aes.c
@@ -1752,6 +1752,12 @@ static int atmel_aes_gcm_setauthsize(struct crypto_aead *tfm,
static int atmel_aes_gcm_encrypt(struct aead_request *req)
{
+ int err;
+
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
return atmel_aes_gcm_crypt(req, AES_FLAGS_ENCRYPT);
}
--
2.9.3
^ permalink raw reply related
* [PATCH 08/13] crypto: ixp4xx - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:39 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
drivers/crypto/ixp4xx_crypto.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index 7868765..20a5bd8 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -1178,6 +1178,12 @@ static int aead_setkey(struct crypto_aead *tfm, const u8 *key,
static int aead_encrypt(struct aead_request *req)
{
+ int err;
+
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
return aead_perform(req, 1, req->assoclen, req->cryptlen, req->iv);
}
--
2.9.3
^ permalink raw reply related
* [PATCH 07/13] crypto: picoxcell - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:38 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
drivers/crypto/picoxcell_crypto.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/crypto/picoxcell_crypto.c b/drivers/crypto/picoxcell_crypto.c
index 4757609..ef955d3 100644
--- a/drivers/crypto/picoxcell_crypto.c
+++ b/drivers/crypto/picoxcell_crypto.c
@@ -688,6 +688,11 @@ static int spacc_aead_encrypt(struct aead_request *req)
{
struct crypto_aead *aead = crypto_aead_reqtfm(req);
struct spacc_aead *alg = to_spacc_aead(crypto_aead_alg(aead));
+ int err;
+
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
return spacc_aead_setup(req, alg->type, 1);
}
--
2.9.3
^ permalink raw reply related
* [PATCH 06/13] crypto: talitos - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:38 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
drivers/crypto/talitos.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 0bba6a1..6a6e343 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1429,6 +1429,11 @@ static int aead_encrypt(struct aead_request *req)
struct crypto_aead *authenc = crypto_aead_reqtfm(req);
struct talitos_ctx *ctx = crypto_aead_ctx(authenc);
struct talitos_edesc *edesc;
+ int err;
+
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
/* allocate extended descriptor */
edesc = aead_edesc_alloc(req, req->iv, 0, true);
--
2.9.3
^ permalink raw reply related
* [PATCH 05/13] crypto: ccm-aes-ce - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:38 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
>From acad5f6af68d18bc1dbac460bd44f2298902018b Mon Sep 17 00:00:00 2001
From: Stephan Mueller <smueller@chronox.de>
Date: Mon, 9 Jan 2017 14:32:02 +0100
Subject:
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
arch/arm64/crypto/aes-ce-ccm-glue.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/crypto/aes-ce-ccm-glue.c b/arch/arm64/crypto/aes-ce-ccm-glue.c
index cc5515d..7dfcb1f 100644
--- a/arch/arm64/crypto/aes-ce-ccm-glue.c
+++ b/arch/arm64/crypto/aes-ce-ccm-glue.c
@@ -155,6 +155,10 @@ static int ccm_encrypt(struct aead_request *req)
u32 len = req->cryptlen;
int err;
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
err = ccm_init_mac(req, mac, len);
if (err)
return err;
--
2.9.3
^ permalink raw reply related
* [PATCH 04/13] crypto: rfc4106-gcm-aesni - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:37 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
arch/x86/crypto/aesni-intel_glue.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 36ca150..6149018 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -882,6 +882,7 @@ static int rfc4106_encrypt(struct aead_request *req)
struct crypto_aead *tfm = crypto_aead_reqtfm(req);
struct cryptd_aead **ctx = crypto_aead_ctx(tfm);
struct cryptd_aead *cryptd_tfm = *ctx;
+ int err;
tfm = &cryptd_tfm->base;
if (irq_fpu_usable() && (!in_atomic() ||
@@ -890,6 +891,10 @@ static int rfc4106_encrypt(struct aead_request *req)
aead_request_set_tfm(req, tfm);
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
return crypto_aead_encrypt(req);
}
--
2.9.3
^ permalink raw reply related
* [PATCH 03/13] crypto: ccm_generic - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:37 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
crypto/ccm.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/crypto/ccm.c b/crypto/ccm.c
index 26b924d..e2b6d3d 100644
--- a/crypto/ccm.c
+++ b/crypto/ccm.c
@@ -358,6 +358,10 @@ static int crypto_ccm_encrypt(struct aead_request *req)
if (err)
return err;
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
err = crypto_ccm_auth(req, sg_next(pctx->src), cryptlen);
if (err)
return err;
@@ -749,9 +753,15 @@ static struct aead_request *crypto_rfc4309_crypt(struct aead_request *req)
static int crypto_rfc4309_encrypt(struct aead_request *req)
{
+ int err;
+
if (req->assoclen != 16 && req->assoclen != 20)
return -EINVAL;
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
req = crypto_rfc4309_crypt(req);
return crypto_aead_encrypt(req);
--
2.9.3
^ permalink raw reply related
* [PATCH 02/13] crypto: gcm_generic - copy AAD during encryption
From: Stephan Müller @ 2017-01-10 1:37 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
>From 8690e52f7e90663b14b2490d37a3b5b1c5a53bfe Mon Sep 17 00:00:00 2001
From: Stephan Mueller <smueller@chronox.de>
Date: Mon, 9 Jan 2017 12:35:35 +0100
Subject:
Invoke the crypto_aead_copy_ad function during the encryption code path
to copy the AAD from the source to the destination buffer.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
crypto/gcm.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/crypto/gcm.c b/crypto/gcm.c
index b7ad808..1ca0b05c 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -496,11 +496,16 @@ static int crypto_gcm_encrypt(struct aead_request *req)
struct crypto_gcm_req_priv_ctx *pctx = crypto_gcm_reqctx(req);
struct skcipher_request *skreq = &pctx->u.skreq;
u32 flags = aead_request_flags(req);
+ int err;
crypto_gcm_init_common(req);
crypto_gcm_init_crypt(req, req->cryptlen);
skcipher_request_set_callback(skreq, flags, gcm_encrypt_done, req);
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
return crypto_skcipher_encrypt(skreq) ?:
gcm_encrypt_continue(req, flags);
}
@@ -866,9 +871,15 @@ static struct aead_request *crypto_rfc4106_crypt(struct aead_request *req)
static int crypto_rfc4106_encrypt(struct aead_request *req)
{
+ int err;
+
if (req->assoclen != 16 && req->assoclen != 20)
return -EINVAL;
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
req = crypto_rfc4106_crypt(req);
return crypto_aead_encrypt(req);
@@ -1099,6 +1110,12 @@ static int crypto_rfc4543_copy_src_to_dst(struct aead_request *req, bool enc)
static int crypto_rfc4543_encrypt(struct aead_request *req)
{
+ int err;
+
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+
return crypto_rfc4543_crypt(req, true);
}
--
2.9.3
^ permalink raw reply related
* [PATCH 01/13] crypto: service function to copy AAD from src to dst
From: Stephan Müller @ 2017-01-10 1:36 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
In-Reply-To: <10526995.lyZ7Je1KMx@positron.chronox.de>
The service function crypto_aead_copy_ad uses the null cipher to copy
the AAD from teh source to the destination SGL. The copy operation is
prevented when the source and destination SGL is identical.
The required null cipher is allocated during the allocation of the TFM
and released with the TFM. Therefore, a use of the helper function only
requires adding an invocation of this function in the encrypt code path
shortly before the encrypt operation is invoked but after the SGLs are
set with the AEAD request.
The patch converts the authenc implementation to use this service
function instead of its own copy logic.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
crypto/Kconfig | 4 ++--
crypto/aead.c | 36 ++++++++++++++++++++++++++++++++++--
crypto/authenc.c | 36 ++++--------------------------------
include/crypto/aead.h | 2 ++
include/crypto/internal/aead.h | 12 ++++++++++++
5 files changed, 54 insertions(+), 36 deletions(-)
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 160f08e..f53928a 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -44,6 +44,8 @@ config CRYPTO_AEAD
tristate
select CRYPTO_AEAD2
select CRYPTO_ALGAPI
+ select CRYPTO_BLKCIPHER
+ select CRYPTO_NULL
config CRYPTO_AEAD2
tristate
@@ -227,10 +229,8 @@ config CRYPTO_MCRYPTD
config CRYPTO_AUTHENC
tristate "Authenc support"
select CRYPTO_AEAD
- select CRYPTO_BLKCIPHER
select CRYPTO_MANAGER
select CRYPTO_HASH
- select CRYPTO_NULL
help
Authenc: Combined mode wrapper for IPsec.
This is required for IPSec.
diff --git a/crypto/aead.c b/crypto/aead.c
index 3f5c5ff..e9291e6 100644
--- a/crypto/aead.c
+++ b/crypto/aead.c
@@ -79,11 +79,31 @@ int crypto_aead_setauthsize(struct crypto_aead *tfm, unsigned int authsize)
}
EXPORT_SYMBOL_GPL(crypto_aead_setauthsize);
+int crypto_aead_copy_ad(struct aead_request *req)
+{
+ struct crypto_aead *aead = crypto_aead_reqtfm(req);
+ SKCIPHER_REQUEST_ON_STACK(skreq, aead->null);
+
+ /* No copy operation if src and dst are identical. */
+ if (req->src == req->dst)
+ return 0;
+
+ skcipher_request_set_tfm(skreq, aead->null);
+ skcipher_request_set_callback(skreq, aead_request_flags(req),
+ NULL, NULL);
+ skcipher_request_set_crypt(skreq, req->src, req->dst, req->assoclen,
+ NULL);
+
+ return crypto_skcipher_encrypt(skreq);
+}
+EXPORT_SYMBOL_GPL(crypto_aead_copy_ad);
+
static void crypto_aead_exit_tfm(struct crypto_tfm *tfm)
{
struct crypto_aead *aead = __crypto_aead_cast(tfm);
struct aead_alg *alg = crypto_aead_alg(aead);
+ crypto_put_default_null_skcipher2();
alg->exit(aead);
}
@@ -91,14 +111,26 @@ static int crypto_aead_init_tfm(struct crypto_tfm *tfm)
{
struct crypto_aead *aead = __crypto_aead_cast(tfm);
struct aead_alg *alg = crypto_aead_alg(aead);
+ struct crypto_skcipher *null;
+ int err;
aead->authsize = alg->maxauthsize;
+ null = crypto_get_default_null_skcipher2();
+ err = PTR_ERR(null);
+ if (IS_ERR(null))
+ return err;
+ aead->null = null;
+
if (alg->exit)
aead->base.exit = crypto_aead_exit_tfm;
- if (alg->init)
- return alg->init(aead);
+ if (alg->init) {
+ err = alg->init(aead);
+ if (err)
+ crypto_put_default_null_skcipher2();
+ return err;
+ }
return 0;
}
diff --git a/crypto/authenc.c b/crypto/authenc.c
index 875470b..a7a304a 100644
--- a/crypto/authenc.c
+++ b/crypto/authenc.c
@@ -14,7 +14,6 @@
#include <crypto/internal/hash.h>
#include <crypto/internal/skcipher.h>
#include <crypto/authenc.h>
-#include <crypto/null.h>
#include <crypto/scatterwalk.h>
#include <linux/err.h>
#include <linux/init.h>
@@ -33,7 +32,6 @@ struct authenc_instance_ctx {
struct crypto_authenc_ctx {
struct crypto_ahash *auth;
struct crypto_skcipher *enc;
- struct crypto_skcipher *null;
};
struct authenc_request_ctx {
@@ -180,21 +178,6 @@ static void crypto_authenc_encrypt_done(struct crypto_async_request *req,
authenc_request_complete(areq, err);
}
-static int crypto_authenc_copy_assoc(struct aead_request *req)
-{
- struct crypto_aead *authenc = crypto_aead_reqtfm(req);
- struct crypto_authenc_ctx *ctx = crypto_aead_ctx(authenc);
- SKCIPHER_REQUEST_ON_STACK(skreq, ctx->null);
-
- skcipher_request_set_tfm(skreq, ctx->null);
- skcipher_request_set_callback(skreq, aead_request_flags(req),
- NULL, NULL);
- skcipher_request_set_crypt(skreq, req->src, req->dst, req->assoclen,
- NULL);
-
- return crypto_skcipher_encrypt(skreq);
-}
-
static int crypto_authenc_encrypt(struct aead_request *req)
{
struct crypto_aead *authenc = crypto_aead_reqtfm(req);
@@ -212,13 +195,12 @@ static int crypto_authenc_encrypt(struct aead_request *req)
src = scatterwalk_ffwd(areq_ctx->src, req->src, req->assoclen);
dst = src;
- if (req->src != req->dst) {
- err = crypto_authenc_copy_assoc(req);
- if (err)
- return err;
+ err = crypto_aead_copy_ad(req);
+ if (err)
+ return err;
+ if (req->src != req->dst)
dst = scatterwalk_ffwd(areq_ctx->dst, req->dst, req->assoclen);
- }
skcipher_request_set_tfm(skreq, enc);
skcipher_request_set_callback(skreq, aead_request_flags(req),
@@ -317,7 +299,6 @@ static int crypto_authenc_init_tfm(struct crypto_aead *tfm)
struct crypto_authenc_ctx *ctx = crypto_aead_ctx(tfm);
struct crypto_ahash *auth;
struct crypto_skcipher *enc;
- struct crypto_skcipher *null;
int err;
auth = crypto_spawn_ahash(&ictx->auth);
@@ -329,14 +310,8 @@ static int crypto_authenc_init_tfm(struct crypto_aead *tfm)
if (IS_ERR(enc))
goto err_free_ahash;
- null = crypto_get_default_null_skcipher2();
- err = PTR_ERR(null);
- if (IS_ERR(null))
- goto err_free_skcipher;
-
ctx->auth = auth;
ctx->enc = enc;
- ctx->null = null;
crypto_aead_set_reqsize(
tfm,
@@ -350,8 +325,6 @@ static int crypto_authenc_init_tfm(struct crypto_aead *tfm)
return 0;
-err_free_skcipher:
- crypto_free_skcipher(enc);
err_free_ahash:
crypto_free_ahash(auth);
return err;
@@ -363,7 +336,6 @@ static void crypto_authenc_exit_tfm(struct crypto_aead *tfm)
crypto_free_ahash(ctx->auth);
crypto_free_skcipher(ctx->enc);
- crypto_put_default_null_skcipher2();
}
static void crypto_authenc_free(struct aead_instance *inst)
diff --git a/include/crypto/aead.h b/include/crypto/aead.h
index 03b9762..9872f92 100644
--- a/include/crypto/aead.h
+++ b/include/crypto/aead.h
@@ -16,6 +16,7 @@
#include <linux/crypto.h>
#include <linux/kernel.h>
#include <linux/slab.h>
+#include <crypto/skcipher.h>
/**
* DOC: Authenticated Encryption With Associated Data (AEAD) Cipher API
@@ -155,6 +156,7 @@ struct crypto_aead {
unsigned int authsize;
unsigned int reqsize;
+ struct crypto_skcipher *null;
struct crypto_tfm base;
};
diff --git a/include/crypto/internal/aead.h b/include/crypto/internal/aead.h
index 6ad8e31..01050c0 100644
--- a/include/crypto/internal/aead.h
+++ b/include/crypto/internal/aead.h
@@ -187,5 +187,17 @@ void crypto_unregister_aeads(struct aead_alg *algs, int count);
int aead_register_instance(struct crypto_template *tmpl,
struct aead_instance *inst);
+/**
+ * crypto_aead_copy_ad - copy the AAD from src to dst buffers
+ * @req: AEAD cipher request
+ *
+ * This function is intended for the encrypt operation only as the ciphertext
+ * should be accompanied by the AAD. The copy operation is performed with
+ * the null cipher that is allocated during initialization of the AEAD TFM.
+ *
+ * Return: 0 upon success, < 0 in case of error
+ */
+int crypto_aead_copy_ad(struct aead_request *req);
+
#endif /* _CRYPTO_INTERNAL_AEAD_H */
--
2.9.3
^ permalink raw reply related
* [PATCH 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Stephan Müller @ 2017-01-10 1:36 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
Hi,
to all driver maintainers: the patches I added are compile tested, but
I do not have the hardware to verify the code. May I ask the respective
hardware maintainers to verify that the code is appropriate and works
as intended? Thanks a lot.
Herbert, this is my proprosal for our discussion around copying the
AAD for algif_aead. Instead of adding the code to algif_aead and wait
until it transpires to all cipher implementations, I thought it would
be more helpful to fix all cipher implementations.
To do so, the AAD copy function found in authenc is extracted to a global
service function. Furthermore, the generic AEAD TFM initialization code
now allocates the null cipher too. This allows us now to only invoke
the AAD copy function in the various implementations without any additional
allocation logic.
The code for x86 and the generic code was tested with libkcapi.
The code for the drivers is compile tested for drivers applicable to
x86 only. All others are neither compile tested nor functionally tested.
Stephan Mueller (13):
crypto: service function to copy AAD from src to dst
crypto: gcm_generic - copy AAD during encryption
crypto: ccm_generic - copy AAD during encryption
crypto: rfc4106-gcm-aesni - copy AAD during encryption
crypto: ccm-aes-ce - copy AAD during encryption
crypto: talitos - copy AAD during encryption
crypto: picoxcell - copy AAD during encryption
crypto: ixp4xx - copy AAD during encryption
crypto: atmel - copy AAD during encryption
crypto: caam - copy AAD during encryption
crypto: chelsio - copy AAD during encryption
crypto: nx - copy AAD during encryption
crypto: qat - copy AAD during encryption
arch/arm64/crypto/aes-ce-ccm-glue.c | 4 ++++
arch/x86/crypto/aesni-intel_glue.c | 5 +++++
crypto/Kconfig | 4 ++--
crypto/aead.c | 36 ++++++++++++++++++++++++++++++--
crypto/authenc.c | 36 ++++----------------------------
crypto/ccm.c | 10 +++++++++
crypto/gcm.c | 17 +++++++++++++++
drivers/crypto/atmel-aes.c | 6 ++++++
drivers/crypto/caam/caamalg.c | 8 +++++++
drivers/crypto/chelsio/chcr_algo.c | 5 +++++
drivers/crypto/ixp4xx_crypto.c | 6 ++++++
drivers/crypto/nx/nx-aes-ccm.c | 4 ++++
drivers/crypto/nx/nx-aes-gcm.c | 10 +++++++++
drivers/crypto/picoxcell_crypto.c | 5 +++++
drivers/crypto/qat/qat_common/qat_algs.c | 4 ++++
drivers/crypto/talitos.c | 5 +++++
include/crypto/aead.h | 2 ++
include/crypto/internal/aead.h | 12 +++++++++++
18 files changed, 143 insertions(+), 36 deletions(-)
--
2.9.3
^ permalink raw reply
* Re: [PATCH v2 1/4] lib: Update LZ4 compressor module based on LZ4 v1.7.2.
From: Sven Schmidt @ 2017-01-10 9:21 UTC (permalink / raw)
To: gregkh
Cc: akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky, linux-kernel,
herbert, davem, linux-crypto, anton, ccross, keescook, tony.luck,
phillip, Sven Schmidt
In-Reply-To: <20170108112542.GC12798@kroah.com>
On 01/08/2017 12:25 PM, Greg KH wrote:
>On Sat, Jan 07, 2017 at 05:55:42PM +0100, Sven Schmidt wrote:
>> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
>> The kernel module is inspired by the previous work by Chanho Min.
>> The updated LZ4 module will not break existing code since there were alias
>> methods added to ensure backwards compatibility.
>
> Meta-comment. Does this update include all of the security fixes that
> we have made over the past few years to the lz4 code? I don't want to
> be adding back insecure functions that will cause us problems.
>
> Specifically look at the changes I made in 2014 in this directory for an
> example of what I am talking about here.
>
Hi Greg,
it doesn't. I didn't have that in mind until now.
But I had a look at the related commits after I received your E-Mail and will review what I have to change
(and, obviously, do the changes :)).
Thanks,
Sven
^ permalink raw reply
* Re: [PATCH v2 1/4] lib: Update LZ4 compressor module based on LZ4 v1.7.2.
From: Sven Schmidt @ 2017-01-10 9:32 UTC (permalink / raw)
To: gregkh
Cc: akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky, linux-kernel,
herbert, davem, linux-crypto, anton, ccross, keescook, tony.luck,
phillip, Sven Schmidt
In-Reply-To: <20170108112223.GA12798@kroah.com>
On 01/08/2017 12:23 PM, Greg KH wrote:
> On Sat, Jan 07, 2017 at 05:55:43PM +0100, Sven Schmidt wrote:
>> This patch updates the unlz4 wrapper to work with the new LZ4 kernel module version.
>>
>> Signed-off-by: Sven Schmidt <4sschmid@informatik.uni-hamburg.de>
>> ---
>> lib/decompress_unlz4.c | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/decompress_unlz4.c b/lib/decompress_unlz4.c
>> index 036fc88..1b0baf3 100644
>> --- a/lib/decompress_unlz4.c
>> +++ b/lib/decompress_unlz4.c
>> @@ -72,7 +72,7 @@ STATIC inline int INIT unlz4(u8 *input, long in_len,
>> error("NULL input pointer and missing fill function");
>> goto exit_1;
>> } else {
>> - inp = large_malloc(lz4_compressbound(uncomp_chunksize));
>> + inp = large_malloc(LZ4_compressBound(uncomp_chunksize));
>
> Having functions differ by different cases of the characters is ripe for
> abuse and confusion. Please never do that, especially as these "new"
> functions you created don't follow the correct kernel coding style rules
> :(
>
> thanks,
>
> greg k-h
Hi Greg,
you're right about that. I think I put a little too much effort in keeping old function names here.
I will get rid of that.
I also want to quote your previous E-Mail here:
> And follow the proper kernel coding style rules, putting your patches
> through scripts/checkpatch.pl should help you out here.
Sorry, I didn't know about that particular script. I already put Patches 2, 3 and 4 through checkpatch.pl;
patch 1 is a little more to rework but I will, of course, do that.
Thanks,
Sven
^ permalink raw reply
* Re: [PATCH v2 4/4] fs/pstore: fs/squashfs: Change usage of LZ4 to comply with new LZ4 module version
From: Sven Schmidt @ 2017-01-10 9:45 UTC (permalink / raw)
To: keescook
Cc: gregkh, akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky,
linux-kernel, herbert, davem, linux-crypto, anton, ccross,
tony.luck, phillip, Sven Schmidt
In-Reply-To: <CAGXu5jKqMUMU8DbnE=AamrCp_-JN7sGRCpaGMps16X_TshfK+Q@mail.gmail.com>
Hi Kees,
On 01/07/2017 10:33 PM, Kees Cook wrote:
>On Sat, Jan 7, 2017 at 8:55 AM, Sven Schmidt
><4sschmid@informatik.uni-hamburg.de> wrote:
>> This patch updates fs/pstore and fs/squashfs to use the updated functions from
>> the new LZ4 module.
>>
>> Signed-off-by: Sven Schmidt <4sschmid@informatik.uni-hamburg.de>
>> ---
>> fs/pstore/platform.c | 14 ++++++++------
>> fs/squashfs/lz4_wrapper.c | 12 ++++++------
>> 2 files changed, 14 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
>> index 729677e..a0d8ca8 100644
>> --- a/fs/pstore/platform.c
>> +++ b/fs/pstore/platform.c
>> @@ -342,31 +342,33 @@ static int compress_lz4(const void *in, void *out, size_t inlen, size_t outlen)
>> {
>> int ret;
>>
>> - ret = lz4_compress(in, inlen, out, &outlen, workspace);
>> + ret = LZ4_compress_default(in, out, inlen, outlen, workspace);
>> if (ret) {
>> + // ret is 0 means an error occured
>
>If that's true, then shouldn't the "if" logic be changed? Also, here
>and in all following comments are C++ style instead of kernel C-style.
>This should be "/* ret == 0 means an error occured */", though really,
>that should be obvious from the code and the comment isn't really
>needed.
indeed, it should. I fixed that one.
>> pr_err("lz4_compress error, ret = %d!\n", ret);
>If it's always going to be zero here, is there a better place to get
>details on why it failed?
It is always going to be zero. Honestly, after looking at the current LZ4 in kernel again
I don't get why there actually was a need to print the return value since lz4_compress
will (as far as I see) always return -1 in case of an error while the new lz4_compress_fast/default
will return 0 in such case. Maybe I should just stick with the error?
Thanks,
Sven
^ permalink raw reply
* Re: [PATCH v2 1/4] lib: Update LZ4 compressor module based on LZ4 v1.7.2.
From: Greg KH @ 2017-01-10 9:59 UTC (permalink / raw)
To: Sven Schmidt
Cc: akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky, linux-kernel,
herbert, davem, linux-crypto, anton, ccross, keescook, tony.luck,
phillip
In-Reply-To: <1484040737-5400-1-git-send-email-4sschmid@informatik.uni-hamburg.de>
On Tue, Jan 10, 2017 at 10:32:17AM +0100, Sven Schmidt wrote:
> On 01/08/2017 12:23 PM, Greg KH wrote:
> > And follow the proper kernel coding style rules, putting your patches
> > through scripts/checkpatch.pl should help you out here.
>
> Sorry, I didn't know about that particular script. I already put
> Patches 2, 3 and 4 through checkpatch.pl; patch 1 is a little more to
> rework but I will, of course, do that.
You do know about Documentation/SubmittingPatches, right? If not, take
a look at that as well :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v2 1/4] lib: Update LZ4 compressor module based on LZ4 v1.7.2.
From: Greg KH @ 2017-01-10 10:00 UTC (permalink / raw)
To: Sven Schmidt
Cc: akpm, bongkyu.kim, rsalvaterra, sergey.senozhatsky, linux-kernel,
herbert, davem, linux-crypto, anton, ccross, keescook, tony.luck,
phillip
In-Reply-To: <1484040076-5004-1-git-send-email-4sschmid@informatik.uni-hamburg.de>
On Tue, Jan 10, 2017 at 10:21:16AM +0100, Sven Schmidt wrote:
> On 01/08/2017 12:25 PM, Greg KH wrote:
> >On Sat, Jan 07, 2017 at 05:55:42PM +0100, Sven Schmidt wrote:
> >> This patch updates LZ4 kernel module to LZ4 v1.7.2 by Yann Collet.
> >> The kernel module is inspired by the previous work by Chanho Min.
> >> The updated LZ4 module will not break existing code since there were alias
> >> methods added to ensure backwards compatibility.
> >
> > Meta-comment. Does this update include all of the security fixes that
> > we have made over the past few years to the lz4 code? I don't want to
> > be adding back insecure functions that will cause us problems.
> >
> > Specifically look at the changes I made in 2014 in this directory for an
> > example of what I am talking about here.
> >
>
> Hi Greg,
>
> it doesn't. I didn't have that in mind until now.
Ick, those changes never got made "upstream"? Not good, but makes sense
as we couldn't really find an "upstream" when we made them :(
As you took this code from somewhere, you might want to also push your
changes for these issues there as well, so that others don't run into
them in the future.
thanks,
greg k-h
^ 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