2013-09-11 06:14:43

by [email protected]

Subject: [PATCH] drivers/crypto:caam:Map src buffer before access in CAAM driver

From: Yashpal Dutta <[email protected]>

KMap the buffers before copying trailing bytes during hmac in CAAM driver into a
session temporary buffer. This is required if pinned buffer from user-space
is send to CAAM driver during hmac and is safe even if hmac request is generated
from within kernel.

Signed-off-by: Yashpal Dutta <[email protected]>
---
drivers/crypto/caam/sg_sw_sec4.h | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/caam/sg_sw_sec4.h b/drivers/crypto/caam/sg_sw_sec4.h
index e0037c8..9575985 100644
--- a/drivers/crypto/caam/sg_sw_sec4.h
+++ b/drivers/crypto/caam/sg_sw_sec4.h
@@ -117,6 +117,21 @@ static int dma_unmap_sg_chained(struct device *dev, struct scatterlist *sg,
return nents;
}

+/* Map SG page in kernel virtual address space and copy */
+static inline void sg_map_copy(u8 *dest, struct scatterlist *sg,
+ int len, int offset)
+{
+ u8 *mapped_addr;
+
+ /*
+ * Page here can be user-space pinned using get_user_pages
+ * Same must be kmapped before use and kunmapped subsequently
+ */
+ mapped_addr = kmap_atomic(sg_page(sg));
+ memcpy(dest, mapped_addr + offset, len);
+ kunmap_atomic(mapped_addr);
+}
+
/* Copy from len bytes of sg to dest, starting from beginning */
static inline void sg_copy(u8 *dest, struct scatterlist *sg, unsigned int len)
{
@@ -124,15 +139,15 @@ static inline void sg_copy(u8 *dest, struct scatterlist *sg, unsigned int len)
int cpy_index = 0, next_cpy_index = current_sg->length;

while (next_cpy_index < len) {
- memcpy(dest + cpy_index, (u8 *) sg_virt(current_sg),
- current_sg->length);
+ sg_map_copy(dest + cpy_index, current_sg, current_sg->length,
+ current_sg->offset);
current_sg = scatterwalk_sg_next(current_sg);
cpy_index = next_cpy_index;
next_cpy_index += current_sg->length;
}
if (cpy_index < len)
- memcpy(dest + cpy_index, (u8 *) sg_virt(current_sg),
- len - cpy_index);
+ sg_map_copy(dest + cpy_index, current_sg, len-cpy_index,
+ current_sg->offset);
}

/* Copy sg data, from to_skip to end, to dest */
@@ -140,7 +155,7 @@ static inline void sg_copy_part(u8 *dest, struct scatterlist *sg,
int to_skip, unsigned int end)
{
struct scatterlist *current_sg = sg;
- int sg_index, cpy_index;
+ int sg_index, cpy_index, offset;

sg_index = current_sg->length;
while (sg_index <= to_skip) {
@@ -148,9 +163,10 @@ static inline void sg_copy_part(u8 *dest, struct scatterlist *sg,
sg_index += current_sg->length;
}
cpy_index = sg_index - to_skip;
- memcpy(dest, (u8 *) sg_virt(current_sg) +
- current_sg->length - cpy_index, cpy_index);
- current_sg = scatterwalk_sg_next(current_sg);
- if (end - sg_index)
+ offset = current_sg->offset + current_sg->length - cpy_index;
+ sg_map_copy(dest, current_sg, cpy_index, offset);
+ if (end - sg_index) {
+ current_sg = scatterwalk_sg_next(current_sg);
sg_copy(dest + cpy_index, current_sg, end - sg_index);
+ }
}
--
1.7.10.4

2013-09-11 12:54:44

by Horia Geantă

Subject: Re: [PATCH] drivers/crypto:caam:Map src buffer before access in CAAM driver

On 9/11/2013 9:02 AM, [email protected] wrote:
> From: Yashpal Dutta <[email protected]>
>
> KMap the buffers before copying trailing bytes during hmac in CAAM driver into a
> session temporary buffer. This is required if pinned buffer from user-space
> is send to CAAM driver during hmac and is safe even if hmac request is generated
> from within kernel.
>
> Signed-off-by: Yashpal Dutta <[email protected]>
> ---

Subject not consistent with previous caam driver commits. Prefix should
be "crypto: caam - ".
I would drop the "in CAAM driver" since it's redundant - the prefix
already mentions this.

> @@ -124,15 +139,15 @@ static inline void sg_copy(u8 *dest, struct scatterlist *sg, unsigned int len)
> int cpy_index = 0, next_cpy_index = current_sg->length;
>
> while (next_cpy_index < len) {
> - memcpy(dest + cpy_index, (u8 *) sg_virt(current_sg),
> - current_sg->length);
> + sg_map_copy(dest + cpy_index, current_sg, current_sg->length,
> + current_sg->offset);

CHECK: Alignment should match open parenthesis
#49: FILE: drivers/crypto/caam/sg_sw_sec4.h:143:
+ sg_map_copy(dest + cpy_index, current_sg,
current_sg->length,
+ current_sg->offset);

> current_sg = scatterwalk_sg_next(current_sg);
> cpy_index = next_cpy_index;
> next_cpy_index += current_sg->length;
> }
> if (cpy_index < len)
> - memcpy(dest + cpy_index, (u8 *) sg_virt(current_sg),
> - len - cpy_index);
> + sg_map_copy(dest + cpy_index, current_sg, len-cpy_index,
> + current_sg->offset);

CHECK: Alignment should match open parenthesis
#58: FILE: drivers/crypto/caam/sg_sw_sec4.h:150:
+ sg_map_copy(dest + cpy_index, current_sg, len-cpy_index,
+ current_sg->offset);