* [PATCH 5/6] crypto: talitos - implement cra_priority
From: Christophe Leroy @ 2016-05-27 9:32 UTC (permalink / raw)
To: Kim Phillips, Herbert Xu, David S Miller
Cc: linux-kernel, linuxppc-dev, linux-crypto
In-Reply-To: <cover.1464330501.git.christophe.leroy@c-s.fr>
SEC1 doesn't have IPSEC_ESP descriptor type but it is able to perform
IPSEC using HMAC_SNOOP_NO_AFEU, which is also existing on SEC2
In order to be able to define descriptors templates for SEC1 without
breaking SEC2+, we have to give lower priority to HMAC_SNOOP_NO_AFEU
so that SEC2+ selects IPSEC_ESP and not HMAC_SNOOP_NO_AFEU which is
less performant.
This is done by adding a priority field in the template. If the field
is 0, we use the default priority, otherwise we used the one in the
field.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/crypto/talitos.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index fcdf83b..b554f56 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2103,6 +2103,7 @@ static int ahash_setkey(struct crypto_ahash *tfm, const u8 *key,
struct talitos_alg_template {
u32 type;
+ u32 priority;
union {
struct crypto_alg crypto;
struct ahash_alg hash;
@@ -2880,7 +2881,10 @@ static struct talitos_crypto_alg *talitos_alg_alloc(struct device *dev,
}
alg->cra_module = THIS_MODULE;
- alg->cra_priority = TALITOS_CRA_PRIORITY;
+ if (t_alg->algt.priority)
+ alg->cra_priority = t_alg->algt.priority;
+ else
+ alg->cra_priority = TALITOS_CRA_PRIORITY;
alg->cra_alignmask = 0;
alg->cra_ctxsize = sizeof(struct talitos_ctx);
alg->cra_flags |= CRYPTO_ALG_KERN_DRIVER_ONLY;
--
2.1.0
^ permalink raw reply related
* [PATCH 4/6] crypto: talitos - sg_to_link_tbl() not used anymore, remove it
From: Christophe Leroy @ 2016-05-27 9:32 UTC (permalink / raw)
To: Kim Phillips, Herbert Xu, David S Miller
Cc: linux-kernel, linuxppc-dev, linux-crypto
In-Reply-To: <cover.1464330501.git.christophe.leroy@c-s.fr>
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/crypto/talitos.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index c17e6c9..fcdf83b 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1111,14 +1111,6 @@ next:
return count;
}
-static inline int sg_to_link_tbl(struct scatterlist *sg, int sg_count,
- int cryptlen,
- struct talitos_ptr *link_tbl_ptr)
-{
- return sg_to_link_tbl_offset(sg, sg_count, 0, cryptlen,
- link_tbl_ptr);
-}
-
int talitos_sg_map(struct device *dev, struct scatterlist *src,
unsigned int len, struct talitos_edesc *edesc,
struct talitos_ptr *ptr,
--
2.1.0
^ permalink raw reply related
* [PATCH 3/6] crypto: talitos - Implement AEAD for SEC1 using HMAC_SNOOP_NO_AFEU
From: Christophe Leroy @ 2016-05-27 9:32 UTC (permalink / raw)
To: Kim Phillips, Herbert Xu, David S Miller
Cc: linux-kernel, linuxppc-dev, linux-crypto
In-Reply-To: <cover.1464330501.git.christophe.leroy@c-s.fr>
This patchs enhances the IPSEC_ESP related functions for them to
also supports the same operations with descriptor type
HMAC_SNOOP_NO_AFEU.
The differences between the two descriptor types are:
* pointeurs 2 and 3 are swaped (Confidentiality key and
Primary EU Context IN)
* HMAC_SNOOP_NO_AFEU has CICV out in pointer 6
* HMAC_SNOOP_NO_AFEU has no primary EU context out so we get it
from the end of data out
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/crypto/talitos.c | 205 +++++++++++++++++++++++++++--------------------
1 file changed, 119 insertions(+), 86 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 0f5a739..c17e6c9 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -940,7 +940,13 @@ static void ipsec_esp_unmap(struct device *dev,
struct talitos_edesc *edesc,
struct aead_request *areq)
{
- unmap_single_talitos_ptr(dev, &edesc->desc.ptr[6], DMA_FROM_DEVICE);
+ struct crypto_aead *aead = crypto_aead_reqtfm(areq);
+ struct talitos_ctx *ctx = crypto_aead_ctx(aead);
+ unsigned int ivsize = crypto_aead_ivsize(aead);
+
+ if (edesc->desc.hdr & DESC_HDR_TYPE_IPSEC_ESP)
+ unmap_single_talitos_ptr(dev, &edesc->desc.ptr[6],
+ DMA_FROM_DEVICE);
unmap_single_talitos_ptr(dev, &edesc->desc.ptr[3], DMA_TO_DEVICE);
unmap_single_talitos_ptr(dev, &edesc->desc.ptr[2], DMA_TO_DEVICE);
unmap_single_talitos_ptr(dev, &edesc->desc.ptr[0], DMA_TO_DEVICE);
@@ -951,6 +957,13 @@ static void ipsec_esp_unmap(struct device *dev,
if (edesc->dma_len)
dma_unmap_single(dev, edesc->dma_link_tbl, edesc->dma_len,
DMA_BIDIRECTIONAL);
+
+ if (!(edesc->desc.hdr & DESC_HDR_TYPE_IPSEC_ESP)) {
+ unsigned int dst_nents = edesc->dst_nents ? : 1;
+
+ sg_pcopy_to_buffer(areq->dst, dst_nents, ctx->iv, ivsize,
+ areq->assoclen + areq->cryptlen - ivsize);
+ }
}
/*
@@ -960,6 +973,8 @@ static void ipsec_esp_encrypt_done(struct device *dev,
struct talitos_desc *desc, void *context,
int err)
{
+ struct talitos_private *priv = dev_get_drvdata(dev);
+ bool is_sec1 = has_ftr_sec1(priv);
struct aead_request *areq = context;
struct crypto_aead *authenc = crypto_aead_reqtfm(areq);
unsigned int authsize = crypto_aead_authsize(authenc);
@@ -973,8 +988,11 @@ static void ipsec_esp_encrypt_done(struct device *dev,
/* copy the generated ICV to dst */
if (edesc->icv_ool) {
- icvdata = &edesc->link_tbl[edesc->src_nents +
- edesc->dst_nents + 2];
+ if (is_sec1)
+ icvdata = edesc->buf + areq->assoclen + areq->cryptlen;
+ else
+ icvdata = &edesc->link_tbl[edesc->src_nents +
+ edesc->dst_nents + 2];
sg = sg_last(areq->dst, edesc->dst_nents);
memcpy((char *)sg_virt(sg) + sg->length - authsize,
icvdata, authsize);
@@ -995,6 +1013,8 @@ static void ipsec_esp_decrypt_swauth_done(struct device *dev,
struct talitos_edesc *edesc;
struct scatterlist *sg;
char *oicv, *icv;
+ struct talitos_private *priv = dev_get_drvdata(dev);
+ bool is_sec1 = has_ftr_sec1(priv);
edesc = container_of(desc, struct talitos_edesc, desc);
@@ -1006,7 +1026,12 @@ static void ipsec_esp_decrypt_swauth_done(struct device *dev,
icv = (char *)sg_virt(sg) + sg->length - authsize;
if (edesc->dma_len) {
- oicv = (char *)&edesc->link_tbl[edesc->src_nents +
+ if (is_sec1)
+ oicv = (char *)&edesc->dma_link_tbl +
+ req->assoclen + req->cryptlen;
+ else
+ oicv = (char *)
+ &edesc->link_tbl[edesc->src_nents +
edesc->dst_nents + 2];
if (edesc->icv_ool)
icv = oicv + authsize;
@@ -1150,42 +1175,49 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
int tbl_off = 0;
int sg_count, ret;
int sg_link_tbl_len;
+ struct talitos_private *priv = dev_get_drvdata(dev);
+ bool is_sec1 = has_ftr_sec1(priv);
/* hmac key */
map_single_talitos_ptr(dev, &desc->ptr[0], ctx->authkeylen, &ctx->key,
DMA_TO_DEVICE);
- sg_count = dma_map_sg(dev, areq->src, edesc->src_nents ?: 1,
- (areq->src == areq->dst) ? DMA_BIDIRECTIONAL
- : DMA_TO_DEVICE);
- /* hmac data */
- to_talitos_ptr_len(&desc->ptr[1], areq->assoclen, 0);
- if (sg_count > 1 &&
- (ret = sg_to_link_tbl_offset(areq->src, sg_count, 0,
- areq->assoclen,
- &edesc->link_tbl[tbl_off])) > 1) {
- to_talitos_ptr(&desc->ptr[1], edesc->dma_link_tbl + tbl_off *
- sizeof(struct talitos_ptr), 0);
- to_talitos_ptr_ext_set(&desc->ptr[1], DESC_PTR_LNKTBL_JUMP, 0);
+ sg_count = edesc->src_nents ?: 1;
+ if (is_sec1 && sg_count > 1)
+ sg_copy_to_buffer(areq->src, sg_count, edesc->buf,
+ areq->assoclen + cryptlen);
+ else
+ sg_count = dma_map_sg(dev, areq->src, sg_count,
+ (areq->src == areq->dst) ?
+ DMA_BIDIRECTIONAL : DMA_TO_DEVICE);
- dma_sync_single_for_device(dev, edesc->dma_link_tbl,
- edesc->dma_len, DMA_BIDIRECTIONAL);
+ /* hmac data */
+ ret = talitos_sg_map(dev, areq->src, areq->assoclen, edesc,
+ &desc->ptr[1], sg_count, 0, tbl_off);
+ if (ret > 1)
tbl_off += ret;
- } else {
- to_talitos_ptr(&desc->ptr[1], sg_dma_address(areq->src), 0);
- to_talitos_ptr_ext_set(&desc->ptr[1], 0, 0);
- }
/* cipher iv */
- to_talitos_ptr(&desc->ptr[2], edesc->iv_dma, 0);
- to_talitos_ptr_len(&desc->ptr[2], ivsize, 0);
- to_talitos_ptr_ext_set(&desc->ptr[2], 0, 0);
+ if (desc->hdr & DESC_HDR_TYPE_IPSEC_ESP) {
+ to_talitos_ptr(&desc->ptr[2], edesc->iv_dma, is_sec1);
+ to_talitos_ptr_len(&desc->ptr[2], ivsize, is_sec1);
+ to_talitos_ptr_ext_set(&desc->ptr[2], 0, is_sec1);
+ } else {
+ to_talitos_ptr(&desc->ptr[3], edesc->iv_dma, is_sec1);
+ to_talitos_ptr_len(&desc->ptr[3], ivsize, is_sec1);
+ to_talitos_ptr_ext_set(&desc->ptr[3], 0, is_sec1);
+ }
/* cipher key */
- map_single_talitos_ptr(dev, &desc->ptr[3], ctx->enckeylen,
- (char *)&ctx->key + ctx->authkeylen,
- DMA_TO_DEVICE);
+ if (desc->hdr & DESC_HDR_TYPE_IPSEC_ESP)
+ map_single_talitos_ptr(dev, &desc->ptr[3], ctx->enckeylen,
+ (char *)&ctx->key + ctx->authkeylen,
+ DMA_TO_DEVICE);
+ else
+ map_single_talitos_ptr(dev, &desc->ptr[2], ctx->enckeylen,
+ (char *)&ctx->key + ctx->authkeylen,
+ DMA_TO_DEVICE);
/*
* cipher in
@@ -1193,78 +1225,79 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
* extent is bytes of HMAC postpended to ciphertext,
* typically 12 for ipsec
*/
- to_talitos_ptr_len(&desc->ptr[4], cryptlen, 0);
- to_talitos_ptr_ext_set(&desc->ptr[4], authsize, 0);
+ to_talitos_ptr_len(&desc->ptr[4], cryptlen, is_sec1);
+ to_talitos_ptr_ext_set(&desc->ptr[4], 0, is_sec1);
sg_link_tbl_len = cryptlen;
- if (edesc->desc.hdr & DESC_HDR_MODE1_MDEU_CICV)
- sg_link_tbl_len += authsize;
- if (sg_count == 1) {
- to_talitos_ptr(&desc->ptr[4], sg_dma_address(areq->src) +
- areq->assoclen, 0);
- } else if ((ret = sg_to_link_tbl_offset(areq->src, sg_count,
- areq->assoclen, sg_link_tbl_len,
- &edesc->link_tbl[tbl_off])) >
- 1) {
- to_talitos_ptr_ext_or(&desc->ptr[4], DESC_PTR_LNKTBL_JUMP, 0);
- to_talitos_ptr(&desc->ptr[4], edesc->dma_link_tbl +
- tbl_off *
- sizeof(struct talitos_ptr), 0);
- dma_sync_single_for_device(dev, edesc->dma_link_tbl,
- edesc->dma_len,
- DMA_BIDIRECTIONAL);
- tbl_off += ret;
- } else {
- copy_talitos_ptr(&desc->ptr[4], &edesc->link_tbl[tbl_off], 0);
+ if (desc->hdr & DESC_HDR_TYPE_IPSEC_ESP) {
+ to_talitos_ptr_ext_set(&desc->ptr[4], authsize, is_sec1);
+
+ if (edesc->desc.hdr & DESC_HDR_MODE1_MDEU_CICV)
+ sg_link_tbl_len += authsize;
}
- /* cipher out */
- to_talitos_ptr_len(&desc->ptr[5], cryptlen, 0);
- to_talitos_ptr_ext_set(&desc->ptr[5], authsize, 0);
+ sg_count = talitos_sg_map(dev, areq->src, cryptlen, edesc,
+ &desc->ptr[4], sg_count, areq->assoclen,
+ tbl_off);
+
+ if (sg_count > 1)
+ tbl_off += sg_count;
- if (areq->src != areq->dst)
- sg_count = dma_map_sg(dev, areq->dst, edesc->dst_nents ? : 1,
- DMA_FROM_DEVICE);
+ /* cipher out */
+ if (areq->src != areq->dst) {
+ sg_count = edesc->dst_nents ? : 1;
+ if (!is_sec1 || sg_count == 1)
+ dma_map_sg(dev, areq->dst, sg_count, DMA_FROM_DEVICE);
+ }
- edesc->icv_ool = false;
+ sg_count = talitos_sg_map(dev, areq->dst, cryptlen, edesc,
+ &desc->ptr[5], sg_count, areq->assoclen,
+ tbl_off);
- if (sg_count == 1) {
- to_talitos_ptr(&desc->ptr[5], sg_dma_address(areq->dst) +
- areq->assoclen, 0);
- } else if ((sg_count =
- sg_to_link_tbl_offset(areq->dst, sg_count,
- areq->assoclen, cryptlen,
- &edesc->link_tbl[tbl_off])) > 1) {
- struct talitos_ptr *tbl_ptr = &edesc->link_tbl[tbl_off];
-
- to_talitos_ptr(&desc->ptr[5], edesc->dma_link_tbl +
- tbl_off * sizeof(struct talitos_ptr), 0);
-
- /* Add an entry to the link table for ICV data */
- tbl_ptr += sg_count - 1;
- to_talitos_ptr_ext_set(tbl_ptr, 0, 0);
- tbl_ptr++;
- to_talitos_ptr_ext_set(tbl_ptr, DESC_PTR_LNKTBL_RETURN, 0);
- to_talitos_ptr_len(tbl_ptr, authsize, 0);
-
- /* icv data follows link tables */
- to_talitos_ptr(tbl_ptr, edesc->dma_link_tbl +
- (edesc->src_nents + edesc->dst_nents +
- 2) * sizeof(struct talitos_ptr) +
- authsize, 0);
- to_talitos_ptr_ext_or(&desc->ptr[5], DESC_PTR_LNKTBL_JUMP, 0);
- dma_sync_single_for_device(ctx->dev, edesc->dma_link_tbl,
- edesc->dma_len, DMA_BIDIRECTIONAL);
+ if (desc->hdr & DESC_HDR_TYPE_IPSEC_ESP)
+ to_talitos_ptr_ext_or(&desc->ptr[5], authsize, is_sec1);
+ if (sg_count > 1) {
edesc->icv_ool = true;
+
+ if (desc->hdr & DESC_HDR_TYPE_IPSEC_ESP) {
+ struct talitos_ptr *tbl_ptr = &edesc->link_tbl[tbl_off];
+ int offset = (edesc->src_nents + edesc->dst_nents + 2) *
+ sizeof(struct talitos_ptr) + authsize;
+
+ /* Add an entry to the link table for ICV data */
+ tbl_ptr += sg_count - 1;
+ to_talitos_ptr_ext_set(tbl_ptr, 0, is_sec1);
+ tbl_ptr++;
+ to_talitos_ptr_ext_set(tbl_ptr, DESC_PTR_LNKTBL_RETURN,
+ is_sec1);
+ to_talitos_ptr_len(tbl_ptr, authsize, is_sec1);
+
+ /* icv data follows link tables */
+ to_talitos_ptr(tbl_ptr, edesc->dma_link_tbl + offset,
+ is_sec1);
+ }
} else {
- copy_talitos_ptr(&desc->ptr[5], &edesc->link_tbl[tbl_off], 0);
+ edesc->icv_ool = false;
+ }
+
+ /* ICV data */
+ if (!(desc->hdr & DESC_HDR_TYPE_IPSEC_ESP)) {
+ to_talitos_ptr_len(&desc->ptr[6], authsize, is_sec1);
+ to_talitos_ptr(&desc->ptr[6], edesc->dma_link_tbl +
+ areq->assoclen + cryptlen, is_sec1);
}
/* iv out */
- map_single_talitos_ptr(dev, &desc->ptr[6], ivsize, ctx->iv,
- DMA_FROM_DEVICE);
+ if (desc->hdr & DESC_HDR_TYPE_IPSEC_ESP)
+ map_single_talitos_ptr(dev, &desc->ptr[6], ivsize, ctx->iv,
+ DMA_FROM_DEVICE);
+
+ if (edesc->dma_len)
+ dma_sync_single_for_device(dev, edesc->dma_link_tbl,
+ edesc->dma_len,
+ DMA_BIDIRECTIONAL);
ret = talitos_submit(dev, ctx->ch, desc, callback, areq);
if (ret != -EINPROGRESS) {
--
2.1.0
^ permalink raw reply related
* [PATCH 2/6] crypto: talitos - making mapping helpers more generic
From: Christophe Leroy @ 2016-05-27 9:32 UTC (permalink / raw)
To: Kim Phillips, Herbert Xu, David S Miller
Cc: linux-kernel, linuxppc-dev, linux-crypto
In-Reply-To: <cover.1464330501.git.christophe.leroy@c-s.fr>
In preparation of IPSEC for SEC1, first step is to make the mapping
helpers more generic so that they can also be used by AEAD functions.
First, the functions are moved before IPSEC functions in talitos.c
talitos_sg_unmap() and unmap_sg_talitos_ptr() are merged as they
are quite similar, the second one handling the SEC1 case an calling
the first one for SEC2
map_sg_in_talitos_ptr() and map_sg_out_talitos_ptr() are merged
into talitos_sg_map() and enhenced to support offseted zones
as used for AEAD. The actual mapping is now performed outside that
helper.
talitos_edesc_alloc() size calculation are fixed to also take into
account AEAD specific parts also for SEC1
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/crypto/talitos.c | 243 ++++++++++++++++++-----------------------------
1 file changed, 94 insertions(+), 149 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index a92aa37..0f5a739 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -911,19 +911,29 @@ struct talitos_edesc {
static void talitos_sg_unmap(struct device *dev,
struct talitos_edesc *edesc,
struct scatterlist *src,
- struct scatterlist *dst)
+ struct scatterlist *dst,
+ unsigned int len, unsigned int offset)
{
+ struct talitos_private *priv = dev_get_drvdata(dev);
+ bool is_sec1 = has_ftr_sec1(priv);
unsigned int src_nents = edesc->src_nents ? : 1;
unsigned int dst_nents = edesc->dst_nents ? : 1;
+ if (is_sec1 && dst && dst_nents > 1) {
+ dma_sync_single_for_device(dev, edesc->dma_link_tbl + offset,
+ len, DMA_FROM_DEVICE);
+ sg_pcopy_from_buffer(dst, dst_nents, edesc->buf + offset, len,
+ offset);
+ }
if (src != dst) {
- dma_unmap_sg(dev, src, src_nents, DMA_TO_DEVICE);
+ if (src_nents == 1 || !is_sec1)
+ dma_unmap_sg(dev, src, src_nents, DMA_TO_DEVICE);
- if (dst) {
+ if (dst && (dst_nents == 1 || !is_sec1))
dma_unmap_sg(dev, dst, dst_nents, DMA_FROM_DEVICE);
- }
- } else
+ } else if (src_nents == 1 || !is_sec1) {
dma_unmap_sg(dev, src, src_nents, DMA_BIDIRECTIONAL);
+ }
}
static void ipsec_esp_unmap(struct device *dev,
@@ -935,7 +945,8 @@ static void ipsec_esp_unmap(struct device *dev,
unmap_single_talitos_ptr(dev, &edesc->desc.ptr[2], DMA_TO_DEVICE);
unmap_single_talitos_ptr(dev, &edesc->desc.ptr[0], DMA_TO_DEVICE);
- talitos_sg_unmap(dev, edesc, areq->src, areq->dst);
+ talitos_sg_unmap(dev, edesc, areq->src, areq->dst, areq->cryptlen,
+ areq->assoclen);
if (edesc->dma_len)
dma_unmap_single(dev, edesc->dma_link_tbl, edesc->dma_len,
@@ -1083,6 +1094,44 @@ static inline int sg_to_link_tbl(struct scatterlist *sg, int sg_count,
link_tbl_ptr);
}
+int talitos_sg_map(struct device *dev, struct scatterlist *src,
+ unsigned int len, struct talitos_edesc *edesc,
+ struct talitos_ptr *ptr,
+ int sg_count, unsigned int offset, int tbl_off)
+{
+ struct talitos_private *priv = dev_get_drvdata(dev);
+ bool is_sec1 = has_ftr_sec1(priv);
+
+ to_talitos_ptr_len(ptr, len, is_sec1);
+ to_talitos_ptr_ext_set(ptr, 0, is_sec1);
+
+ if (sg_count == 1) {
+ to_talitos_ptr(ptr, sg_dma_address(src) + offset, is_sec1);
+ return sg_count;
+ }
+ if (is_sec1) {
+ to_talitos_ptr(ptr, edesc->dma_link_tbl + offset, is_sec1);
+ dma_sync_single_for_device(dev, edesc->dma_link_tbl,
+ edesc->dma_len, DMA_BIDIRECTIONAL);
+ return sg_count;
+ }
+ sg_count = sg_to_link_tbl_offset(src, sg_count, offset, len,
+ &edesc->link_tbl[tbl_off])
+ if (sg_count == 1) {
+ /* Only one segment now, so no link tbl needed*/
+ copy_talitos_ptr(ptr, &edesc->link_tbl[tbl_off], is_sec1);
+ return sg_count;
+ }
+ to_talitos_ptr(ptr, edesc->dma_link_tbl +
+ tbl_off * sizeof(struct talitos_ptr), is_sec1);
+ to_talitos_ptr_ext_or(ptr, DESC_PTR_LNKTBL_JUMP, is_sec1);
+
+ dma_sync_single_for_device(dev, edesc->dma_link_tbl, edesc->dma_len,
+ DMA_BIDIRECTIONAL);
+
+ return sg_count;
+}
+
/*
* fill in and submit ipsec_esp descriptor
*/
@@ -1241,7 +1290,7 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
bool encrypt)
{
struct talitos_edesc *edesc;
- int src_nents, dst_nents, alloc_len, dma_len;
+ int src_nents, dst_nents, alloc_len, dma_len, src_len, dst_len;
dma_addr_t iv_dma = 0;
gfp_t flags = cryptoflags & CRYPTO_TFM_REQ_MAY_SLEEP ? GFP_KERNEL :
GFP_ATOMIC;
@@ -1259,8 +1308,8 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
iv_dma = dma_map_single(dev, iv, ivsize, DMA_TO_DEVICE);
if (!dst || dst == src) {
- src_nents = sg_nents_for_len(src,
- assoclen + cryptlen + authsize);
+ src_len = assoclen + cryptlen + authsize;
+ src_nents = sg_nents_for_len(src, src_len);
if (src_nents < 0) {
dev_err(dev, "Invalid number of src SG.\n");
err = ERR_PTR(-EINVAL);
@@ -1268,17 +1317,18 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
}
src_nents = (src_nents == 1) ? 0 : src_nents;
dst_nents = dst ? src_nents : 0;
+ dst_len = 0;
} else { /* dst && dst != src*/
- src_nents = sg_nents_for_len(src, assoclen + cryptlen +
- (encrypt ? 0 : authsize));
+ src_len = assoclen + cryptlen + (encrypt ? 0 : authsize);
+ src_nents = sg_nents_for_len(src, src_len);
if (src_nents < 0) {
dev_err(dev, "Invalid number of src SG.\n");
err = ERR_PTR(-EINVAL);
goto error_sg;
}
src_nents = (src_nents == 1) ? 0 : src_nents;
- dst_nents = sg_nents_for_len(dst, assoclen + cryptlen +
- (encrypt ? authsize : 0));
+ dst_len = assoclen + cryptlen + (encrypt ? authsize : 0);
+ dst_nents = sg_nents_for_len(dst, dst_len);
if (dst_nents < 0) {
dev_err(dev, "Invalid number of dst SG.\n");
err = ERR_PTR(-EINVAL);
@@ -1295,8 +1345,8 @@ static struct talitos_edesc *talitos_edesc_alloc(struct device *dev,
alloc_len = sizeof(struct talitos_edesc);
if (src_nents || dst_nents) {
if (is_sec1)
- dma_len = (src_nents ? cryptlen : 0) +
- (dst_nents ? cryptlen : 0);
+ dma_len = (src_nents ? src_len : 0) +
+ (dst_nents ? dst_len : 0);
else
dma_len = (src_nents + dst_nents + 2) *
sizeof(struct talitos_ptr) + authsize * 2;
@@ -1420,40 +1470,13 @@ static int ablkcipher_setkey(struct crypto_ablkcipher *cipher,
return 0;
}
-static void unmap_sg_talitos_ptr(struct device *dev, struct scatterlist *src,
- struct scatterlist *dst, unsigned int len,
- struct talitos_edesc *edesc)
-{
- struct talitos_private *priv = dev_get_drvdata(dev);
- bool is_sec1 = has_ftr_sec1(priv);
-
- if (is_sec1) {
- if (!edesc->src_nents) {
- dma_unmap_sg(dev, src, 1,
- dst != src ? DMA_TO_DEVICE
- : DMA_BIDIRECTIONAL);
- }
- if (dst && edesc->dst_nents) {
- dma_sync_single_for_device(dev,
- edesc->dma_link_tbl + len,
- len, DMA_FROM_DEVICE);
- sg_copy_from_buffer(dst, edesc->dst_nents ? : 1,
- edesc->buf + len, len);
- } else if (dst && dst != src) {
- dma_unmap_sg(dev, dst, 1, DMA_FROM_DEVICE);
- }
- } else {
- talitos_sg_unmap(dev, edesc, src, dst);
- }
-}
-
static void common_nonsnoop_unmap(struct device *dev,
struct talitos_edesc *edesc,
struct ablkcipher_request *areq)
{
unmap_single_talitos_ptr(dev, &edesc->desc.ptr[5], DMA_FROM_DEVICE);
- unmap_sg_talitos_ptr(dev, areq->src, areq->dst, areq->nbytes, edesc);
+ talitos_sg_unmap(dev, edesc, areq->src, areq->dst, areq->nbytes, 0);
unmap_single_talitos_ptr(dev, &edesc->desc.ptr[2], DMA_TO_DEVICE);
unmap_single_talitos_ptr(dev, &edesc->desc.ptr[1], DMA_TO_DEVICE);
@@ -1478,101 +1501,6 @@ static void ablkcipher_done(struct device *dev,
areq->base.complete(&areq->base, err);
}
-int map_sg_in_talitos_ptr(struct device *dev, struct scatterlist *src,
- unsigned int len, struct talitos_edesc *edesc,
- enum dma_data_direction dir, struct talitos_ptr *ptr)
-{
- int sg_count;
- struct talitos_private *priv = dev_get_drvdata(dev);
- bool is_sec1 = has_ftr_sec1(priv);
-
- to_talitos_ptr_len(ptr, len, is_sec1);
-
- if (is_sec1) {
- sg_count = edesc->src_nents ? : 1;
-
- if (sg_count == 1) {
- dma_map_sg(dev, src, 1, dir);
- to_talitos_ptr(ptr, sg_dma_address(src), is_sec1);
- } else {
- sg_copy_to_buffer(src, sg_count, edesc->buf, len);
- to_talitos_ptr(ptr, edesc->dma_link_tbl, is_sec1);
- dma_sync_single_for_device(dev, edesc->dma_link_tbl,
- len, DMA_TO_DEVICE);
- }
- } else {
- to_talitos_ptr_ext_set(ptr, 0, is_sec1);
-
- sg_count = dma_map_sg(dev, src, edesc->src_nents ? : 1, dir);
-
- if (sg_count == 1) {
- to_talitos_ptr(ptr, sg_dma_address(src), is_sec1);
- } else {
- sg_count = sg_to_link_tbl(src, sg_count, len,
- &edesc->link_tbl[0]);
- if (sg_count > 1) {
- to_talitos_ptr(ptr, edesc->dma_link_tbl, 0);
- to_talitos_ptr_ext_or(ptr, DESC_PTR_LNKTBL_JUMP,
- 0);
- dma_sync_single_for_device(dev,
- edesc->dma_link_tbl,
- edesc->dma_len,
- DMA_BIDIRECTIONAL);
- } else {
- /* Only one segment now, so no link tbl needed*/
- to_talitos_ptr(ptr, sg_dma_address(src),
- is_sec1);
- }
- }
- }
- return sg_count;
-}
-
-void map_sg_out_talitos_ptr(struct device *dev, struct scatterlist *dst,
- unsigned int len, struct talitos_edesc *edesc,
- enum dma_data_direction dir,
- struct talitos_ptr *ptr, int sg_count)
-{
- struct talitos_private *priv = dev_get_drvdata(dev);
- bool is_sec1 = has_ftr_sec1(priv);
-
- if (dir != DMA_NONE)
- sg_count = dma_map_sg(dev, dst, edesc->dst_nents ? : 1, dir);
-
- to_talitos_ptr_len(ptr, len, is_sec1);
-
- if (is_sec1) {
- if (sg_count == 1) {
- if (dir != DMA_NONE)
- dma_map_sg(dev, dst, 1, dir);
- to_talitos_ptr(ptr, sg_dma_address(dst), is_sec1);
- } else {
- to_talitos_ptr(ptr, edesc->dma_link_tbl + len, is_sec1);
- dma_sync_single_for_device(dev,
- edesc->dma_link_tbl + len,
- len, DMA_FROM_DEVICE);
- }
- } else {
- to_talitos_ptr_ext_set(ptr, 0, is_sec1);
-
- if (sg_count == 1) {
- to_talitos_ptr(ptr, sg_dma_address(dst), is_sec1);
- } else {
- struct talitos_ptr *link_tbl_ptr =
- &edesc->link_tbl[edesc->src_nents + 1];
-
- to_talitos_ptr(ptr, edesc->dma_link_tbl +
- (edesc->src_nents + 1) *
- sizeof(struct talitos_ptr), 0);
- to_talitos_ptr_ext_or(ptr, DESC_PTR_LNKTBL_JUMP, 0);
- sg_to_link_tbl(dst, sg_count, len, link_tbl_ptr);
- dma_sync_single_for_device(dev, edesc->dma_link_tbl,
- edesc->dma_len,
- DMA_BIDIRECTIONAL);
- }
- }
-}
-
static int common_nonsnoop(struct talitos_edesc *edesc,
struct ablkcipher_request *areq,
void (*callback) (struct device *dev,
@@ -1601,19 +1529,29 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
map_single_talitos_ptr(dev, &desc->ptr[2], ctx->keylen,
(char *)&ctx->key, DMA_TO_DEVICE);
+ sg_count = edesc->src_nents ?: 1;
+ if (is_sec1 && sg_count > 1)
+ sg_copy_to_buffer(areq->src, sg_count, edesc->buf,
+ cryptlen);
+ else
+ sg_count = dma_map_sg(dev, areq->src, sg_count,
+ (areq->src == areq->dst) ?
+ DMA_BIDIRECTIONAL : DMA_TO_DEVICE);
/*
* cipher in
*/
- sg_count = map_sg_in_talitos_ptr(dev, areq->src, cryptlen, edesc,
- (areq->src == areq->dst) ?
- DMA_BIDIRECTIONAL : DMA_TO_DEVICE,
- &desc->ptr[3]);
+ sg_count = talitos_sg_map(dev, areq->src, cryptlen, edesc,
+ &desc->ptr[3], sg_count, 0, 0);
/* cipher out */
- map_sg_out_talitos_ptr(dev, areq->dst, cryptlen, edesc,
- (areq->src == areq->dst) ? DMA_NONE
- : DMA_FROM_DEVICE,
- &desc->ptr[4], sg_count);
+ if (areq->src != areq->dst) {
+ sg_count = edesc->dst_nents ? : 1;
+ if (!is_sec1 || sg_count == 1)
+ dma_map_sg(dev, areq->dst, sg_count, DMA_FROM_DEVICE);
+ }
+
+ talitos_sg_map(dev, areq->dst, cryptlen, edesc, &desc->ptr[4], sg_count,
+ 0, (edesc->src_nents + 1));
/* iv out */
map_single_talitos_ptr(dev, &desc->ptr[5], ivsize, ctx->iv,
@@ -1685,7 +1623,7 @@ static void common_nonsnoop_hash_unmap(struct device *dev,
unmap_single_talitos_ptr(dev, &edesc->desc.ptr[5], DMA_FROM_DEVICE);
- unmap_sg_talitos_ptr(dev, req_ctx->psrc, NULL, 0, edesc);
+ talitos_sg_unmap(dev, edesc, req_ctx->psrc, NULL, 0, 0);
/* When using hashctx-in, must unmap it. */
if (from_talitos_ptr_len(&edesc->desc.ptr[1], is_sec1))
@@ -1758,6 +1696,7 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
int ret;
struct talitos_private *priv = dev_get_drvdata(dev);
bool is_sec1 = has_ftr_sec1(priv);
+ int sg_count;
/* first DWORD empty */
desc->ptr[0] = zero_entry;
@@ -1782,11 +1721,17 @@ static int common_nonsnoop_hash(struct talitos_edesc *edesc,
else
desc->ptr[2] = zero_entry;
+ sg_count = edesc->src_nents ?: 1;
+ if (is_sec1 && sg_count > 1)
+ sg_copy_to_buffer(areq->src, sg_count, edesc->buf, length);
+ else
+ sg_count = dma_map_sg(dev, req_ctx->psrc, sg_count,
+ DMA_TO_DEVICE);
/*
* data in
*/
- map_sg_in_talitos_ptr(dev, req_ctx->psrc, length, edesc,
- DMA_TO_DEVICE, &desc->ptr[3]);
+ talitos_sg_map(dev, req_ctx->psrc, length, edesc, &desc->ptr[3],
+ sg_count, 0, 0);
/* fifth DWORD empty */
desc->ptr[4] = zero_entry;
--
2.1.0
^ permalink raw reply related
* [PATCH 1/6] crypto: talitos - using helpers for all talitos_ptr operations
From: Christophe Leroy @ 2016-05-27 9:32 UTC (permalink / raw)
To: Kim Phillips, Herbert Xu, David S Miller
Cc: linux-kernel, linuxppc-dev, linux-crypto
In-Reply-To: <cover.1464330501.git.christophe.leroy@c-s.fr>
Use helper for all modifications to talitos_ptr in preparation to
the implementation of AEAD for SEC1
to_talitos_ptr_extent_clear() has been removed in favor of
to_talitos_ptr_ext_set() to set any value and
to_talitos_ptr_ext_or() to or the extent field with a value
name has been shorten to help keeping single lines of 80 chars
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/crypto/talitos.c | 59 ++++++++++++++++++++++++++++--------------------
1 file changed, 34 insertions(+), 25 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index b7ee8d3..a92aa37 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -91,10 +91,17 @@ static unsigned short from_talitos_ptr_len(struct talitos_ptr *ptr,
return be16_to_cpu(ptr->len);
}
-static void to_talitos_ptr_extent_clear(struct talitos_ptr *ptr, bool is_sec1)
+static void to_talitos_ptr_ext_set(struct talitos_ptr *ptr, u8 val,
+ bool is_sec1)
{
if (!is_sec1)
- ptr->j_extent = 0;
+ ptr->j_extent = val;
+}
+
+static void to_talitos_ptr_ext_or(struct talitos_ptr *ptr, u8 val, bool is_sec1)
+{
+ if (!is_sec1)
+ ptr->j_extent |= val;
}
/*
@@ -111,7 +118,7 @@ static void map_single_talitos_ptr(struct device *dev,
to_talitos_ptr_len(ptr, len, is_sec1);
to_talitos_ptr(ptr, dma_addr, is_sec1);
- to_talitos_ptr_extent_clear(ptr, is_sec1);
+ to_talitos_ptr_ext_set(ptr, 0, is_sec1);
}
/*
@@ -1050,8 +1057,8 @@ static int sg_to_link_tbl_offset(struct scatterlist *sg, int sg_count,
to_talitos_ptr(link_tbl_ptr + count,
sg_dma_address(sg) + offset, 0);
- link_tbl_ptr[count].len = cpu_to_be16(len);
- link_tbl_ptr[count].j_extent = 0;
+ to_talitos_ptr_len(link_tbl_ptr + count, len, 0);
+ to_talitos_ptr_ext_set(link_tbl_ptr + count, 0, 0);
count++;
cryptlen -= len;
offset = 0;
@@ -1062,7 +1069,8 @@ next:
/* tag end of link table */
if (count > 0)
- link_tbl_ptr[count - 1].j_extent = DESC_PTR_LNKTBL_RETURN;
+ to_talitos_ptr_ext_set(link_tbl_ptr + count - 1,
+ DESC_PTR_LNKTBL_RETURN, 0);
return count;
}
@@ -1102,14 +1110,14 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
(areq->src == areq->dst) ? DMA_BIDIRECTIONAL
: DMA_TO_DEVICE);
/* hmac data */
- desc->ptr[1].len = cpu_to_be16(areq->assoclen);
+ to_talitos_ptr_len(&desc->ptr[1], areq->assoclen, 0);
if (sg_count > 1 &&
(ret = sg_to_link_tbl_offset(areq->src, sg_count, 0,
areq->assoclen,
&edesc->link_tbl[tbl_off])) > 1) {
to_talitos_ptr(&desc->ptr[1], edesc->dma_link_tbl + tbl_off *
sizeof(struct talitos_ptr), 0);
- desc->ptr[1].j_extent = DESC_PTR_LNKTBL_JUMP;
+ to_talitos_ptr_ext_set(&desc->ptr[1], DESC_PTR_LNKTBL_JUMP, 0);
dma_sync_single_for_device(dev, edesc->dma_link_tbl,
edesc->dma_len, DMA_BIDIRECTIONAL);
@@ -1117,13 +1125,13 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
tbl_off += ret;
} else {
to_talitos_ptr(&desc->ptr[1], sg_dma_address(areq->src), 0);
- desc->ptr[1].j_extent = 0;
+ to_talitos_ptr_ext_set(&desc->ptr[1], 0, 0);
}
/* cipher iv */
to_talitos_ptr(&desc->ptr[2], edesc->iv_dma, 0);
- desc->ptr[2].len = cpu_to_be16(ivsize);
- desc->ptr[2].j_extent = 0;
+ to_talitos_ptr_len(&desc->ptr[2], ivsize, 0);
+ to_talitos_ptr_ext_set(&desc->ptr[2], 0, 0);
/* cipher key */
map_single_talitos_ptr(dev, &desc->ptr[3], ctx->enckeylen,
@@ -1136,8 +1144,8 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
* extent is bytes of HMAC postpended to ciphertext,
* typically 12 for ipsec
*/
- desc->ptr[4].len = cpu_to_be16(cryptlen);
- desc->ptr[4].j_extent = authsize;
+ to_talitos_ptr_len(&desc->ptr[4], cryptlen, 0);
+ to_talitos_ptr_ext_set(&desc->ptr[4], authsize, 0);
sg_link_tbl_len = cryptlen;
if (edesc->desc.hdr & DESC_HDR_MODE1_MDEU_CICV)
@@ -1150,7 +1158,7 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
areq->assoclen, sg_link_tbl_len,
&edesc->link_tbl[tbl_off])) >
1) {
- desc->ptr[4].j_extent |= DESC_PTR_LNKTBL_JUMP;
+ to_talitos_ptr_ext_or(&desc->ptr[4], DESC_PTR_LNKTBL_JUMP, 0);
to_talitos_ptr(&desc->ptr[4], edesc->dma_link_tbl +
tbl_off *
sizeof(struct talitos_ptr), 0);
@@ -1163,8 +1171,8 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
}
/* cipher out */
- desc->ptr[5].len = cpu_to_be16(cryptlen);
- desc->ptr[5].j_extent = authsize;
+ to_talitos_ptr_len(&desc->ptr[5], cryptlen, 0);
+ to_talitos_ptr_ext_set(&desc->ptr[5], authsize, 0);
if (areq->src != areq->dst)
sg_count = dma_map_sg(dev, areq->dst, edesc->dst_nents ? : 1,
@@ -1186,17 +1194,17 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct aead_request *areq,
/* Add an entry to the link table for ICV data */
tbl_ptr += sg_count - 1;
- tbl_ptr->j_extent = 0;
+ to_talitos_ptr_ext_set(tbl_ptr, 0, 0);
tbl_ptr++;
- tbl_ptr->j_extent = DESC_PTR_LNKTBL_RETURN;
- tbl_ptr->len = cpu_to_be16(authsize);
+ to_talitos_ptr_ext_set(tbl_ptr, DESC_PTR_LNKTBL_RETURN, 0);
+ to_talitos_ptr_len(tbl_ptr, authsize, 0);
/* icv data follows link tables */
to_talitos_ptr(tbl_ptr, edesc->dma_link_tbl +
(edesc->src_nents + edesc->dst_nents +
2) * sizeof(struct talitos_ptr) +
authsize, 0);
- desc->ptr[5].j_extent |= DESC_PTR_LNKTBL_JUMP;
+ to_talitos_ptr_ext_or(&desc->ptr[5], DESC_PTR_LNKTBL_JUMP, 0);
dma_sync_single_for_device(ctx->dev, edesc->dma_link_tbl,
edesc->dma_len, DMA_BIDIRECTIONAL);
@@ -1493,7 +1501,7 @@ int map_sg_in_talitos_ptr(struct device *dev, struct scatterlist *src,
len, DMA_TO_DEVICE);
}
} else {
- to_talitos_ptr_extent_clear(ptr, is_sec1);
+ to_talitos_ptr_ext_set(ptr, 0, is_sec1);
sg_count = dma_map_sg(dev, src, edesc->src_nents ? : 1, dir);
@@ -1504,7 +1512,8 @@ int map_sg_in_talitos_ptr(struct device *dev, struct scatterlist *src,
&edesc->link_tbl[0]);
if (sg_count > 1) {
to_talitos_ptr(ptr, edesc->dma_link_tbl, 0);
- ptr->j_extent |= DESC_PTR_LNKTBL_JUMP;
+ to_talitos_ptr_ext_or(ptr, DESC_PTR_LNKTBL_JUMP,
+ 0);
dma_sync_single_for_device(dev,
edesc->dma_link_tbl,
edesc->dma_len,
@@ -1544,7 +1553,7 @@ void map_sg_out_talitos_ptr(struct device *dev, struct scatterlist *dst,
len, DMA_FROM_DEVICE);
}
} else {
- to_talitos_ptr_extent_clear(ptr, is_sec1);
+ to_talitos_ptr_ext_set(ptr, 0, is_sec1);
if (sg_count == 1) {
to_talitos_ptr(ptr, sg_dma_address(dst), is_sec1);
@@ -1555,7 +1564,7 @@ void map_sg_out_talitos_ptr(struct device *dev, struct scatterlist *dst,
to_talitos_ptr(ptr, edesc->dma_link_tbl +
(edesc->src_nents + 1) *
sizeof(struct talitos_ptr), 0);
- ptr->j_extent |= DESC_PTR_LNKTBL_JUMP;
+ to_talitos_ptr_ext_or(ptr, DESC_PTR_LNKTBL_JUMP, 0);
sg_to_link_tbl(dst, sg_count, len, link_tbl_ptr);
dma_sync_single_for_device(dev, edesc->dma_link_tbl,
edesc->dma_len,
@@ -1586,7 +1595,7 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
/* cipher iv */
to_talitos_ptr(&desc->ptr[1], edesc->iv_dma, is_sec1);
to_talitos_ptr_len(&desc->ptr[1], ivsize, is_sec1);
- to_talitos_ptr_extent_clear(&desc->ptr[1], is_sec1);
+ to_talitos_ptr_ext_set(&desc->ptr[1], 0, is_sec1);
/* cipher key */
map_single_talitos_ptr(dev, &desc->ptr[2], ctx->keylen,
--
2.1.0
^ permalink raw reply related
* [PATCH 0/6] crypto: talitos - implementation of AEAD for SEC1
From: Christophe Leroy @ 2016-05-27 9:32 UTC (permalink / raw)
To: Kim Phillips, Herbert Xu, David S Miller
Cc: linux-kernel, linuxppc-dev, linux-crypto
This set of patches provides the implementation of AEAD for
talitos SEC1.
Christophe Leroy (6):
crypto: talitos - using helpers for all talitos_ptr operations
crypto: talitos - making mapping helpers more generic
crypto: talitos - Implement AEAD for SEC1 using HMAC_SNOOP_NO_AFEU
crypto: talitos - sg_to_link_tbl() not used anymore, remove it
crypto: talitos - implement cra_priority
crypto: talitos - templates for AEAD using HMAC_SNOOP_NO_AFEU
drivers/crypto/talitos.c | 657 +++++++++++++++++++++++++++++------------------
1 file changed, 410 insertions(+), 247 deletions(-)
--
2.1.0
^ permalink raw reply
* Re: tcrypt failing on hmac(crc32)
From: Stephan Mueller @ 2016-05-27 9:24 UTC (permalink / raw)
To: Marcus Meissner; +Cc: linux-crypto
In-Reply-To: <20160527091932.GB924@suse.de>
Am Freitag, 27. Mai 2016, 11:19:32 schrieb Marcus Meissner:
Hi Marcus,
>
> And it actually is:
>
> [ 180.942532] hmac: blocksize check failed, ds=4, cra_blocksize=1, ss=4
> [ 180.942541] alg: hash: Failed to load transform for hmac(crc32): -2
> [ 180.989191] tcrypt: one or more tests failed!
>
> Should I remove hmac(crc32) from the testmgr list?
I am wondering about that: there are test vectors for this cipher, so there
seem to be valid use cases. But the implementation is broken and nobody
noticed up to now. So, it seems it is not in use?
Given that, I would think that hmac(crc32) could be removed from testmgr.
Ciao
Stephan
^ permalink raw reply
* Re: tcrypt failing on hmac(crc32)
From: Marcus Meissner @ 2016-05-27 9:19 UTC (permalink / raw)
To: Stephan Mueller; +Cc: linux-crypto
In-Reply-To: <20160525130528.GA30676@suse.de>
On Wed, May 25, 2016 at 03:05:28PM +0200, Marcus Meissner wrote:
> On Wed, May 25, 2016 at 01:39:46PM +0200, Stephan Mueller wrote:
> > Am Mittwoch, 25. Mai 2016, 13:36:10 schrieb Marcus Meissner:
> >
> > Hi Marcus,
> >
> > > Hi,
> > >
> > > On Wed, May 25, 2016 at 09:10:31AM +0200, Stephan Mueller wrote:
> > > > Am Mittwoch, 25. Mai 2016, 09:07:52 schrieb Marcus Meissner:
> > > >
> > > > Hi Marcus,
> > > >
> > > > > Hi,
> > > > >
> > > > > when enabling the testmgr framework and FIPS in 4.6 and 4.4 and running
> > > > > "modprobe tcrypt"
> > > > >
> > > > }, {
> > > >
> > > > .alg = "hmac(crc32)",
> > > > .test = alg_test_hash,
> > > >
> > > > ...
> > > >
> > > > fips_allowed = 1 missing?
> > >
> > > The kernel was not in FIPS mode, and adding it did not help. :/
> >
> > Sorry, I read FIPS and implied fips=1 :-)
>
> I think we are running in a precondition
>
> ds = salg->digestsize; // is CHKSUM_DIGEST_SIZE == 4 for CRC32
> ss = salg->statesize; // ? cant find it
> alg = &salg->base; // base.cra_blocksize seems CHKSUM_BLOCKSIZE == 1
> if (ds > alg->cra_blocksize ||
> ss < alg->cra_blocksize)
> goto out_put_alg;
>
> 4 > 1 ... so EINVAL return.
>
> If this is the case, hmac(crc32) might be kind of non-sensical?
And it actually is:
[ 180.942532] hmac: blocksize check failed, ds=4, cra_blocksize=1, ss=4
[ 180.942541] alg: hash: Failed to load transform for hmac(crc32): -2
[ 180.989191] tcrypt: one or more tests failed!
Should I remove hmac(crc32) from the testmgr list?
Ciao, Marcus
^ permalink raw reply
* Re: [RFC 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Baolin Wang @ 2016-05-27 9:04 UTC (permalink / raw)
To: Milan Broz
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), Herbert Xu, David Miller,
Eric Biggers, Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida,
Shaohua Li, Dan Williams, Martin K. Petersen, Sagi Grimberg,
Kent Overstreet, Keith Busch, Tejun Heo, Ming Lei, Mark Brown,
Arnd Bergmann, linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <5747FD04.4090707@gmail.com>
On 27 May 2016 at 15:53, Milan Broz <gmazyland@gmail.com> wrote:
> On 05/27/2016 09:04 AM, Baolin Wang wrote:
>> Hi Milan,
>>
>> On 27 May 2016 at 14:31, Milan Broz <gmazyland@gmail.com> wrote:
>>> On 05/25/2016 08:12 AM, Baolin Wang wrote:
>>>> Now some cipher hardware engines prefer to handle bulk block rather than one
>>>> sector (512 bytes) created by dm-crypt, cause these cipher engines can handle
>>>> the intermediate values (IV) by themselves in one bulk block. This means we
>>>> can increase the size of the request by merging request rather than always 512
>>>> bytes and thus increase the hardware engine processing speed.
>>>
>>> Hi,
>>>
>>> could you please elaborate how exactly you are processing independently
>>> encrypted sectors? For example with XTS mode. Do you play internally with
>>> tweak calculation? Does this keep 512 bytes sector encryption blocks independent?
>>>
>>> (If not, it is breaking compatibility everywhere and you are reinventing
>>> disk encryption logic here - just for performance reason for some hw
>>> not designed for this task... But that was said several times already.)
>>
>> These are what the cipher hardware engine and engine driver should do,
>> for software we just need send one initial IV and bulk data to crypto
>> layer, which is enough.
>
> Hi,
>
> Thanks for answer.
>
> So this is just doing some kind of batch processing optimization inside the driver?
> I still do not understand why it is not possible to "batch" these requests inside
> you driver (with async API) then and process them in one go without any changes
> to dmcrypt though. (But I am not familiar with these drivers.)
I think it is not only for the driver level, but also for cipher API.
If one cipher engine can support bulk mode, then we should send bulk
data to it, not only one sector, which need to be implemented at top
API level, such as:
skcipher_request_set_crypt(req, sgt_in.sgl, sgt_out.sgl, total_bytes,
iv); /* send sg table to crypt layer */
If move these optimization into driver, it means we need to merge
requests from dm-crypt together to be one big request for engine
driver, but it will be low efficiency and not help. Why we can not
send one bulk request at first in dm-crypt? We just need to set the
different parameters for skcipher_request_set_crypt() function
according to different cipher mode in dm-crypt.
>
> If I understand it correctly, subsequent IVs are calculated inside
> your driver just based on some initial value?
Yes, something like this.
> This can work only for sequential IVs (or, better said, for predictable
> IVs like plain64, implemented inside your hw/driver).
>
> It cannot work for IVs/tweaks that are randomized or keyed (like ESSIV).
> Yes, I know that using ESSIV for XTS is considered overkill but I do not
> see you are checking this condition anywhere in code (we can definitely
> configure such a mapping).
So if some ciphers (like: aes(cbc)) can not support bulk mode, then we
should not add the CRYPTO_ALG_BULK flag for this cipher in the cipher
driver.
>
>>>> So introduce 'CRYPTO_ALG_BULK' flag to indicate this cipher can support bulk
>>>> mode.
>>>
>>> What exactly skcipher will do if this flag is set?
>>
>> I think that depends on how to implement the cipher engine driver.
>
> I do not care about implementation details, I just would like to see
> some high-level description for the new API flag.
> (Or at least read the code that implements it :)
I think for high-level description, we can use sg table to map one
bulk block and send to crypt layer by skcipher_request_set_crypt()
function.
>
>>> Which drivers it should use? I do not see any posted patch that uses this flag yet.
>>> How we can test it?
>>
>> Some cipher engine drivers which support bulk mode should use this
>> flag. Yeah, we need upstream one cipher driver with this flag for
>> testing.
>
> I think if you introduce new flag, you should also post drivers that uses it.
> Otherwise it is just unused code for mainline.
Yeah, that's right.
>
> And I definitely would like to test it somewhere and see real
> performance data (not just simple dd).
OK. Thanks.
>
> Thanks,
> Milan
>
>>
>>>
>>> Milan
>>>
>>>>
>>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>>> ---
>>>> include/crypto/skcipher.h | 7 +++++++
>>>> include/linux/crypto.h | 6 ++++++
>>>> 2 files changed, 13 insertions(+)
>>>>
>>>> diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
>>>> index 0f987f5..d89d29a 100644
>>>> --- a/include/crypto/skcipher.h
>>>> +++ b/include/crypto/skcipher.h
>>>> @@ -519,5 +519,12 @@ static inline void skcipher_request_set_crypt(
>>>> req->iv = iv;
>>>> }
>>>>
>>>> +static inline unsigned int skcipher_is_bulk_mode(struct crypto_skcipher *sk_tfm)
>>>> +{
>>>> + struct crypto_tfm *tfm = crypto_skcipher_tfm(sk_tfm);
>>>> +
>>>> + return crypto_tfm_alg_bulk(tfm);
>>>> +}
>>>> +
>>>> #endif /* _CRYPTO_SKCIPHER_H */
>>>>
>>>> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
>>>> index 6e28c89..a315487 100644
>>>> --- a/include/linux/crypto.h
>>>> +++ b/include/linux/crypto.h
>>>> @@ -63,6 +63,7 @@
>>>> #define CRYPTO_ALG_DEAD 0x00000020
>>>> #define CRYPTO_ALG_DYING 0x00000040
>>>> #define CRYPTO_ALG_ASYNC 0x00000080
>>>> +#define CRYPTO_ALG_BULK 0x00000100
>>>>
>>>> /*
>>>> * Set this bit if and only if the algorithm requires another algorithm of
>>>> @@ -623,6 +624,11 @@ static inline u32 crypto_tfm_alg_type(struct crypto_tfm *tfm)
>>>> return tfm->__crt_alg->cra_flags & CRYPTO_ALG_TYPE_MASK;
>>>> }
>>>>
>>>> +static inline unsigned int crypto_tfm_alg_bulk(struct crypto_tfm *tfm)
>>>> +{
>>>> + return tfm->__crt_alg->cra_flags & CRYPTO_ALG_BULK;
>>>> +}
>>>> +
>>>> static inline unsigned int crypto_tfm_alg_blocksize(struct crypto_tfm *tfm)
>>>> {
>>>> return tfm->__crt_alg->cra_blocksize;
>>>>
>>>
>>
>>
>>
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [RFC 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Milan Broz @ 2016-05-27 7:53 UTC (permalink / raw)
To: Baolin Wang
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), Herbert Xu, David Miller,
Eric Biggers, Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida,
Shaohua Li, Dan Williams, Martin K. Petersen, Sagi Grimberg,
Kent Overstreet, Keith Busch, Tejun Heo, Ming Lei, Mark Brown,
Arnd Bergmann, linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <CAMz4kuK2yFqtdNJBn1POOo9Y_4=Rk2q8BP8P1P+JdYJk2oLnFw@mail.gmail.com>
On 05/27/2016 09:04 AM, Baolin Wang wrote:
> Hi Milan,
>
> On 27 May 2016 at 14:31, Milan Broz <gmazyland@gmail.com> wrote:
>> On 05/25/2016 08:12 AM, Baolin Wang wrote:
>>> Now some cipher hardware engines prefer to handle bulk block rather than one
>>> sector (512 bytes) created by dm-crypt, cause these cipher engines can handle
>>> the intermediate values (IV) by themselves in one bulk block. This means we
>>> can increase the size of the request by merging request rather than always 512
>>> bytes and thus increase the hardware engine processing speed.
>>
>> Hi,
>>
>> could you please elaborate how exactly you are processing independently
>> encrypted sectors? For example with XTS mode. Do you play internally with
>> tweak calculation? Does this keep 512 bytes sector encryption blocks independent?
>>
>> (If not, it is breaking compatibility everywhere and you are reinventing
>> disk encryption logic here - just for performance reason for some hw
>> not designed for this task... But that was said several times already.)
>
> These are what the cipher hardware engine and engine driver should do,
> for software we just need send one initial IV and bulk data to crypto
> layer, which is enough.
Hi,
Thanks for answer.
So this is just doing some kind of batch processing optimization inside the driver?
I still do not understand why it is not possible to "batch" these requests inside
you driver (with async API) then and process them in one go without any changes
to dmcrypt though. (But I am not familiar with these drivers.)
If I understand it correctly, subsequent IVs are calculated inside
your driver just based on some initial value?
This can work only for sequential IVs (or, better said, for predictable
IVs like plain64, implemented inside your hw/driver).
It cannot work for IVs/tweaks that are randomized or keyed (like ESSIV).
Yes, I know that using ESSIV for XTS is considered overkill but I do not
see you are checking this condition anywhere in code (we can definitely
configure such a mapping).
>>> So introduce 'CRYPTO_ALG_BULK' flag to indicate this cipher can support bulk
>>> mode.
>>
>> What exactly skcipher will do if this flag is set?
>
> I think that depends on how to implement the cipher engine driver.
I do not care about implementation details, I just would like to see
some high-level description for the new API flag.
(Or at least read the code that implements it :)
>> Which drivers it should use? I do not see any posted patch that uses this flag yet.
>> How we can test it?
>
> Some cipher engine drivers which support bulk mode should use this
> flag. Yeah, we need upstream one cipher driver with this flag for
> testing.
I think if you introduce new flag, you should also post drivers that uses it.
Otherwise it is just unused code for mainline.
And I definitely would like to test it somewhere and see real
performance data (not just simple dd).
Thanks,
Milan
>
>>
>> Milan
>>
>>>
>>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>>> ---
>>> include/crypto/skcipher.h | 7 +++++++
>>> include/linux/crypto.h | 6 ++++++
>>> 2 files changed, 13 insertions(+)
>>>
>>> diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
>>> index 0f987f5..d89d29a 100644
>>> --- a/include/crypto/skcipher.h
>>> +++ b/include/crypto/skcipher.h
>>> @@ -519,5 +519,12 @@ static inline void skcipher_request_set_crypt(
>>> req->iv = iv;
>>> }
>>>
>>> +static inline unsigned int skcipher_is_bulk_mode(struct crypto_skcipher *sk_tfm)
>>> +{
>>> + struct crypto_tfm *tfm = crypto_skcipher_tfm(sk_tfm);
>>> +
>>> + return crypto_tfm_alg_bulk(tfm);
>>> +}
>>> +
>>> #endif /* _CRYPTO_SKCIPHER_H */
>>>
>>> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
>>> index 6e28c89..a315487 100644
>>> --- a/include/linux/crypto.h
>>> +++ b/include/linux/crypto.h
>>> @@ -63,6 +63,7 @@
>>> #define CRYPTO_ALG_DEAD 0x00000020
>>> #define CRYPTO_ALG_DYING 0x00000040
>>> #define CRYPTO_ALG_ASYNC 0x00000080
>>> +#define CRYPTO_ALG_BULK 0x00000100
>>>
>>> /*
>>> * Set this bit if and only if the algorithm requires another algorithm of
>>> @@ -623,6 +624,11 @@ static inline u32 crypto_tfm_alg_type(struct crypto_tfm *tfm)
>>> return tfm->__crt_alg->cra_flags & CRYPTO_ALG_TYPE_MASK;
>>> }
>>>
>>> +static inline unsigned int crypto_tfm_alg_bulk(struct crypto_tfm *tfm)
>>> +{
>>> + return tfm->__crt_alg->cra_flags & CRYPTO_ALG_BULK;
>>> +}
>>> +
>>> static inline unsigned int crypto_tfm_alg_blocksize(struct crypto_tfm *tfm)
>>> {
>>> return tfm->__crt_alg->cra_blocksize;
>>>
>>
>
>
>
^ permalink raw reply
* Re: AES-NI: slower than aes-generic?
From: Stephan Mueller @ 2016-05-27 7:08 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Sandy Harris, linux-crypto
In-Reply-To: <20160527021429.GA21331@thunk.org>
Am Donnerstag, 26. Mai 2016, 22:14:29 schrieb Theodore Ts'o:
Hi Theodore,
> On Thu, May 26, 2016 at 08:49:39PM +0200, Stephan Mueller wrote:
> > Using the kernel crypto API one can relieve the CPU of the crypto work, if
> > a hardware or assembler implementation is available. This may be of
> > particular interest for smaller systems. So, for smaller systems (where
> > kernel bloat is bad, but where now these days more and more hardware
> > crypto support is added), we must weigh the kernel bloat (of 3 or 4
> > additional C files for the basic kernel crypto API logic) against
> > relieving the CPU of work.
>
> There are a number of caveats with using hardware acceleration; one is
> that many hardware accelerators are optimized for bulk data
> encryption, and so key scheduling, or switching between key schedules,
> can have a higher overhead that a pure software implementation.
Squeezing the last drop of speed out of the ciphers for the LRNG is not my
priority given that the speed is limited by the reseeding. The LRNG should
allow the CPU to offload the crypto work. For small systems, crypto is intense
work that could be spend elsewhere.
>
> There have also been situations where the hardware crypto engine is
> actually slower than a highly optimized software implementation. This
> has been the case for certain ARM SOC's, for example.
And I would be fine with that. Besides, if a user wants to use software
implementations with the LRNG and still offer HW support for all else, all
they need to do is to statically compile the software implementation and
compile the hardware support as a module. As the LRNG initializes before
kernel modules can be loaded, it can only use what it finds in the static
kernel.
>
> This is not that big of deal, if you are developing a cryptographic
> application (such as file system level encryption, for example) for a
> specific hardware platform (such as a specific Nexus device). But if
> you are trying to develop a generic service that has to work on a wide
> variety of CPU architectures, and specific CPU/SOC implementations,
> life is a lot more complicated. I've worked on both problems, let me
> assure you the second is way tricker than the first.
>
> > Then, the use of the DRBG offers users to choose between a Hash/HMAC and
> > CTR implementation to suit their needs. The DRBG code is agnostic of the
> > underlying cipher. So, you could even use Blowfish instead of AES or
> > whirlpool instead of SHA -- these changes are just one entry in
> > drbg_cores[] away without any code change.
>
> I really question how much this matters in practice. Unless you are a
> US Government Agency, where you might be laboring under a Federal
> mandate to use DUAL-EC DRBG (for example), most users really don't
I am not sure such references help the discussion.
> care about the details of the algorithm used in their random number
> generator. Giving users choice (or lots of knobs) isn't necessarily
> always a feature, as the many TLS downgrade attacks have demonstrated.
The options are at compile time, not at runtime.
>
> This is why from my perspectve it's more important to implement an
> interface which is always there, and which by default is secure, to
> minimize the chances that random JV-team kernel developers working for
> a Linux distribution or some consumer electronics manufacturer won't
> actually make things worse. As the Debian's attempt to "improve" the
> security of OpenSSL demonstrates, it doesn't always end well. :-)
Rest assured, the current implementation of /dev/random gives many people many
headaches. And I can tell you that I have seen "random JV-team kernel
developers" doing things you do not want to know just to make the behavior
better.
And even if they do not change anything, I am yet under the impression that
the current implementation has shortcommings in typical deployment scenarios
(mainly VMs and headless server systems).
Hence I want to give a framework where people can safely alter a few things to
suit their needs. But the things they can change should not affect the overall
security.
>
> If we implement something which happens to result in a 2 minute stall
> in boot times, the danger is that a clueless engineer at Sony, or LGE,
> or Motorola, or BMW, or Toyota, etc, will "fix" the problem without
> telling anyone about what they did, and we might not notice right away
> that the fix was in fact catastrophically bad.
Such "fixes" are employed these days already! And they are not employed
because of the used crypto (which was the topic this thread started), but due
to the handling and accounting of the initial entropy.
So, I think that the used crypto for the DRNG side is just the icing (hence I
said I can live with SP800-90A, your Chacha20, even X9.31 given that the LRNG
ensures proper seeding and reseeding). The real issues are in the entropy
accounting and maintenance and the reseeding of the DRNGs.
Ciao
Stephan
^ permalink raw reply
* Re: [RFC 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Baolin Wang @ 2016-05-27 7:04 UTC (permalink / raw)
To: Milan Broz
Cc: Jens Axboe, Alasdair G Kergon, Mike Snitzer,
open list:DEVICE-MAPPER (LVM), Herbert Xu, David Miller,
Eric Biggers, Joonsoo Kim, tadeusz.struk, smueller, Masanari Iida,
Shaohua Li, Dan Williams, Martin K. Petersen, Sagi Grimberg,
Kent Overstreet, Keith Busch, Tejun Heo, Ming Lei, Mark Brown,
Arnd Bergmann, linux-crypto, linux-block,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT
In-Reply-To: <5747E9CF.7010706@gmail.com>
Hi Milan,
On 27 May 2016 at 14:31, Milan Broz <gmazyland@gmail.com> wrote:
> On 05/25/2016 08:12 AM, Baolin Wang wrote:
>> Now some cipher hardware engines prefer to handle bulk block rather than one
>> sector (512 bytes) created by dm-crypt, cause these cipher engines can handle
>> the intermediate values (IV) by themselves in one bulk block. This means we
>> can increase the size of the request by merging request rather than always 512
>> bytes and thus increase the hardware engine processing speed.
>
> Hi,
>
> could you please elaborate how exactly you are processing independently
> encrypted sectors? For example with XTS mode. Do you play internally with
> tweak calculation? Does this keep 512 bytes sector encryption blocks independent?
>
> (If not, it is breaking compatibility everywhere and you are reinventing
> disk encryption logic here - just for performance reason for some hw
> not designed for this task... But that was said several times already.)
These are what the cipher hardware engine and engine driver should do,
for software we just need send one initial IV and bulk data to crypto
layer, which is enough.
>
>> So introduce 'CRYPTO_ALG_BULK' flag to indicate this cipher can support bulk
>> mode.
>
> What exactly skcipher will do if this flag is set?
I think that depends on how to implement the cipher engine driver.
>
> Which drivers it should use? I do not see any posted patch that uses this flag yet.
> How we can test it?
Some cipher engine drivers which support bulk mode should use this
flag. Yeah, we need upstream one cipher driver with this flag for
testing.
>
> Milan
>
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>> include/crypto/skcipher.h | 7 +++++++
>> include/linux/crypto.h | 6 ++++++
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
>> index 0f987f5..d89d29a 100644
>> --- a/include/crypto/skcipher.h
>> +++ b/include/crypto/skcipher.h
>> @@ -519,5 +519,12 @@ static inline void skcipher_request_set_crypt(
>> req->iv = iv;
>> }
>>
>> +static inline unsigned int skcipher_is_bulk_mode(struct crypto_skcipher *sk_tfm)
>> +{
>> + struct crypto_tfm *tfm = crypto_skcipher_tfm(sk_tfm);
>> +
>> + return crypto_tfm_alg_bulk(tfm);
>> +}
>> +
>> #endif /* _CRYPTO_SKCIPHER_H */
>>
>> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
>> index 6e28c89..a315487 100644
>> --- a/include/linux/crypto.h
>> +++ b/include/linux/crypto.h
>> @@ -63,6 +63,7 @@
>> #define CRYPTO_ALG_DEAD 0x00000020
>> #define CRYPTO_ALG_DYING 0x00000040
>> #define CRYPTO_ALG_ASYNC 0x00000080
>> +#define CRYPTO_ALG_BULK 0x00000100
>>
>> /*
>> * Set this bit if and only if the algorithm requires another algorithm of
>> @@ -623,6 +624,11 @@ static inline u32 crypto_tfm_alg_type(struct crypto_tfm *tfm)
>> return tfm->__crt_alg->cra_flags & CRYPTO_ALG_TYPE_MASK;
>> }
>>
>> +static inline unsigned int crypto_tfm_alg_bulk(struct crypto_tfm *tfm)
>> +{
>> + return tfm->__crt_alg->cra_flags & CRYPTO_ALG_BULK;
>> +}
>> +
>> static inline unsigned int crypto_tfm_alg_blocksize(struct crypto_tfm *tfm)
>> {
>> return tfm->__crt_alg->cra_blocksize;
>>
>
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [RFC 2/3] crypto: Introduce CRYPTO_ALG_BULK flag
From: Milan Broz @ 2016-05-27 6:31 UTC (permalink / raw)
To: Baolin Wang, axboe, agk, snitzer, dm-devel, herbert, davem
Cc: ebiggers3, js1304, tadeusz.struk, smueller, standby24x7, shli,
dan.j.williams, martin.petersen, sagig, kent.overstreet,
keith.busch, tj, ming.lei, broonie, arnd, linux-crypto,
linux-block, linux-raid, linux-kernel
In-Reply-To: <7f0fef5fe473a451e352b2d42e1eed483fd28667.1464144791.git.baolin.wang@linaro.org>
On 05/25/2016 08:12 AM, Baolin Wang wrote:
> Now some cipher hardware engines prefer to handle bulk block rather than one
> sector (512 bytes) created by dm-crypt, cause these cipher engines can handle
> the intermediate values (IV) by themselves in one bulk block. This means we
> can increase the size of the request by merging request rather than always 512
> bytes and thus increase the hardware engine processing speed.
Hi,
could you please elaborate how exactly you are processing independently
encrypted sectors? For example with XTS mode. Do you play internally with
tweak calculation? Does this keep 512 bytes sector encryption blocks independent?
(If not, it is breaking compatibility everywhere and you are reinventing
disk encryption logic here - just for performance reason for some hw
not designed for this task... But that was said several times already.)
> So introduce 'CRYPTO_ALG_BULK' flag to indicate this cipher can support bulk
> mode.
What exactly skcipher will do if this flag is set?
Which drivers it should use? I do not see any posted patch that uses this flag yet.
How we can test it?
Milan
>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> include/crypto/skcipher.h | 7 +++++++
> include/linux/crypto.h | 6 ++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
> index 0f987f5..d89d29a 100644
> --- a/include/crypto/skcipher.h
> +++ b/include/crypto/skcipher.h
> @@ -519,5 +519,12 @@ static inline void skcipher_request_set_crypt(
> req->iv = iv;
> }
>
> +static inline unsigned int skcipher_is_bulk_mode(struct crypto_skcipher *sk_tfm)
> +{
> + struct crypto_tfm *tfm = crypto_skcipher_tfm(sk_tfm);
> +
> + return crypto_tfm_alg_bulk(tfm);
> +}
> +
> #endif /* _CRYPTO_SKCIPHER_H */
>
> diff --git a/include/linux/crypto.h b/include/linux/crypto.h
> index 6e28c89..a315487 100644
> --- a/include/linux/crypto.h
> +++ b/include/linux/crypto.h
> @@ -63,6 +63,7 @@
> #define CRYPTO_ALG_DEAD 0x00000020
> #define CRYPTO_ALG_DYING 0x00000040
> #define CRYPTO_ALG_ASYNC 0x00000080
> +#define CRYPTO_ALG_BULK 0x00000100
>
> /*
> * Set this bit if and only if the algorithm requires another algorithm of
> @@ -623,6 +624,11 @@ static inline u32 crypto_tfm_alg_type(struct crypto_tfm *tfm)
> return tfm->__crt_alg->cra_flags & CRYPTO_ALG_TYPE_MASK;
> }
>
> +static inline unsigned int crypto_tfm_alg_bulk(struct crypto_tfm *tfm)
> +{
> + return tfm->__crt_alg->cra_flags & CRYPTO_ALG_BULK;
> +}
> +
> static inline unsigned int crypto_tfm_alg_blocksize(struct crypto_tfm *tfm)
> {
> return tfm->__crt_alg->cra_blocksize;
>
^ permalink raw reply
* Re: [RFC 3/3] md: dm-crypt: Introduce the bulk mode method when sending request
From: Baolin Wang @ 2016-05-27 6:03 UTC (permalink / raw)
To: Mike Snitzer
Cc: Jens Axboe, Alasdair G Kergon, open list:DEVICE-MAPPER (LVM),
Herbert Xu, David Miller, Sagi Grimberg,
open list:SOFTWARE RAID (Multiple Disks) SUPPORT,
Martin K. Petersen, Joonsoo Kim, tadeusz.struk, smueller,
Ming Lei, Eric Biggers, linux-block, Keith Busch, LKML,
Masanari Iida, Mark Brown, Arnd Bergmann, linux-crypto, Tejun Heo,
Dan Williams, Shaohua Li, Kent Overstreet <kent.overs
In-Reply-To: <20160526140440.GA24865@redhat.com>
On 26 May 2016 at 22:04, Mike Snitzer <snitzer@redhat.com> wrote:
> Comments inlined.
>
> In general the most concerning bit is the need for memory allocation in
> the IO path (see comment/question below near call to sg_alloc_table).
> In DM targets we make heavy use of .ctr preallocated memory and/or
> per-bio-data to avoid memory allocations in the IO path.
Make sense.
>
> On Wed, May 25 2016 at 2:12am -0400,
> Baolin Wang <baolin.wang@linaro.org> wrote:
>
>> In now dm-crypt code, it is ineffective to map one segment (always one
>> sector) of one bio with just only one scatterlist at one time for hardware
>> crypto engine. Especially for some encryption mode (like ecb or xts mode)
>> cooperating with the crypto engine, they just need one initial IV or null
>> IV instead of different IV for each sector. In this situation We can consider
>> to use multiple scatterlists to map the whole bio and send all scatterlists
>> of one bio to crypto engine to encrypt or decrypt, which can improve the
>> hardware engine's efficiency.
>>
>> With this optimization, On my test setup (beaglebone black board) using 64KB
>> I/Os on an eMMC storage device I saw about 60% improvement in throughput for
>> encrypted writes, and about 100% improvement for encrypted reads. But this
>> is not fit for other modes which need different IV for each sector.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>> drivers/md/dm-crypt.c | 188 +++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 176 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
>> index 4f3cb35..1c86ea7 100644
>> --- a/drivers/md/dm-crypt.c
>> +++ b/drivers/md/dm-crypt.c
>> @@ -33,6 +33,7 @@
>> #include <linux/device-mapper.h>
>>
>> #define DM_MSG_PREFIX "crypt"
>> +#define DM_MAX_SG_LIST 1024
>>
>> /*
>> * context holding the current state of a multi-part conversion
>> @@ -46,6 +47,8 @@ struct convert_context {
>> sector_t cc_sector;
>> atomic_t cc_pending;
>> struct skcipher_request *req;
>> + struct sg_table sgt_in;
>> + struct sg_table sgt_out;
>> };
>>
>> /*
>> @@ -803,6 +806,108 @@ static struct crypt_iv_operations crypt_iv_tcw_ops = {
>> .post = crypt_iv_tcw_post
>> };
>>
>> +/*
>> + * Check how many sg entry numbers are needed when map one bio
>> + * with scatterlists in advance.
>> + */
>> +static unsigned int crypt_sg_entry(struct bio *bio_t)
>> +{
>> + struct request_queue *q = bdev_get_queue(bio_t->bi_bdev);
>> + int cluster = blk_queue_cluster(q);
>> + struct bio_vec bvec, bvprv = { NULL };
>> + struct bvec_iter biter;
>> + unsigned long nbytes = 0, sg_length = 0;
>> + unsigned int sg_cnt = 0, first_bvec = 0;
>> +
>> + if (bio_t->bi_rw & REQ_DISCARD) {
>> + if (bio_t->bi_vcnt)
>> + return 1;
>> + return 0;
>> + }
>> +
>> + if (bio_t->bi_rw & REQ_WRITE_SAME)
>> + return 1;
>> +
>> + bio_for_each_segment(bvec, bio_t, biter) {
>> + nbytes = bvec.bv_len;
>> +
>> + if (!cluster) {
>> + sg_cnt++;
>> + continue;
>> + }
>> +
>> + if (!first_bvec) {
>> + first_bvec = 1;
>> + goto new_segment;
>> + }
>> +
>> + if (sg_length + nbytes > queue_max_segment_size(q))
>> + goto new_segment;
>> +
>> + if (!BIOVEC_PHYS_MERGEABLE(&bvprv, &bvec))
>> + goto new_segment;
>> +
>> + if (!BIOVEC_SEG_BOUNDARY(q, &bvprv, &bvec))
>> + goto new_segment;
>> +
>> + sg_length += nbytes;
>> + continue;
>> +
>> +new_segment:
>> + memcpy(&bvprv, &bvec, sizeof(struct bio_vec));
>> + sg_length = nbytes;
>> + sg_cnt++;
>> + }
>> +
>> + return sg_cnt;
>> +}
>> +
>> +static int crypt_convert_alloc_table(struct crypt_config *cc,
>> + struct convert_context *ctx)
>> +{
>> + struct bio *bio_in = ctx->bio_in;
>> + struct bio *bio_out = ctx->bio_out;
>> + unsigned int mode = skcipher_is_bulk_mode(any_tfm(cc));
>
> please use: bool bulk_mode = ...
OK.
>
>> + unsigned int sg_in_max, sg_out_max;
>> + int ret = 0;
>> +
>> + if (!mode)
>> + goto out2;
>
> Please use more descriptive label names than out[1-3]
OK.
>
>> +
>> + /*
>> + * Need to calculate how many sg entry need to be used
>> + * for this bio.
>> + */
>> + sg_in_max = crypt_sg_entry(bio_in) + 1;
>
> The return from crypt_sg_entry() is pretty awkward, given you just go on
> to add 1; as is the bounds checking.. the magic value of 2 needs to be
> be made clearer.
I'll remove the crypt_sg_entry() function.
>
>> + if (sg_in_max > DM_MAX_SG_LIST || sg_in_max <= 2)
>> + goto out2;
>> +
>> + ret = sg_alloc_table(&ctx->sgt_in, sg_in_max, GFP_KERNEL);
>
> Is it safe to be using GFP_KERNEL here? AFAIK this is in the IO mapping
> path and we try to avoid memory allocations at all costs -- due to the
> risk of deadlock when issuing IO to stacked block devices (dm-crypt
> could be part of a much more elaborate IO stack).
OK. I'll move the sg table allocation to be preallocated in the .ctr function.
>
>> + if (ret)
>> + goto out2;
>> +
>> + if (bio_data_dir(bio_in) == READ)
>> + goto out1;
>> +
>> + sg_out_max = crypt_sg_entry(bio_out) + 1;
>> + if (sg_out_max > DM_MAX_SG_LIST || sg_out_max <= 2)
>> + goto out3;
>> +
>> + ret = sg_alloc_table(&ctx->sgt_out, sg_out_max, GFP_KERNEL);
>> + if (ret)
>> + goto out3;
>> +
>> + return 0;
>> +
>> +out3:
>
> out_free_table?
>
>> + sg_free_table(&ctx->sgt_in);
>> +out2:
>
> out_skip_alloc?
>
>> + ctx->sgt_in.orig_nents = 0;
>> +out1:
>
> out_skip_write?
>
>> + ctx->sgt_out.orig_nents = 0;
>> + return ret;
>> +}
>> +
>> static void crypt_convert_init(struct crypt_config *cc,
>> struct convert_context *ctx,
>> struct bio *bio_out, struct bio *bio_in,
>> @@ -843,7 +948,13 @@ static int crypt_convert_block(struct crypt_config *cc,
>> {
>> struct bio_vec bv_in = bio_iter_iovec(ctx->bio_in, ctx->iter_in);
>> struct bio_vec bv_out = bio_iter_iovec(ctx->bio_out, ctx->iter_out);
>> + unsigned int mode = skcipher_is_bulk_mode(any_tfm(cc));
>
> again please use: bool bulk_mode = ...
OK.
>
>> + struct bio *bio_in = ctx->bio_in;
>> + struct bio *bio_out = ctx->bio_out;
>> + unsigned int total_bytes = bio_in->bi_iter.bi_size;
>> struct dm_crypt_request *dmreq;
>> + struct scatterlist *sg_in;
>> + struct scatterlist *sg_out;
>> u8 *iv;
>> int r;
>>
>> @@ -852,16 +963,6 @@ static int crypt_convert_block(struct crypt_config *cc,
>>
>> dmreq->iv_sector = ctx->cc_sector;
>> dmreq->ctx = ctx;
>> - sg_init_table(&dmreq->sg_in, 1);
>> - sg_set_page(&dmreq->sg_in, bv_in.bv_page, 1 << SECTOR_SHIFT,
>> - bv_in.bv_offset);
>> -
>> - sg_init_table(&dmreq->sg_out, 1);
>> - sg_set_page(&dmreq->sg_out, bv_out.bv_page, 1 << SECTOR_SHIFT,
>> - bv_out.bv_offset);
>> -
>> - bio_advance_iter(ctx->bio_in, &ctx->iter_in, 1 << SECTOR_SHIFT);
>> - bio_advance_iter(ctx->bio_out, &ctx->iter_out, 1 << SECTOR_SHIFT);
>>
>> if (cc->iv_gen_ops) {
>> r = cc->iv_gen_ops->generator(cc, iv, dmreq);
>> @@ -869,8 +970,63 @@ static int crypt_convert_block(struct crypt_config *cc,
>> return r;
>> }
>>
>> - skcipher_request_set_crypt(req, &dmreq->sg_in, &dmreq->sg_out,
>> - 1 << SECTOR_SHIFT, iv);
>> + if (mode && ctx->sgt_in.orig_nents > 0) {
>> + struct scatterlist *sg = NULL;
>> + unsigned int total_sg_in, total_sg_out;
>> +
>> + total_sg_in = blk_bio_map_sg(bdev_get_queue(bio_in->bi_bdev),
>> + bio_in, ctx->sgt_in.sgl, &sg);
>> + if ((total_sg_in <= 0) ||
>> + (total_sg_in > ctx->sgt_in.orig_nents)) {
>> + DMERR("%s in sg map error %d, sg table nents[%d]\n",
>> + __func__, total_sg_in, ctx->sgt_in.orig_nents);
>> + return -EINVAL;
>> + }
>> +
>> + if (sg)
>> + sg_mark_end(sg);
>> +
>> + ctx->iter_in.bi_size -= total_bytes;
>> + sg_in = ctx->sgt_in.sgl;
>> + sg_out = ctx->sgt_in.sgl;
>> +
>> + if (bio_data_dir(bio_in) == READ)
>> + goto set_crypt;
>> +
>> + sg = NULL;
>> + total_sg_out = blk_bio_map_sg(bdev_get_queue(bio_out->bi_bdev),
>> + bio_out, ctx->sgt_out.sgl, &sg);
>> + if ((total_sg_out <= 0) ||
>> + (total_sg_out > ctx->sgt_out.orig_nents)) {
>> + DMERR("%s out sg map error %d, sg table nents[%d]\n",
>> + __func__, total_sg_out, ctx->sgt_out.orig_nents);
>> + return -EINVAL;
>> + }
>> +
>> + if (sg)
>> + sg_mark_end(sg);
>> +
>> + ctx->iter_out.bi_size -= total_bytes;
>> + sg_out = ctx->sgt_out.sgl;
>> + } else {
>> + sg_init_table(&dmreq->sg_in, 1);
>> + sg_set_page(&dmreq->sg_in, bv_in.bv_page, 1 << SECTOR_SHIFT,
>> + bv_in.bv_offset);
>> +
>> + sg_init_table(&dmreq->sg_out, 1);
>> + sg_set_page(&dmreq->sg_out, bv_out.bv_page, 1 << SECTOR_SHIFT,
>> + bv_out.bv_offset);
>> +
>> + bio_advance_iter(ctx->bio_in, &ctx->iter_in, 1 << SECTOR_SHIFT);
>> + bio_advance_iter(ctx->bio_out, &ctx->iter_out, 1 << SECTOR_SHIFT);
>> +
>> + sg_in = &dmreq->sg_in;
>> + sg_out = &dmreq->sg_out;
>> + total_bytes = 1 << SECTOR_SHIFT;
>> + }
>> +
>> +set_crypt:
>> + skcipher_request_set_crypt(req, sg_in, sg_out, total_bytes, iv);
>
> Given how long this code has gotten I'd prefer to see this factored out
> to a new setup method.
I'll refactor this long function. Thanks for your comments.
>
>> if (bio_data_dir(ctx->bio_in) == WRITE)
>> r = crypto_skcipher_encrypt(req);
>> @@ -1081,6 +1237,8 @@ static void crypt_dec_pending(struct dm_crypt_io *io)
>> if (io->ctx.req)
>> crypt_free_req(cc, io->ctx.req, base_bio);
>>
>> + sg_free_table(&io->ctx.sgt_in);
>> + sg_free_table(&io->ctx.sgt_out);
>> base_bio->bi_error = error;
>> bio_endio(base_bio);
>> }
>> @@ -1312,6 +1470,9 @@ static void kcryptd_crypt_write_convert(struct dm_crypt_io *io)
>> io->ctx.iter_out = clone->bi_iter;
>>
>> sector += bio_sectors(clone);
>> + r = crypt_convert_alloc_table(cc, &io->ctx);
>> + if (r < 0)
>> + io->error = -EIO;
>>
>> crypt_inc_pending(io);
>> r = crypt_convert(cc, &io->ctx);
>> @@ -1343,6 +1504,9 @@ static void kcryptd_crypt_read_convert(struct dm_crypt_io *io)
>>
>> crypt_convert_init(cc, &io->ctx, io->base_bio, io->base_bio,
>> io->sector);
>> + r = crypt_convert_alloc_table(cc, &io->ctx);
>> + if (r < 0)
>> + io->error = -EIO;
>>
>> r = crypt_convert(cc, &io->ctx);
>> if (r < 0)
>> --
>> 1.7.9.5
>>
>> --
>> dm-devel mailing list
>> dm-devel@redhat.com
>> https://www.redhat.com/mailman/listinfo/dm-devel
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: AES-NI: slower than aes-generic?
From: Theodore Ts'o @ 2016-05-27 2:14 UTC (permalink / raw)
To: Stephan Mueller; +Cc: Sandy Harris, Stephan Mueller, linux-crypto
In-Reply-To: <4972668.UQ1QRNriDb@positron.chronox.de>
On Thu, May 26, 2016 at 08:49:39PM +0200, Stephan Mueller wrote:
>
> Using the kernel crypto API one can relieve the CPU of the crypto work, if a
> hardware or assembler implementation is available. This may be of particular
> interest for smaller systems. So, for smaller systems (where kernel bloat is
> bad, but where now these days more and more hardware crypto support is added),
> we must weigh the kernel bloat (of 3 or 4 additional C files for the basic
> kernel crypto API logic) against relieving the CPU of work.
There are a number of caveats with using hardware acceleration; one is
that many hardware accelerators are optimized for bulk data
encryption, and so key scheduling, or switching between key schedules,
can have a higher overhead that a pure software implementation.
There have also been situations where the hardware crypto engine is
actually slower than a highly optimized software implementation. This
has been the case for certain ARM SOC's, for example.
This is not that big of deal, if you are developing a cryptographic
application (such as file system level encryption, for example) for a
specific hardware platform (such as a specific Nexus device). But if
you are trying to develop a generic service that has to work on a wide
variety of CPU architectures, and specific CPU/SOC implementations,
life is a lot more complicated. I've worked on both problems, let me
assure you the second is way tricker than the first.
> Then, the use of the DRBG offers users to choose between a Hash/HMAC and CTR
> implementation to suit their needs. The DRBG code is agnostic of the
> underlying cipher. So, you could even use Blowfish instead of AES or whirlpool
> instead of SHA -- these changes are just one entry in drbg_cores[] away
> without any code change.
I really question how much this matters in practice. Unless you are a
US Government Agency, where you might be laboring under a Federal
mandate to use DUAL-EC DRBG (for example), most users really don't
care about the details of the algorithm used in their random number
generator. Giving users choice (or lots of knobs) isn't necessarily
always a feature, as the many TLS downgrade attacks have demonstrated.
This is why from my perspectve it's more important to implement an
interface which is always there, and which by default is secure, to
minimize the chances that random JV-team kernel developers working for
a Linux distribution or some consumer electronics manufacturer won't
actually make things worse. As the Debian's attempt to "improve" the
security of OpenSSL demonstrates, it doesn't always end well. :-)
If we implement something which happens to result in a 2 minute stall
in boot times, the danger is that a clueless engineer at Sony, or LGE,
or Motorola, or BMW, or Toyota, etc, will "fix" the problem without
telling anyone about what they did, and we might not notice right away
that the fix was in fact catastrophically bad.
These aren't the standard things which academics tend to worry about,
which tend to assume that attackers will be able to read arbitrary
kernel memory, and recovering such an exposure of the entropy pool is
_the_ most important thing to worry about (as opposed to say, the
contents of the user's private keys in the ssh-agent process). But
this will perhaps explain why worrying about accomodating users who
care about whether Blowfish or AES should be used in their random
number generator isn't near the top of my personal priority list.
Cheers,
- Ted
^ permalink raw reply
* [PATCH] KEYS: Add placeholder for KDF usage with DH
From: David Howells @ 2016-05-26 22:40 UTC (permalink / raw)
To: jmorris
Cc: dhowells, Mat Martineau, Stephan Mueller, keyrings, linux-crypto,
linux-security-module
Hi James,
Could you pass this along to Linus as soon as possible, please? This
alters a new keyctl function added in the current merge window to allow for
a future extension planned for the next merge window.
Thanks,
David
---
From: Stephan Mueller <smueller@chronox.de>
Date: Thu May 26 23:38:12 2016 +0200
KEYS: Add placeholder for KDF usage with DH
The values computed during Diffie-Hellman key exchange are often used
in combination with key derivation functions to create cryptographic
keys. Add a placeholder for a later implementation to configure a
key derivation function that will transform the Diffie-Hellman
result returned by the KEYCTL_DH_COMPUTE command.
[This patch was stripped down from a patch produced by Mat Martineau that
had a bug in the compat code - so for the moment Stephan's patch simply
requires that the placeholder argument must be NULL]
Original-signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
Signed-off-by: David Howells <dhowells@redhat.com>
---
Documentation/security/keys.txt | 5 ++++-
security/keys/compat.c | 2 +-
security/keys/dh.c | 8 +++++++-
security/keys/internal.h | 5 +++--
security/keys/keyctl.c | 4 ++--
5 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index 20d05719bceb..3849814bfe6d 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -826,7 +826,8 @@ The keyctl syscall functions are:
(*) Compute a Diffie-Hellman shared secret or public key
long keyctl(KEYCTL_DH_COMPUTE, struct keyctl_dh_params *params,
- char *buffer, size_t buflen);
+ char *buffer, size_t buflen,
+ void *reserved);
The params struct contains serial numbers for three keys:
@@ -843,6 +844,8 @@ The keyctl syscall functions are:
public key. If the base is the remote public key, the result is
the shared secret.
+ The reserved argument must be set to NULL.
+
The buffer length must be at least the length of the prime, or zero.
If the buffer length is nonzero, the length of the result is
diff --git a/security/keys/compat.c b/security/keys/compat.c
index c8783b3b628c..36c80bf5b89c 100644
--- a/security/keys/compat.c
+++ b/security/keys/compat.c
@@ -134,7 +134,7 @@ COMPAT_SYSCALL_DEFINE5(keyctl, u32, option,
case KEYCTL_DH_COMPUTE:
return keyctl_dh_compute(compat_ptr(arg2), compat_ptr(arg3),
- arg4);
+ arg4, compat_ptr(arg5));
default:
return -EOPNOTSUPP;
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 880505a4b9f1..531ed2ec132f 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -78,7 +78,8 @@ error:
}
long keyctl_dh_compute(struct keyctl_dh_params __user *params,
- char __user *buffer, size_t buflen)
+ char __user *buffer, size_t buflen,
+ void __user *reserved)
{
long ret;
MPI base, private, prime, result;
@@ -97,6 +98,11 @@ long keyctl_dh_compute(struct keyctl_dh_params __user *params,
goto out;
}
+ if (reserved) {
+ ret = -EINVAL;
+ goto out;
+ }
+
keylen = mpi_from_key(pcopy.prime, buflen, &prime);
if (keylen < 0 || !prime) {
/* buflen == 0 may be used to query the required buffer size,
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 8ec7a528365d..a705a7d92ad7 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -260,10 +260,11 @@ static inline long keyctl_get_persistent(uid_t uid, key_serial_t destring)
#ifdef CONFIG_KEY_DH_OPERATIONS
extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *,
- size_t);
+ size_t, void __user *);
#else
static inline long keyctl_dh_compute(struct keyctl_dh_params __user *params,
- char __user *buffer, size_t buflen)
+ char __user *buffer, size_t buflen,
+ void __user *reserved)
{
return -EOPNOTSUPP;
}
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 3b135a0af344..d580ad06b792 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1688,8 +1688,8 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
case KEYCTL_DH_COMPUTE:
return keyctl_dh_compute((struct keyctl_dh_params __user *) arg2,
- (char __user *) arg3,
- (size_t) arg4);
+ (char __user *) arg3, (size_t) arg4,
+ (void __user *) arg5);
default:
return -EOPNOTSUPP;
^ permalink raw reply related
* [PATCH v2] KEYS: Add placeholder KDF usage with DH
From: Stephan Mueller @ 2016-05-26 21:38 UTC (permalink / raw)
To: David Howells; +Cc: Mat Martineau, keyrings, linux-crypto
In-Reply-To: <23038.1464293450@warthog.procyon.org.uk>
Am Donnerstag, 26. Mai 2016, 21:10:50 schrieb David Howells:
Hi David,
Mat asked me to step in in case his patch needed an update.
---8<---
The values computed during Diffie-Hellman key exchange are often used
in combination with key derivation functions to create cryptographic
keys. Add a placeholder for a later implementation to configure a
key derivation function that will transform the Diffie-Hellman
result returned by the KEYCTL_DH_COMPUTE command.
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
Documentation/security/keys.txt | 5 ++++-
security/keys/compat.c | 2 +-
security/keys/dh.c | 8 +++++++-
security/keys/internal.h | 5 +++--
security/keys/keyctl.c | 4 ++--
5 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/Documentation/security/keys.txt b/Documentation/security/keys.txt
index 20d0571..3849814 100644
--- a/Documentation/security/keys.txt
+++ b/Documentation/security/keys.txt
@@ -826,7 +826,8 @@ The keyctl syscall functions are:
(*) Compute a Diffie-Hellman shared secret or public key
long keyctl(KEYCTL_DH_COMPUTE, struct keyctl_dh_params *params,
- char *buffer, size_t buflen);
+ char *buffer, size_t buflen,
+ void *reserved);
The params struct contains serial numbers for three keys:
@@ -843,6 +844,8 @@ The keyctl syscall functions are:
public key. If the base is the remote public key, the result is
the shared secret.
+ The reserved argument must be set to NULL.
+
The buffer length must be at least the length of the prime, or zero.
If the buffer length is nonzero, the length of the result is
diff --git a/security/keys/compat.c b/security/keys/compat.c
index c8783b3..36c80bf 100644
--- a/security/keys/compat.c
+++ b/security/keys/compat.c
@@ -134,7 +134,7 @@ COMPAT_SYSCALL_DEFINE5(keyctl, u32, option,
case KEYCTL_DH_COMPUTE:
return keyctl_dh_compute(compat_ptr(arg2), compat_ptr(arg3),
- arg4);
+ arg4, compat_ptr(arg5));
default:
return -EOPNOTSUPP;
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 880505a..531ed2e 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -78,7 +78,8 @@ error:
}
long keyctl_dh_compute(struct keyctl_dh_params __user *params,
- char __user *buffer, size_t buflen)
+ char __user *buffer, size_t buflen,
+ void __user *reserved)
{
long ret;
MPI base, private, prime, result;
@@ -97,6 +98,11 @@ long keyctl_dh_compute(struct keyctl_dh_params __user *params,
goto out;
}
+ if (reserved) {
+ ret = -EINVAL;
+ goto out;
+ }
+
keylen = mpi_from_key(pcopy.prime, buflen, &prime);
if (keylen < 0 || !prime) {
/* buflen == 0 may be used to query the required buffer size,
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 8ec7a52..a705a7d 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -260,10 +260,11 @@ static inline long keyctl_get_persistent(uid_t uid, key_serial_t destring)
#ifdef CONFIG_KEY_DH_OPERATIONS
extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *,
- size_t);
+ size_t, void __user *);
#else
static inline long keyctl_dh_compute(struct keyctl_dh_params __user *params,
- char __user *buffer, size_t buflen)
+ char __user *buffer, size_t buflen,
+ void __user *reserved)
{
return -EOPNOTSUPP;
}
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 3b135a0..d580ad0 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1688,8 +1688,8 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
case KEYCTL_DH_COMPUTE:
return keyctl_dh_compute((struct keyctl_dh_params __user *) arg2,
- (char __user *) arg3,
- (size_t) arg4);
+ (char __user *) arg3, (size_t) arg4,
+ (void __user *) arg5);
default:
return -EOPNOTSUPP;
--
2.5.5
^ permalink raw reply related
* RE: [PATCH 2/7] crypto : async implementation for sha1-mb
From: Yu, Fenghua @ 2016-05-26 21:25 UTC (permalink / raw)
To: Dey, Megha, herbert@gondor.apana.org.au
Cc: tim.c.chen@linux.intel.com, davem@davemloft.net,
linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
Megha Dey
In-Reply-To: <1463705008-2183-1-git-send-email-megha.dey@intel.com>
> From: Dey, Megha
> Sent: Thursday, May 19, 2016 5:43 PM
> To: herbert@gondor.apana.org.au
> Cc: tim.c.chen@linux.intel.com; davem@davemloft.net; linux-
> crypto@vger.kernel.org; linux-kernel@vger.kernel.org; Dey, Megha
> <megha.dey@intel.com>; Yu, Fenghua <fenghua.yu@intel.com>; Megha
> Dey <megha.dey@linux.intel.com>
> Subject: [PATCH 2/7] crypto : async implementation for sha1-mb
>
> From: Megha Dey <megha.dey@linux.intel.com>
>
> Herbert wants the sha1-mb algorithm to have an async implementation:
> https://lkml.org/lkml/2016/4/5/286.
> Currently, sha1-mb uses an async interface for the outer algorithm and a sync
> interface for the inner algorithm. This patch introduces a async interface for
> even the inner algorithm.
>
> Signed-off-by: Megha Dey <megha.dey@linux.intel.com>
> Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
> ---
> arch/x86/crypto/sha-mb/sha1_mb.c | 190 ++++++++++++++++++++++------
> -----------
> crypto/ahash.c | 6 --
> crypto/mcryptd.c | 117 +++++++++++++-----------
> include/crypto/hash.h | 6 ++
> include/crypto/internal/hash.h | 8 +-
> include/crypto/mcryptd.h | 8 +-
> 6 files changed, 184 insertions(+), 151 deletions(-)
>
Hi, Herbert,
Any comment/feedback on this patch set? Is there any plan to push it to upstream?
Thanks.
-Fenghua
^ permalink raw reply
* [PATCH 0/5] refactor mpi_read_from_buffer()
From: Nicolai Stange @ 2016-05-26 21:19 UTC (permalink / raw)
To: Herbert Xu
Cc: David Howells, Tadeusz Struk, Michal Marek, linux-crypto,
linux-kernel, Nicolai Stange
mpi_read_from_buffer() and mpi_read_raw_data() do almost the same and share a
fair amount of common code.
This patchset attempts to rewrite mpi_read_from_buffer() in order to implement
it in terms of mpi_read_raw_data().
The patches 1 and 3, i.e.
"lib/mpi: mpi_read_from_buffer(): return error code"
and
"lib/mpi: mpi_read_from_buffer(): return -EINVAL upon too short buffer"
do the groundwork in that they move any error detection unique to
mpi_read_from_buffer() out of the data handling loop.
The patches 2 and 4, that is
"lib/digsig: digsig_verify_rsa(): return -EINVAL if modulo length is zero"
and
"lib/mpi: mpi_read_from_buffer(): sanitize short buffer printk"
are not strictly necessary for the refactoring: they cleanup some minor oddities
related to error handling I came across.
Finally, the last patch in this series,
"lib/mpi: refactor mpi_read_from_buffer() in terms of mpi_read_raw_data()"
actually does what this series is all about.
Applicable to linux-next-20160325.
A note on testing: allmodconfig on x86_64 builds fine.
However, the only caller of mpi_read_from_buffer() is digsig_verify_rsa()
and this one is solely used by the IMA/EVM infrastructure.
In my current setup, I don't have any IMA/EVM stuff in place and thus,
I can't do any runtime tests without putting *much* effort into it.
I would really appreciate if someone with a working IMA/EVM setup could do some
brief testing...
Nicolai Stange (5):
lib/mpi: mpi_read_from_buffer(): return error code
lib/digsig: digsig_verify_rsa(): return -EINVAL if modulo length is
zero
lib/mpi: mpi_read_from_buffer(): return -EINVAL upon too short buffer
lib/mpi: mpi_read_from_buffer(): sanitize short buffer printk
lib/mpi: refactor mpi_read_from_buffer() in terms of
mpi_read_raw_data()
lib/digsig.c | 16 +++++++++++-----
lib/mpi/mpicoder.c | 46 +++++++++++++---------------------------------
2 files changed, 24 insertions(+), 38 deletions(-)
--
2.8.2
^ permalink raw reply
* [PATCH 5/5] lib/mpi: refactor mpi_read_from_buffer() in terms of mpi_read_raw_data()
From: Nicolai Stange @ 2016-05-26 21:19 UTC (permalink / raw)
To: Herbert Xu
Cc: David Howells, Tadeusz Struk, Michal Marek, linux-crypto,
linux-kernel, Nicolai Stange
In-Reply-To: <1464297595-24032-1-git-send-email-nicstange@gmail.com>
mpi_read_from_buffer() and mpi_read_raw_data() do basically the same thing
except that the former extracts the number of payload bits from the first
two bytes of the input buffer.
Besides that, the data copying logic is exactly the same.
Replace the open coded buffer to MPI instance conversion by a call to
mpi_read_raw_data().
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
lib/mpi/mpicoder.c | 24 +++---------------------
1 file changed, 3 insertions(+), 21 deletions(-)
diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 2f4d039..e8a5742 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -82,10 +82,8 @@ EXPORT_SYMBOL_GPL(mpi_read_raw_data);
MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
{
const uint8_t *buffer = xbuffer;
- int i, j;
- unsigned nbits, nbytes, nlimbs;
- mpi_limb_t a;
- MPI val = NULL;
+ unsigned int nbits, nbytes;
+ MPI val;
if (*ret_nread < 2)
return ERR_PTR(-EINVAL);
@@ -95,7 +93,6 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
pr_info("MPI: mpi too large (%u bits)\n", nbits);
return ERR_PTR(-EINVAL);
}
- buffer += 2;
nbytes = DIV_ROUND_UP(nbits, 8);
if (nbytes + 2 > *ret_nread) {
@@ -104,24 +101,9 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
return ERR_PTR(-EINVAL);
}
- nlimbs = DIV_ROUND_UP(nbytes, BYTES_PER_MPI_LIMB);
- val = mpi_alloc(nlimbs);
+ val = mpi_read_raw_data(buffer + 2, nbytes);
if (!val)
return ERR_PTR(-ENOMEM);
- i = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB;
- i %= BYTES_PER_MPI_LIMB;
- val->nbits = nbits;
- j = val->nlimbs = nlimbs;
- val->sign = 0;
- for (; j > 0; j--) {
- a = 0;
- for (; i < BYTES_PER_MPI_LIMB; i++) {
- a <<= 8;
- a |= *buffer++;
- }
- i = 0;
- val->d[j - 1] = a;
- }
*ret_nread = nbytes + 2;
return val;
--
2.8.2
^ permalink raw reply related
* [PATCH 4/5] lib/mpi: mpi_read_from_buffer(): sanitize short buffer printk
From: Nicolai Stange @ 2016-05-26 21:19 UTC (permalink / raw)
To: Herbert Xu
Cc: David Howells, Tadeusz Struk, Michal Marek, linux-crypto,
linux-kernel, Nicolai Stange
In-Reply-To: <1464297595-24032-1-git-send-email-nicstange@gmail.com>
The first two bytes of the input buffer encode its expected length and
mpi_read_from_buffer() prints a console message if the given buffer is too
short.
However, there are some oddities with how this message is printed:
- It is printed at the default loglevel. This is different from the
one used in the case that the first two bytes' value is unsupportedly
large, i.e. KERN_INFO.
- The format specifier '%d' is used for unsigned ints.
- It prints the values of nread and *ret_nread. This is redundant since
the former is always the latter + 1.
Clean this up as follows:
- Use pr_info() rather than printk() with no loglevel.
- Use the format specifiers '%u' in place if '%d'.
- Do not print the redundant 'nread' but the more helpful 'nbytes' value.
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
lib/mpi/mpicoder.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 869c66c..2f4d039 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -99,8 +99,8 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
nbytes = DIV_ROUND_UP(nbits, 8);
if (nbytes + 2 > *ret_nread) {
- printk("MPI: mpi larger than buffer nread=%d ret_nread=%d\n",
- *ret_nread + 1, *ret_nread);
+ pr_info("MPI: mpi larger than buffer nbytes=%u ret_nread=%u\n",
+ nbytes, *ret_nread);
return ERR_PTR(-EINVAL);
}
--
2.8.2
^ permalink raw reply related
* [PATCH 3/5] lib/mpi: mpi_read_from_buffer(): return -EINVAL upon too short buffer
From: Nicolai Stange @ 2016-05-26 21:19 UTC (permalink / raw)
To: Herbert Xu
Cc: David Howells, Tadeusz Struk, Michal Marek, linux-crypto,
linux-kernel, Nicolai Stange
In-Reply-To: <1464297595-24032-1-git-send-email-nicstange@gmail.com>
Currently, if the input buffer is shorter than the expected length as
indicated by its first two bytes, an MPI instance of this expected length
will be allocated and filled with as much data as is available. The rest
will remain uninitialized.
Instead of leaving this condition undetected, an error code should be
reported to the caller.
Since this situation indicates that the input buffer's first two bytes,
encoding the number of expected bits, are garbled, -EINVAL is appropriate
here.
If the input buffer is shorter than indicated by its first two bytes,
make mpi_read_from_buffer() return -EINVAL.
Get rid of the 'nread' variable: with the new semantics, the total number
of bytes read from the input buffer is known in advance.
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
lib/mpi/mpicoder.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 275c71e..869c66c 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -83,7 +83,7 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
{
const uint8_t *buffer = xbuffer;
int i, j;
- unsigned nbits, nbytes, nlimbs, nread = 0;
+ unsigned nbits, nbytes, nlimbs;
mpi_limb_t a;
MPI val = NULL;
@@ -96,9 +96,14 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
return ERR_PTR(-EINVAL);
}
buffer += 2;
- nread = 2;
nbytes = DIV_ROUND_UP(nbits, 8);
+ if (nbytes + 2 > *ret_nread) {
+ printk("MPI: mpi larger than buffer nread=%d ret_nread=%d\n",
+ *ret_nread + 1, *ret_nread);
+ return ERR_PTR(-EINVAL);
+ }
+
nlimbs = DIV_ROUND_UP(nbytes, BYTES_PER_MPI_LIMB);
val = mpi_alloc(nlimbs);
if (!val)
@@ -111,12 +116,6 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
for (; j > 0; j--) {
a = 0;
for (; i < BYTES_PER_MPI_LIMB; i++) {
- if (++nread > *ret_nread) {
- printk
- ("MPI: mpi larger than buffer nread=%d ret_nread=%d\n",
- nread, *ret_nread);
- goto leave;
- }
a <<= 8;
a |= *buffer++;
}
@@ -124,8 +123,7 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
val->d[j - 1] = a;
}
-leave:
- *ret_nread = nread;
+ *ret_nread = nbytes + 2;
return val;
}
EXPORT_SYMBOL_GPL(mpi_read_from_buffer);
--
2.8.2
^ permalink raw reply related
* [PATCH 2/5] lib/digsig: digsig_verify_rsa(): return -EINVAL if modulo length is zero
From: Nicolai Stange @ 2016-05-26 21:19 UTC (permalink / raw)
To: Herbert Xu
Cc: David Howells, Tadeusz Struk, Michal Marek, linux-crypto,
linux-kernel, Nicolai Stange
In-Reply-To: <1464297595-24032-1-git-send-email-nicstange@gmail.com>
Currently, if digsig_verify_rsa() detects that the modulo's length is zero,
i.e. mlen == 0, it returns -ENOMEM which doesn't really fit here.
Make digsig_verify_rsa() return -EINVAL upon mlen == 0.
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
lib/digsig.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/lib/digsig.c b/lib/digsig.c
index a121cbc..55b8b2f 100644
--- a/lib/digsig.c
+++ b/lib/digsig.c
@@ -114,13 +114,15 @@ static int digsig_verify_rsa(struct key *key,
datap += remaining;
}
- err = -ENOMEM;
-
mblen = mpi_get_nbits(pkey[0]);
mlen = DIV_ROUND_UP(mblen, 8);
- if (mlen == 0)
+ if (mlen == 0) {
+ err = -EINVAL;
goto err;
+ }
+
+ err = -ENOMEM;
out1 = kzalloc(mlen, GFP_KERNEL);
if (!out1)
--
2.8.2
^ permalink raw reply related
* [PATCH 1/5] lib/mpi: mpi_read_from_buffer(): return error code
From: Nicolai Stange @ 2016-05-26 21:19 UTC (permalink / raw)
To: Herbert Xu
Cc: David Howells, Tadeusz Struk, Michal Marek, linux-crypto,
linux-kernel, Nicolai Stange
In-Reply-To: <1464297595-24032-1-git-send-email-nicstange@gmail.com>
mpi_read_from_buffer() reads a MPI from a buffer into a newly allocated
MPI instance. It expects the buffer's leading two bytes to contain the
number of bits, followed by the actual payload.
On failure, it returns NULL and updates the in/out argument ret_nread
somewhat inconsistently:
- If the given buffer is too short to contain the leading two bytes
encoding the number of bits or their value is unsupported, then
ret_nread will be cleared.
- If the allocation of the resulting MPI instance fails, ret_nread is left
as is.
The only user of mpi_read_from_buffer(), digsig_verify_rsa(), simply checks
for a return value of NULL and returns -ENOMEM if that happens.
While this is all of cosmetic nature only, there is another error condition
which currently isn't detectable by the caller of mpi_read_from_buffer():
if the given buffer is too small to hold the number of bits as encoded in
its first two bytes, the return value will be non-NULL and *ret_nread > 0.
In preparation of communicating this condition to the caller, let
mpi_read_from_buffer() return error values by means of the ERR_PTR()
mechanism.
Make the sole caller of mpi_read_from_buffer(), digsig_verify_rsa(),
check the return value for IS_ERR() rather than == NULL. If IS_ERR() is
true, return the associated error value rather than the fixed -ENOMEM.
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
---
lib/digsig.c | 12 ++++++++----
lib/mpi/mpicoder.c | 6 +++---
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/lib/digsig.c b/lib/digsig.c
index 07be6c1..a121cbc 100644
--- a/lib/digsig.c
+++ b/lib/digsig.c
@@ -104,16 +104,18 @@ static int digsig_verify_rsa(struct key *key,
datap = pkh->mpi;
endp = ukp->data + ukp->datalen;
- err = -ENOMEM;
-
for (i = 0; i < pkh->nmpi; i++) {
unsigned int remaining = endp - datap;
pkey[i] = mpi_read_from_buffer(datap, &remaining);
- if (!pkey[i])
+ if (IS_ERR(pkey[i])) {
+ err = PTR_ERR(pkey[i]);
goto err;
+ }
datap += remaining;
}
+ err = -ENOMEM;
+
mblen = mpi_get_nbits(pkey[0]);
mlen = DIV_ROUND_UP(mblen, 8);
@@ -126,8 +128,10 @@ static int digsig_verify_rsa(struct key *key,
nret = siglen;
in = mpi_read_from_buffer(sig, &nret);
- if (!in)
+ if (IS_ERR(in)) {
+ err = PTR_ERR(in);
goto err;
+ }
res = mpi_alloc(mpi_get_nlimbs(in) * 2);
if (!res)
diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c
index 747606f..275c71e 100644
--- a/lib/mpi/mpicoder.c
+++ b/lib/mpi/mpicoder.c
@@ -88,12 +88,12 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
MPI val = NULL;
if (*ret_nread < 2)
- goto leave;
+ return ERR_PTR(-EINVAL);
nbits = buffer[0] << 8 | buffer[1];
if (nbits > MAX_EXTERN_MPI_BITS) {
pr_info("MPI: mpi too large (%u bits)\n", nbits);
- goto leave;
+ return ERR_PTR(-EINVAL);
}
buffer += 2;
nread = 2;
@@ -102,7 +102,7 @@ MPI mpi_read_from_buffer(const void *xbuffer, unsigned *ret_nread)
nlimbs = DIV_ROUND_UP(nbytes, BYTES_PER_MPI_LIMB);
val = mpi_alloc(nlimbs);
if (!val)
- return NULL;
+ return ERR_PTR(-ENOMEM);
i = BYTES_PER_MPI_LIMB - nbytes % BYTES_PER_MPI_LIMB;
i %= BYTES_PER_MPI_LIMB;
val->nbits = nbits;
--
2.8.2
^ permalink raw reply related
* Re: [PATCH] KEYS: Add optional key derivation parameters for DH
From: David Howells @ 2016-05-26 20:10 UTC (permalink / raw)
To: Mat Martineau; +Cc: dhowells, smueller, keyrings, linux-crypto
In-Reply-To: <20160526165741.2634-1-mathew.j.martineau@linux.intel.com>
Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
> +struct keyctl_kdf_params {
> + char *name;
> + __u8 reserved[32]; /* Reserved for future use, must be 0 */
> +};
> +
> #endif /* _LINUX_KEYCTL_H */
> diff --git a/security/keys/compat.c b/security/keys/compat.c
> index c8783b3..36c80bf 100644
> --- a/security/keys/compat.c
> +++ b/security/keys/compat.c
> @@ -134,7 +134,7 @@ COMPAT_SYSCALL_DEFINE5(keyctl, u32, option,
>
> case KEYCTL_DH_COMPUTE:
> return keyctl_dh_compute(compat_ptr(arg2), compat_ptr(arg3),
> - arg4);
> + arg4, compat_ptr(arg5));
Given the new structure above, this won't work. The problem is that on a
64-bit system the kernel expects 'name' to be a 64-bit pointer, but if we're
in the compat handler, we have a 32-bit userspace's idea of the struct - in
which 'name' is a 31-bit (s390x) or a 32-bit pointer without any padding.
So in compat code you can't just pass the user pointer direct through to
keyctl_dh_compute(). You need to supply a compat_keyctl_kdf_params struct and
translator code.
What I would recommend you do at the moment is to mark the syscall argument as
"reserved, must be 0" and deal with the implementation in the next merge
window.
David
^ 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