[PATCH] crypto: AF_ALG - remove SGL end indicator when chaining
The SGL is MAX_SGL_ENTS + 1 in size. The last SG entry is used for the
chaining and is properly updated with the sg_chain invocation. During
the filling-in of the initial SG entries, sg_mark_end is called for each
SG entry. This is appropriate as long as no additional SGL is chained
with the current SGL. However, when a new SGL is chained and the last
SG entry is updated with sg_chain, the last but one entry still contains
the end marker from the sg_mark_end. This end marker must be removed as
otherwise a walk of the chained SGLs will cause a NULL pointer
dereference at the last but one SG entry, because sg_next will return
NULL.
Fixes: 8ff590903d5fc ("crypto: algif_skcipher - User-space interface
for skcipher operations")
CC:
CC: Herbert Xu
Signed-off-by: Stephan Mueller
---
crypto/algif_skcipher.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 43839b00fe6c..62449a8f14ce 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -139,8 +139,10 @@ static int skcipher_alloc_sgl(struct sock *sk)
sg_init_table(sgl->sg, MAX_SGL_ENTS + 1);
sgl->cur = 0;
- if (sg)
+ if (sg) {
sg_chain(sg, MAX_SGL_ENTS + 1, sgl->sg);
+ sg_unmark_end(sg + (MAX_SGL_ENTS - 1));
+ }
list_add_tail(&sgl->list, &ctx->tsgl);
}
--
2.13.5
Re: [PATCH] crypto: MPI - kunmap after finishing accessing buffer
Am Dienstag, 22. August 2017, 08:33:15 CEST schrieb Herbert Xu:
Hi Herbert,
> On Thu, Aug 10, 2017 at 08:06:18AM +0200, Stephan Müller wrote:
> > Hi Herbert,
> >
> > I found that issue while playing around with edge conditions in my
> > algif_akcipher implementation. This issue only manifests in a
> > segmentation violation on 32 bit machines and with an SGL where each
> > SG points to one byte. SGLs with larger buffers seem to be not
> > affected by this issue.
> >
> > Yet this access-after-unmap should be a candidate for stable, IMHO.
>
> Good catch. Thanks!
>
> Fixes: 4816c9406430 ("lib/mpi: Fix SG miter leak")
> Cc:
Just to confirm: stable is not CCed in this email -- did you send it
separately to stable? (dto for "[PATCH v4] crypto: only call put_page on
referenced and used pages")
Thanks a lot
Stephan
Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API
Hi Tudor, >> you still need to get the public key out of the kernel if you want to use it >> from user space. Or feed the remote public key if you plan to use some sort >> of key derivation function. > > The crypto hardware that I'm working on, generates the private key > internally within the device and never reveals it to software and > immediately returns the public key pair. The user can retrieve the > public key from hardware. and don’t we want some sort of access control here. Only the user / process that requested the private key and has access to the public key is allowed to keep using the private key? >> I am saying this again, if you only have a hammer, everything looks like a >> nail. What about actually looking at how this would be used from user space >> in real crypto cases. >> My point is that the usages here are key generation, some sort of >> key-exchange-agreement (aka DH) and key derivation into a symmetric key. >> Frankly the focus with asymmetric ciphers are the keys and the key >> derivation. They are not encryption and decryption of massive amounts of >> data. > > The hardware uses it's own private key and the public key received from > the other end and computes the ecdh shared secret. The hardware computed > shared secret can then be used for key derivation. And that is normally the case. Get your local public key, send it to the other side, get the other sides public key, give it to the hardware and get shared secret. So how is AF_ALG a good fit here? Regards Marcel
Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API
Hi, Marcel, On 08/30/2017 10:21 AM, Marcel Holtmann wrote: you still need to get the public key out of the kernel if you want to use it from user space. Or feed the remote public key if you plan to use some sort of key derivation function. The crypto hardware that I'm working on, generates the private key internally within the device and never reveals it to software and immediately returns the public key pair. The user can retrieve the public key from hardware. I am saying this again, if you only have a hammer, everything looks like a nail. What about actually looking at how this would be used from user space in real crypto cases. My point is that the usages here are key generation, some sort of key-exchange-agreement (aka DH) and key derivation into a symmetric key. Frankly the focus with asymmetric ciphers are the keys and the key derivation. They are not encryption and decryption of massive amounts of data. The hardware uses it's own private key and the public key received from the other end and computes the ecdh shared secret. The hardware computed shared secret can then be used for key derivation. Cheers, ta
Re: [PATCH v8 0/4] crypto: add algif_akcipher user space API
Hi Tudor, > akcipher can work with its own internal keys, now that we have crypto > accelerators that can generate keys that never leave the hardware. Going > through the kernel's key subsystem seems superfluous in this case. > > I also understand the need of going through the kernel's key subsystem > when the user wants to refer to a key which exists elsewhere, such as in > TPM or within an SGX software enclave, but this seems orthogonal with > crypto accelerators with key generation and retention support. > > How should we interface akcipher/kpp with user-space? you still need to get the public key out of the kernel if you want to use it from user space. Or feed the remote public key if you plan to use some sort of key derivation function. I am saying this again, if you only have a hammer, everything looks like a nail. What about actually looking at how this would be used from user space in real crypto cases. My point is that the usages here are key generation, some sort of key-exchange-agreement (aka DH) and key derivation into a symmetric key. Frankly the focus with asymmetric ciphers are the keys and the key derivation. They are not encryption and decryption of massive amounts of data. Regards Marcel
[PATCH] crypto: AF_ALG - update correct dst SGL entry
When two adjacent TX SGL are processed and parts of both TX SGLs
are pulled into the per-request TX SGL, the wrong per-request
TX SGL entries were updated.
This fixes a NULL pointer dereference when a cipher implementation walks
the TX SGL where some of the SGL entries were NULL.
Signed-off-by: Stephan Mueller
---
crypto/af_alg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index ffa9f4ccd9b4..337cf382718e 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -619,14 +619,14 @@ void af_alg_pull_tsgl(struct sock *sk, size_t used,
struct scatterlist *dst,
struct af_alg_ctx *ctx = ask->private;
struct af_alg_tsgl *sgl;
struct scatterlist *sg;
- unsigned int i, j;
+ unsigned int i, j = 0;
while (!list_empty(&ctx->tsgl_list)) {
sgl = list_first_entry(&ctx->tsgl_list, struct af_alg_tsgl,
list);
sg = sgl->sg;
- for (i = 0, j = 0; i < sgl->cur; i++) {
+ for (i = 0; i < sgl->cur; i++) {
size_t plen = min_t(size_t, used, sg[i].length);
struct page *page = sg_page(sg + i);
--
2.13.5
