* Re: x86-64: Maintain 16-byte stack alignment
From: Josh Poimboeuf @ 2017-01-12 15:18 UTC (permalink / raw)
To: Herbert Xu
Cc: Andy Lutomirski, Linus Torvalds, Linux Kernel Mailing List,
Linux Crypto Mailing List, Ingo Molnar, Thomas Gleixner,
Andy Lutomirski, Ard Biesheuvel
In-Reply-To: <20170112150650.GA16000@gondor.apana.org.au>
On Thu, Jan 12, 2017 at 11:06:50PM +0800, Herbert Xu wrote:
> On Thu, Jan 12, 2017 at 09:03:18AM -0600, Josh Poimboeuf wrote:
> >
> > For the entry code, could we just replace all calls with CALL_ALIGNED?
> > That might be less intrusive than trying to adjust all the pt_regs
> > accesses.
> >
> > Then to ensure that nobody ever uses 'call' directly:
> >
> > '#define call please-use-CALL-ALIGNED-instead-of-call'
> >
> > I think that would be possible if CALL_ALIGNED were a ".macro".
>
> The trouble with that is that you have got things like TRACE_IRQS_OFF
> which are also used outside of the entry code.
Where? As far as I can tell, TRACE_IRQS_OFF is used exclusively by entry
code.
--
Josh
^ permalink raw reply
* Re: [PATCH 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Stephan Müller @ 2017-01-12 15:23 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170112145728.GA15595@gondor.apana.org.au>
Am Donnerstag, 12. Januar 2017, 22:57:28 CET schrieb Herbert Xu:
Hi Herbert,
> On Thu, Jan 12, 2017 at 12:22:09PM +0100, Stephan Müller wrote:
> > When addressing the issue in the algif_aead code, and expect that over
> > time
> > the AEAD implementations will gain the copy operation, eventually we will
> > copy the AAD twice. Of course, this could be prevented, if the algif_aead
> > code somehow uses the same SGL for the src and dst AAD.
>
> Why would you copy it twice? You copy everything before you start
> and then just do in-place crypto.
>
As far as I understand, we have the following AAD copy operations that we
discuss:
- algif_aead: you suggested to add the AAD copy operation here
- AEAD implementations: over time, the AEAD implementations shall receive the
AAD copy operation. The AAD copy operation shall only take place if the src
SGL != dst SGL
The algif_aead code *always* will have the src SGL being different from the
dst SGL. Thus, any existing AEAD implementation copy will always be performed
which would be in addition to the algif_aead AAD copy operation. We can only
avoid the AEAD implementation copy operation, if we add some code to
algif_aead to make sure that the AAD data is pointed to by src/dst SGL that is
identical. However, such logic to make src/dst SGL identical is quite complex
compared to simply use one callback as suggested in the current patch set.
In the followup email, I suggested to add the AAD copy function invocation
into crypto_aead_encrypt. This way, we can drop the numerous patches to the
AEAD implementations and yet we can avoid adding such copy operation and src/
dst SGL unification logic to algif_aead.
> > > BTW, why are you only doing the copy for encryption?
> >
> > I was looking at the only AEAD implementation that does the copy
> > operation:
> > authenc. There, the copy operation is only performed for encryption. I was
> > thinking a bit about why decryption was not covered. I think the answer is
> > the following: for encryption, the AAD is definitely needed in the dst
> > buffer as the dst buffer with the AAD must be sent to the recipient for
> > decryption. The decryption and the associated authentication only works
> > with the AAD. However, after decrypting, all the caller wants is the
> > decrypted plaintext only. There is no further use of the AAD after the
> > decryption step. Hence, copying the AAD to the dst buffer in the
> > decryption step would not serve the caller.
> That's just the current implementation. If we're going to make
> this an API then we should do it for both directions.
Considering the suggestion above to add the AAD copy call to
crypto_aead_encrypt, we can add such copy call also to crypto_aead_decrypt.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Herbert Xu @ 2017-01-12 15:27 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <6428553.skGSKSKcYl@tauon.atsec.com>
On Thu, Jan 12, 2017 at 04:23:50PM +0100, Stephan Müller wrote:
>
> As far as I understand, we have the following AAD copy operations that we
> discuss:
>
> - algif_aead: you suggested to add the AAD copy operation here
>
> - AEAD implementations: over time, the AEAD implementations shall receive the
> AAD copy operation. The AAD copy operation shall only take place if the src
> SGL != dst SGL
If and when that happens we'd simply need to remove the copy from
algif_aead code. But even if we didn't you wouldn't have two copies
because algif_aead should invoke the in-place code-path after doing
a full copy of src to dst.
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 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Stephan Müller @ 2017-01-12 15:34 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170112152710.GA16222@gondor.apana.org.au>
Am Donnerstag, 12. Januar 2017, 23:27:10 CET schrieb Herbert Xu:
Hi Herbert,
> On Thu, Jan 12, 2017 at 04:23:50PM +0100, Stephan Müller wrote:
> > As far as I understand, we have the following AAD copy operations that we
> > discuss:
> >
> > - algif_aead: you suggested to add the AAD copy operation here
> >
> > - AEAD implementations: over time, the AEAD implementations shall receive
> > the AAD copy operation. The AAD copy operation shall only take place if
> > the src SGL != dst SGL
>
> If and when that happens we'd simply need to remove the copy from
> algif_aead code.
We would only be able to remove it if all AEAD implementations are converted.
But for the conversion time, we do face that issue.
> But even if we didn't you wouldn't have two copies
> because algif_aead should invoke the in-place code-path after doing
> a full copy of src to dst.
Are you suggesting that the entire data in the src SGL is first copied to the
dst SGL by algif_aead? If yes, that still requires significant src/dst SGL
tinkering as we have the tag -- the src SGL for encrypt does not have the tag
space where the dst SGL for encrypt is required to have the tag size. This is
vice versa for the decryption operation.
And to me the most elegant solution seems adding the copy operation to
crypto_aead_[en|de]crypt.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Herbert Xu @ 2017-01-12 15:39 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <2819680.QEYfZCVF62@tauon.atsec.com>
On Thu, Jan 12, 2017 at 04:34:39PM +0100, Stephan Müller wrote:
>
> We would only be able to remove it if all AEAD implementations are converted.
> But for the conversion time, we do face that issue.
It doesn't matter. Nobody in the kernel uses that. In fact I
wonder whether we should even do it for the kernel API. We only
need it for the user-space API because it goes through read/write.
> Are you suggesting that the entire data in the src SGL is first copied to the
> dst SGL by algif_aead? If yes, that still requires significant src/dst SGL
> tinkering as we have the tag -- the src SGL for encrypt does not have the tag
> space where the dst SGL for encrypt is required to have the tag size. This is
> vice versa for the decryption operation.
It's really only a problem for decryption. In that case you can
extend the dst SG list to include the tag.
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] crypto: kpp - clear CRYPTO_ALG_DEAD bit in prepare_alg
From: Herbert Xu @ 2017-01-12 15:41 UTC (permalink / raw)
To: Salvatore Benedetto; +Cc: linux-crypto
In-Reply-To: <1483364021-103771-1-git-send-email-salvatore.benedetto@intel.com>
On Mon, Jan 02, 2017 at 01:33:41PM +0000, Salvatore Benedetto wrote:
> Make sure CRYPTO_ALG_DEAD is not set when preparing for
> alg registration. This fixes qat-dh registration that occurs
> when reloading qat_c62x module.
>
> Signed-off-by: Salvatore Benedetto <salvatore.benedetto@intel.com>
> ---
> crypto/kpp.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/crypto/kpp.c b/crypto/kpp.c
> index d36ce05..d1adef8e 100644
> --- a/crypto/kpp.c
> +++ b/crypto/kpp.c
> @@ -101,6 +101,7 @@ static void kpp_prepare_alg(struct kpp_alg *alg)
>
> base->cra_type = &crypto_kpp_type;
> base->cra_flags &= ~CRYPTO_ALG_TYPE_MASK;
> + base->cra_flags &= ~CRYPTO_ALG_DEAD;
> base->cra_flags |= CRYPTO_ALG_TYPE_KPP;
> }
Same comment as in
https://patchwork.kernel.org/patch/9485115/
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 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Stephan Müller @ 2017-01-12 15:50 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170112153924.GA16347@gondor.apana.org.au>
Am Donnerstag, 12. Januar 2017, 23:39:24 CET schrieb Herbert Xu:
Hi Herbert,
> On Thu, Jan 12, 2017 at 04:34:39PM +0100, Stephan Müller wrote:
> > We would only be able to remove it if all AEAD implementations are
> > converted. But for the conversion time, we do face that issue.
>
> It doesn't matter. Nobody in the kernel uses that. In fact I
> wonder whether we should even do it for the kernel API. We only
> need it for the user-space API because it goes through read/write.
So you say that we could remove it from authenc() entirely (this is currently
the only location where such copy operation is found for the encryption
direction)?
I would concur that the kernel does not need that.
>
> > Are you suggesting that the entire data in the src SGL is first copied to
> > the dst SGL by algif_aead? If yes, that still requires significant
> > src/dst SGL tinkering as we have the tag -- the src SGL for encrypt does
> > not have the tag space where the dst SGL for encrypt is required to have
> > the tag size. This is vice versa for the decryption operation.
>
> It's really only a problem for decryption. In that case you can
> extend the dst SG list to include the tag.
If we only want to solve that for algif_aead, wouldn't it make more sense if
the user space caller takes care of that (such as libkcapi)? By tinkering with
the SGLs and copying the data to the dst buffer before the cipher operation
takes place, I guess we will add performance degradation and more complexity
in the kernel.
Having such logic in user space would keep the algif_aead cleaner IMHO.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 1/2] crypto: aead AF_ALG - overhaul memory management
From: Herbert Xu @ 2017-01-12 15:51 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <2789022.qIBfP2h8Cy@positron.chronox.de>
On Sun, Dec 25, 2016 at 06:15:06PM +0100, Stephan Müller wrote:
>
> + * The following concept of the memory management is used:
> + *
> + * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL is
> + * filled by user space with the data submitted via sendpage/sendmsg. Filling
> + * up the TX SGL does not cause a crypto operation -- the data will only be
> + * tracked by the kernel. Upon receipt of one recvmsg call, the caller must
> + * provide a buffer which is tracked with the RX SGL.
> + *
> + * During the processing of the recvmsg operation, the cipher request is
> + * allocated and prepared. To support multiple recvmsg operations operating
> + * on one TX SGL, an offset pointer into the TX SGL is maintained. The TX SGL
> + * that is used for the crypto request is scatterwalk_ffwd by the offset
> + * pointer to obtain the start address the crypto operation shall use for
> + * the input data.
I think this is overcomplicating things. The async interface
should be really simple. It should be exactly the same as the
sync interface, except that completion is out-of-line.
So there should be no mixing of SGLs from different requests.
Just start with a clean slate after each recvmsg regardless of
whether it's sync or async.
The only difference in the async case is that you need to keep a
reference to the old pages and free them upon completion. But this
should in no way interfere with subsequent requests.
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: aead AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-01-12 15:56 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170112155127.GA19252@gondor.apana.org.au>
Am Donnerstag, 12. Januar 2017, 23:51:28 CET schrieb Herbert Xu:
Hi Herbert,
> On Sun, Dec 25, 2016 at 06:15:06PM +0100, Stephan Müller wrote:
> > + * The following concept of the memory management is used:
> > + *
> > + * The kernel maintains two SGLs, the TX SGL and the RX SGL. The TX SGL
> > is
> > + * filled by user space with the data submitted via sendpage/sendmsg.
> > Filling + * up the TX SGL does not cause a crypto operation -- the data
> > will only be + * tracked by the kernel. Upon receipt of one recvmsg call,
> > the caller must + * provide a buffer which is tracked with the RX SGL.
> > + *
> > + * During the processing of the recvmsg operation, the cipher request is
> > + * allocated and prepared. To support multiple recvmsg operations
> > operating + * on one TX SGL, an offset pointer into the TX SGL is
> > maintained. The TX SGL + * that is used for the crypto request is
> > scatterwalk_ffwd by the offset + * pointer to obtain the start address
> > the crypto operation shall use for + * the input data.
>
> I think this is overcomplicating things. The async interface
> should be really simple. It should be exactly the same as the
> sync interface, except that completion is out-of-line.
>
> So there should be no mixing of SGLs from different requests.
> Just start with a clean slate after each recvmsg regardless of
> whether it's sync or async.
>
> The only difference in the async case is that you need to keep a
> reference to the old pages and free them upon completion. But this
> should in no way interfere with subsequent requests.
That would mean that we would only support one IOCB.
At least with algif_skcipher, having multiple IOCBs would reduce the number of
system calls user space needs to make for multiple plaintext / ciphertext
blocks. But then, with the use of IOVECs, user space could provide all input
data with one system call anyway.
Ok, I will update the patch as suggested.
>
> Cheers,
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Stephan Müller @ 2017-01-12 16:01 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170112155344.GB19252@gondor.apana.org.au>
Am Donnerstag, 12. Januar 2017, 23:53:44 CET schrieb Herbert Xu:
Hi Herbert,
>
> > If we only want to solve that for algif_aead, wouldn't it make more sense
> > if the user space caller takes care of that (such as libkcapi)? By
> > tinkering with the SGLs and copying the data to the dst buffer before the
> > cipher operation takes place, I guess we will add performance degradation
> > and more complexity in the kernel.
> >
> > Having such logic in user space would keep the algif_aead cleaner IMHO.
>
> We need to have a sane kernel API that respects POSIX.
I fully agree. Therefore, I was under the impression that disregarding the AAD
in recvmsg entirely would be most appropriate as offered with the patch
"crypto: AF_ALG - disregard AAD buffer space for output". In this case we
would be fully POSIX compliant, the kernel would not copy the AAD (and thus
perform multiple memcpy operations due to copy_from_user and copy_to_user
round trips) and leave the AAD copy operation entirely to user space.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 1/2] crypto: aead AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-01-12 16:05 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <3051008.MbfH4VO98W@tauon.atsec.com>
Am Donnerstag, 12. Januar 2017, 16:56:04 CET schrieb Stephan Müller:
Hi Herbert,
>
> That would mean that we would only support one IOCB.
As we also need to be standards compliant, would it be appropriate to only
support one IOCB? I think this is a significant difference to other AIO
operations like for networking.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Herbert Xu @ 2017-01-12 16:06 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <1604413.m50iU9qQPD@tauon.atsec.com>
On Thu, Jan 12, 2017 at 05:01:13PM +0100, Stephan Müller wrote:
>
> I fully agree. Therefore, I was under the impression that disregarding the AAD
> in recvmsg entirely would be most appropriate as offered with the patch
> "crypto: AF_ALG - disregard AAD buffer space for output". In this case we
> would be fully POSIX compliant, the kernel would not copy the AAD (and thus
> perform multiple memcpy operations due to copy_from_user and copy_to_user
> round trips) and leave the AAD copy operation entirely to user space.
Yes but then you'd have to play nasty games to fit this through
the kernel API. Besides, we could still do in-place crypto even
though you suggested that it's complicated. It's not really.
All we have to do is walk through the SG list and compare each
page/offset. For the common case it's going to be a single-entry
list.
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: aead AF_ALG - overhaul memory management
From: Herbert Xu @ 2017-01-12 16:07 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <2701907.5SyJthQSdc@tauon.atsec.com>
On Thu, Jan 12, 2017 at 05:05:03PM +0100, Stephan Müller wrote:
> Am Donnerstag, 12. Januar 2017, 16:56:04 CET schrieb Stephan Müller:
>
> Hi Herbert,
>
> >
> > That would mean that we would only support one IOCB.
>
> As we also need to be standards compliant, would it be appropriate to only
> support one IOCB? I think this is a significant difference to other AIO
> operations like for networking.
Why would we be limited to one IOCB?
--
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 v1 3/8] crypto:chcr- Fix key length for RFC4106
From: Herbert Xu @ 2017-01-12 16:09 UTC (permalink / raw)
To: Harsh Jain; +Cc: hariprasad, netdev, linux-crypto
In-Reply-To: <6d8e61299e051d51dacdb6bfd6c5e582b230027c.1483599449.git.harsh@chelsio.com>
On Fri, Jan 06, 2017 at 02:01:34PM +0530, Harsh Jain wrote:
> Check keylen before copying salt to avoid wrap around of Integer.
>
> Signed-off-by: Harsh Jain <harsh@chelsio.com>
> ---
> drivers/crypto/chelsio/chcr_algo.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/chelsio/chcr_algo.c b/drivers/crypto/chelsio/chcr_algo.c
> index deec7c0..6c2dea3 100644
> --- a/drivers/crypto/chelsio/chcr_algo.c
> +++ b/drivers/crypto/chelsio/chcr_algo.c
> @@ -2194,8 +2194,8 @@ static int chcr_gcm_setkey(struct crypto_aead *aead, const u8 *key,
> unsigned int ck_size;
> int ret = 0, key_ctx_size = 0;
>
> - if (get_aead_subtype(aead) ==
> - CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) {
> + if (get_aead_subtype(aead) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106 &&
> + keylen > 3) {
> keylen -= 4; /* nonce/salt is present in the last 4 bytes */
> memcpy(aeadctx->salt, key + keylen, 4);
> }
We should return an error in this case.
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: aead AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-01-12 16:10 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170112160739.GB19577@gondor.apana.org.au>
Am Freitag, 13. Januar 2017, 00:07:39 CET schrieb Herbert Xu:
Hi Herbert,
> On Thu, Jan 12, 2017 at 05:05:03PM +0100, Stephan Müller wrote:
> > Am Donnerstag, 12. Januar 2017, 16:56:04 CET schrieb Stephan Müller:
> >
> > Hi Herbert,
> >
> > > That would mean that we would only support one IOCB.
> >
> > As we also need to be standards compliant, would it be appropriate to only
> > support one IOCB? I think this is a significant difference to other AIO
> > operations like for networking.
>
> Why would we be limited to one IOCB?
Each IOCB would transpire into an independent, separate recvmsg invocation
without an additional sendmsg/sendpage operation. Thus, in order to support
multiple IOCBs, all data the multiple recvmsg invocations will operate on must
be injected into the kernel beforehand.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v1 0/8] crypto:chcr- Bug fixes
From: Herbert Xu @ 2017-01-12 16:14 UTC (permalink / raw)
To: Harsh Jain; +Cc: hariprasad, netdev, linux-crypto
In-Reply-To: <cover.1483599449.git.harsh@chelsio.com>
On Fri, Jan 06, 2017 at 02:01:31PM +0530, Harsh Jain wrote:
> The patch series is based on Herbert's cryptodev-2.6 tree.
> It include bug fixes.
>
> Atul Gupta (4):
> crypto:chcr-Change flow IDs
> crypto:chcr- Fix panic on dma_unmap_sg
> crypto:chcr- Check device is allocated before use
> crypto:chcr- Fix wrong typecasting
> Harsh Jain (4):
> crypto:chcr- Fix key length for RFC4106
> crypto:chcr- Use cipher instead of Block Cipher in gcm setkey
> crypto:chcr: Change cra_flags for cipher algos
> crypto:chcr- Change algo priority
When you resubmit this please split it into two series. Please
send the critical bug fixes (panic + key length + alloc check)
in one series separate from the others. This way I can push
them easily to the 4.10 tree.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 1/2] crypto: aead AF_ALG - overhaul memory management
From: Herbert Xu @ 2017-01-12 16:17 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <1946014.7KfpB7DN4Q@tauon.atsec.com>
On Thu, Jan 12, 2017 at 05:10:14PM +0100, Stephan Müller wrote:
>
> Each IOCB would transpire into an independent, separate recvmsg invocation
> without an additional sendmsg/sendpage operation. Thus, in order to support
> multiple IOCBs, all data the multiple recvmsg invocations will operate on must
> be injected into the kernel beforehand.
I don't understand, what's wrong with:
sendmsg(fd, ...)
aio_read(iocb1)
sendmsg(fd, ...)
aio_read(iocb2)
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: aead AF_ALG - overhaul memory management
From: Stephan Müller @ 2017-01-12 16:19 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170112161759.GC19732@gondor.apana.org.au>
Am Freitag, 13. Januar 2017, 00:17:59 CET schrieb Herbert Xu:
Hi Herbert,
> On Thu, Jan 12, 2017 at 05:10:14PM +0100, Stephan Müller wrote:
> > Each IOCB would transpire into an independent, separate recvmsg invocation
> > without an additional sendmsg/sendpage operation. Thus, in order to
> > support
> > multiple IOCBs, all data the multiple recvmsg invocations will operate on
> > must be injected into the kernel beforehand.
>
> I don't understand, what's wrong with:
>
> sendmsg(fd, ...)
> aio_read(iocb1)
> sendmsg(fd, ...)
> aio_read(iocb2)
Sure, that works. But here you limit yourself to one IOCB per aio_read. But
aio_read supports multiple IOCBs in one invocation. And this is the issue I am
considering.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH] fix itnull.cocci warnings
From: Herbert Xu @ 2017-01-12 16:26 UTC (permalink / raw)
To: Julia Lawall
Cc: Harsh Jain, hariprasad, netdev, linux-crypto, Atul Gupta,
kbuild-all
In-Reply-To: <alpine.DEB.2.20.1701071042410.2029@hadrien>
On Sat, Jan 07, 2017 at 10:46:17AM +0100, Julia Lawall wrote:
> The first argument to list_for_each_entry cannot be NULL.
>
> Generated by: scripts/coccinelle/iterators/itnull.cocci
>
> CC: Harsh Jain <harsh@chelsio.com>
> Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> ---
>
> This code comes from the following git tree:
>
> url:
> https://github.com/0day-ci/linux/commits/Harsh-Jain/crypto-chcr-Bug-fixes/20170107-093356
> base:
> https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
> master
> In-Reply-To:
> <8e0086b56d8fb61637d179c32a09a1bca03c4186.1483599449.git.harsh@chelsio.com>
Harsh, please fold this patch into your series when you resubmit.
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Herbert Xu @ 2017-01-12 15:53 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <19793111.SdtrLVTG75@tauon.atsec.com>
On Thu, Jan 12, 2017 at 04:50:46PM +0100, Stephan Müller wrote:
>
> So you say that we could remove it from authenc() entirely (this is currently
> the only location where such copy operation is found for the encryption
> direction)?
>
> I would concur that the kernel does not need that.
authenc needs it for performance reasons. The kernel API as it
stands basically says that it may or may not copy the AAD. If
you choose to have a distinct AAD in the dst SG list then either
you throw away the result or you copy it yourself.
> If we only want to solve that for algif_aead, wouldn't it make more sense if
> the user space caller takes care of that (such as libkcapi)? By tinkering with
> the SGLs and copying the data to the dst buffer before the cipher operation
> takes place, I guess we will add performance degradation and more complexity
> in the kernel.
>
> Having such logic in user space would keep the algif_aead cleaner IMHO.
We need to have a sane kernel API that respects POSIX.
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 00/13] crypto: copy AAD during encrypt for AEAD ciphers
From: Stephan Müller @ 2017-01-12 16:37 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20170112160629.GA19577@gondor.apana.org.au>
Am Freitag, 13. Januar 2017, 00:06:29 CET schrieb Herbert Xu:
Hi Herbert,
> On Thu, Jan 12, 2017 at 05:01:13PM +0100, Stephan Müller wrote:
> > I fully agree. Therefore, I was under the impression that disregarding the
> > AAD in recvmsg entirely would be most appropriate as offered with the
> > patch "crypto: AF_ALG - disregard AAD buffer space for output". In this
> > case we would be fully POSIX compliant, the kernel would not copy the AAD
> > (and thus perform multiple memcpy operations due to copy_from_user and
> > copy_to_user round trips) and leave the AAD copy operation entirely to
> > user space.
> Yes but then you'd have to play nasty games to fit this through
> the kernel API.
I would not understand that statement.
With the patch mentioned above that I provided some weeks ago, we have the
following scenario for an encryption (in case of decryption, it is almost
identical, just the tag location is reversed):
user calls sendmsg with data buffer/IOVEC: AAD || PT
-> algif_aead turns this into the src SGL
user calls recvmsg with data buffer/IOVEC: CT || Tag
-> algif_aead creates the first SG entry in the dst SGL pointing to the
AAD from the src SGL
-> algif_aead appends the user buffers to the dst SGL
-> algif_aead performs its operation and during that operation, only the
CT and Tag parts are changed
I.e. with the pre-pending of the SG pointing to the AAD from the src SGL to
the dst SGL we have a clean invocation of the kernel API.
Yet, we avoid copy round trips of the AAD from src to dst in the kernel.
> Besides, we could still do in-place crypto even
> though you suggested that it's complicated.
Is that crypto-in-place operation really a goal we want to achieve if we "buy"
it with a memcpy of the data from the src SGL to the dst SGL before the crypto
operation?
Can we really call such implementation a crypto-in-place operation?
> It's not really.
I concur, for encryption the suggested memcpy is not difficult. Only for
decryption, the memcpy is more challenging.
> All we have to do is walk through the SG list and compare each
> page/offset. For the common case it's going to be a single-entry
> list.
>
> Cheers,
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v1 3/8] crypto:chcr- Fix key length for RFC4106
From: Harsh Jain @ 2017-01-12 16:38 UTC (permalink / raw)
To: Herbert Xu; +Cc: hariprasad, netdev, linux-crypto
In-Reply-To: <20170112160959.GA19732@gondor.apana.org.au>
On 12-01-2017 21:39, Herbert Xu wrote:
> On Fri, Jan 06, 2017 at 02:01:34PM +0530, Harsh Jain wrote:
>> Check keylen before copying salt to avoid wrap around of Integer.
>>
>> Signed-off-by: Harsh Jain <harsh@chelsio.com>
>> ---
>> drivers/crypto/chelsio/chcr_algo.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/crypto/chelsio/chcr_algo.c b/drivers/crypto/chelsio/chcr_algo.c
>> index deec7c0..6c2dea3 100644
>> --- a/drivers/crypto/chelsio/chcr_algo.c
>> +++ b/drivers/crypto/chelsio/chcr_algo.c
>> @@ -2194,8 +2194,8 @@ static int chcr_gcm_setkey(struct crypto_aead *aead, const u8 *key,
>> unsigned int ck_size;
>> int ret = 0, key_ctx_size = 0;
>>
>> - if (get_aead_subtype(aead) ==
>> - CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) {
>> + if (get_aead_subtype(aead) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106 &&
>> + keylen > 3) {
>> keylen -= 4; /* nonce/salt is present in the last 4 bytes */
>> memcpy(aeadctx->salt, key + keylen, 4);
>> }
> We should return an error in this case.
That case is already handled in next if condition.It will error out with -EINVAL in next condition.
if (keylen == AES_KEYSIZE_128) {
>
> Cheers,
^ permalink raw reply
* Re: [PATCH] crypto: Replaced gcc specific attributes with macros from compiler.h
From: Herbert Xu @ 2017-01-12 16:39 UTC (permalink / raw)
To: gidisrael; +Cc: linux-kernel, linux-crypto, davem, nhorman, joe, akpm
In-Reply-To: <1483199783-31588-1-git-send-email-gidisrael@gmail.com>
On Sat, Dec 31, 2016 at 09:26:23PM +0530, gidisrael@gmail.com wrote:
> From: Gideon Israel Dsouza <gidisrael@gmail.com>
>
> Continuing from this commit: 52f5684c8e1e
> ("kernel: use macros from compiler.h instead of __attribute__((...))")
>
> I submitted 4 total patches. They are part of task I've taken up to
> increase compiler portability in the kernel. I've cleaned up the
> subsystems under /kernel /mm /block and /security, this patch targets
> /crypto.
>
> There is <linux/compiler.h> which provides macros for various gcc specific
> constructs. Eg: __weak for __attribute__((weak)). I've cleaned all
> instances of gcc specific attributes with the right macros for the crypto
> subsystem.
>
> I had to make one additional change into compiler-gcc.h for the case when
> one wants to use this: __attribute__((aligned) and not specify an alignment
> factor. From the gcc docs, this will result in the largest alignment for
> that data type on the target machine so I've named the macro
> __aligned_largest. Please advise if another name is more appropriate.
>
> Signed-off-by: Gideon Israel Dsouza <gidisrael@gmail.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: testmgr - use kmemdup instead of kmalloc+memcpy
From: Herbert Xu @ 2017-01-12 16:39 UTC (permalink / raw)
To: Eric Biggers; +Cc: linux-crypto, David S. Miller, Laura Abbott, Eric Biggers
In-Reply-To: <20161230201200.31467-1-ebiggers3@gmail.com>
On Fri, Dec 30, 2016 at 02:12:00PM -0600, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
>
> It's recommended to use kmemdup instead of kmalloc followed by memcpy.
>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 0/3] crypto: picoxcell - Cleanups removing non-DT code
From: Herbert Xu @ 2017-01-12 16:39 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: linux-kernel, Arnd Bergmann, Jamie Iles, David S. Miller,
linux-crypto, linux-arm-kernel
In-Reply-To: <1483376819-26726-1-git-send-email-javier@osg.samsung.com>
On Mon, Jan 02, 2017 at 02:06:56PM -0300, Javier Martinez Canillas wrote:
> Hello,
>
> This small series contains a couple of cleanups that removes some driver's code
> that isn't needed due the driver being for a DT-only platform.
>
> The changes were suggested by Arnd Bergmann as a response to a previous patch:
> https://lkml.org/lkml/2017/1/2/342
>
> Patch #1 allows the driver to be built when the COMPILE_TEST option is enabled.
> Patch #2 removes the platform ID table since isn't needed for DT-only drivers.
> Patch #3 removes a wrapper function that's also not needed if driver is DT-only.
All applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
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