Ensure scatterlists have a virtual memory mapping before dumping.
Signed-off-by: Catalin Vasile <[email protected]>
---
Changes:
V2:
* resolved issue of sleeping in atomic contexts
---
---
drivers/crypto/caam/caamalg.c | 79 +++++++++++++++++++++++++++++++++----------
1 file changed, 61 insertions(+), 18 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index f1116e7..eb97562 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -111,6 +111,42 @@
#else
#define debug(format, arg...)
#endif
+
+#ifdef DEBUG
+#include <linux/highmem.h>
+
+static void dbg_dump_sg(const char *level, const char *prefix_str,
+ int prefix_type, int rowsize, int groupsize,
+ struct scatterlist *sg, size_t tlen, bool ascii,
+ bool may_sleep)
+{
+ struct scatterlist *it;
+ void *it_page;
+ size_t len;
+ void *buf;
+
+ for (it = sg; it != NULL && tlen > 0 ; it = sg_next(sg)) {
+ /*
+ * make sure the scatterlist's page
+ * has a valid virtual memory mapping
+ */
+ it_page = kmap_atomic(sg_page(it));
+ if (unlikely(!it_page)) {
+ printk(KERN_ERR "dbg_dump_sg: kmap failed\n");
+ return;
+ }
+
+ buf = it_page + it->offset;
+ len = min(tlen, it->length);
+ print_hex_dump(level, prefix_str, prefix_type, rowsize,
+ groupsize, buf, len, ascii);
+ tlen -= len;
+
+ kunmap_atomic(it_page);
+ }
+}
+#endif
+
static struct list_head alg_list;
struct caam_alg_entry {
@@ -1982,9 +2018,9 @@ static void ablkcipher_encrypt_done(struct device *jrdev, u32 *desc, u32 err,
print_hex_dump(KERN_ERR, "dstiv @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->info,
edesc->src_nents > 1 ? 100 : ivsize, 1);
- print_hex_dump(KERN_ERR, "dst @"__stringify(__LINE__)": ",
- DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
- edesc->dst_nents > 1 ? 100 : req->nbytes, 1);
+ dbg_dump_sg(KERN_ERR, "dst @"__stringify(__LINE__)": ",
+ DUMP_PREFIX_ADDRESS, 16, 4, req->dst,
+ edesc->dst_nents > 1 ? 100 : req->nbytes, 1, true);
#endif
ablkcipher_unmap(jrdev, edesc, req);
@@ -2014,9 +2050,9 @@ static void ablkcipher_decrypt_done(struct device *jrdev, u32 *desc, u32 err,
print_hex_dump(KERN_ERR, "dstiv @"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->info,
ivsize, 1);
- print_hex_dump(KERN_ERR, "dst @"__stringify(__LINE__)": ",
- DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
- edesc->dst_nents > 1 ? 100 : req->nbytes, 1);
+ dbg_dump_sg(KERN_ERR, "dst @"__stringify(__LINE__)": ",
+ DUMP_PREFIX_ADDRESS, 16, 4, req->dst,
+ edesc->dst_nents > 1 ? 100 : req->nbytes, 1, true);
#endif
ablkcipher_unmap(jrdev, edesc, req);
@@ -2171,12 +2207,15 @@ static void init_ablkcipher_job(u32 *sh_desc, dma_addr_t ptr,
int len, sec4_sg_index = 0;
#ifdef DEBUG
+ bool may_sleep = ((req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
+ CRYPTO_TFM_REQ_MAY_SLEEP)) != 0);
print_hex_dump(KERN_ERR, "presciv@"__stringify(__LINE__)": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->info,
ivsize, 1);
- print_hex_dump(KERN_ERR, "src @"__stringify(__LINE__)": ",
- DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
- edesc->src_nents ? 100 : req->nbytes, 1);
+ printk(KERN_ERR "asked=%d, nbytes%d\n", (int)edesc->src_nents ? 100 : req->nbytes, req->nbytes);
+ dbg_dump_sg(KERN_ERR, "src @"__stringify(__LINE__)": ",
+ DUMP_PREFIX_ADDRESS, 16, 4, req->src,
+ edesc->src_nents ? 100 : req->nbytes, 1, may_sleep);
#endif
len = desc_len(sh_desc);
@@ -2228,12 +2267,14 @@ static void init_ablkcipher_giv_job(u32 *sh_desc, dma_addr_t ptr,
int len, sec4_sg_index = 0;
#ifdef DEBUG
+ bool may_sleep = ((req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
+ CRYPTO_TFM_REQ_MAY_SLEEP)) != 0);
print_hex_dump(KERN_ERR, "presciv@" __stringify(__LINE__) ": ",
DUMP_PREFIX_ADDRESS, 16, 4, req->info,
ivsize, 1);
- print_hex_dump(KERN_ERR, "src @" __stringify(__LINE__) ": ",
- DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
- edesc->src_nents ? 100 : req->nbytes, 1);
+ dbg_dump_sg(KERN_ERR, "src @" __stringify(__LINE__) ": ",
+ DUMP_PREFIX_ADDRESS, 16, 4, req->src,
+ edesc->src_nents ? 100 : req->nbytes, 1, may_sleep);
#endif
len = desc_len(sh_desc);
@@ -2503,18 +2544,20 @@ static int aead_decrypt(struct aead_request *req)
u32 *desc;
int ret = 0;
+#ifdef DEBUG
+ bool may_sleep = ((req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
+ CRYPTO_TFM_REQ_MAY_SLEEP)) != 0);
+ dbg_dump_sg(KERN_ERR, "dec src@"__stringify(__LINE__)": ",
+ DUMP_PREFIX_ADDRESS, 16, 4, req->src,
+ req->assoclen + req->cryptlen, 1, may_sleep);
+#endif
+
/* allocate extended descriptor */
edesc = aead_edesc_alloc(req, AUTHENC_DESC_JOB_IO_LEN,
&all_contig, false);
if (IS_ERR(edesc))
return PTR_ERR(edesc);
-#ifdef DEBUG
- print_hex_dump(KERN_ERR, "dec src@"__stringify(__LINE__)": ",
- DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
- req->assoclen + req->cryptlen, 1);
-#endif
-
/* Create and submit job descriptor*/
init_authenc_job(req, edesc, all_contig, false);
#ifdef DEBUG
--
1.8.3.1
On 9/22/2016 11:58 AM, Catalin Vasile wrote:
> Ensure scatterlists have a virtual memory mapping before dumping.
>
> Signed-off-by: Catalin Vasile <[email protected]>
> ---
> Changes:
> V2:
> * resolved issue of sleeping in atomic contexts
> ---
> ---
> drivers/crypto/caam/caamalg.c | 79 +++++++++++++++++++++++++++++++++----------
> 1 file changed, 61 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
> index f1116e7..eb97562 100644
> --- a/drivers/crypto/caam/caamalg.c
> +++ b/drivers/crypto/caam/caamalg.c
> @@ -111,6 +111,42 @@
> #else
> #define debug(format, arg...)
> #endif
> +
> +#ifdef DEBUG
> +#include <linux/highmem.h>
> +
> +static void dbg_dump_sg(const char *level, const char *prefix_str,
> + int prefix_type, int rowsize, int groupsize,
> + struct scatterlist *sg, size_t tlen, bool ascii,
> + bool may_sleep)
may_sleep is no longer needed.
> +{
> + struct scatterlist *it;
> + void *it_page;
> + size_t len;
> + void *buf;
> +
> + for (it = sg; it != NULL && tlen > 0 ; it = sg_next(sg)) {
> + /*
> + * make sure the scatterlist's page
> + * has a valid virtual memory mapping
> + */
> + it_page = kmap_atomic(sg_page(it));
> + if (unlikely(!it_page)) {
> + printk(KERN_ERR "dbg_dump_sg: kmap failed\n");
pr_err is preferred.
> + return;
> + }
> +
> + buf = it_page + it->offset;
> + len = min(tlen, it->length);
> + print_hex_dump(level, prefix_str, prefix_type, rowsize,
> + groupsize, buf, len, ascii);
> + tlen -= len;
> +
> + kunmap_atomic(it_page);
> + }
> +}
> +#endif
I see that the function currently shows up only in places where DEBUG
define is already used.
However, it would be nice to have a no-op implementation in case DEBUG
is not set, to avoid ifdeffery as much as possible.
> +
> static struct list_head alg_list;
>
> struct caam_alg_entry {
> @@ -1982,9 +2018,9 @@ static void ablkcipher_encrypt_done(struct device *jrdev, u32 *desc, u32 err,
> print_hex_dump(KERN_ERR, "dstiv @"__stringify(__LINE__)": ",
> DUMP_PREFIX_ADDRESS, 16, 4, req->info,
> edesc->src_nents > 1 ? 100 : ivsize, 1);
> - print_hex_dump(KERN_ERR, "dst @"__stringify(__LINE__)": ",
> - DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
> - edesc->dst_nents > 1 ? 100 : req->nbytes, 1);
> + dbg_dump_sg(KERN_ERR, "dst @"__stringify(__LINE__)": ",
> + DUMP_PREFIX_ADDRESS, 16, 4, req->dst,
> + edesc->dst_nents > 1 ? 100 : req->nbytes, 1, true);
> #endif
>
> ablkcipher_unmap(jrdev, edesc, req);
> @@ -2014,9 +2050,9 @@ static void ablkcipher_decrypt_done(struct device *jrdev, u32 *desc, u32 err,
> print_hex_dump(KERN_ERR, "dstiv @"__stringify(__LINE__)": ",
> DUMP_PREFIX_ADDRESS, 16, 4, req->info,
> ivsize, 1);
> - print_hex_dump(KERN_ERR, "dst @"__stringify(__LINE__)": ",
> - DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
> - edesc->dst_nents > 1 ? 100 : req->nbytes, 1);
> + dbg_dump_sg(KERN_ERR, "dst @"__stringify(__LINE__)": ",
> + DUMP_PREFIX_ADDRESS, 16, 4, req->dst,
> + edesc->dst_nents > 1 ? 100 : req->nbytes, 1, true);
> #endif
>
> ablkcipher_unmap(jrdev, edesc, req);
> @@ -2171,12 +2207,15 @@ static void init_ablkcipher_job(u32 *sh_desc, dma_addr_t ptr,
> int len, sec4_sg_index = 0;
>
> #ifdef DEBUG
> + bool may_sleep = ((req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
> + CRYPTO_TFM_REQ_MAY_SLEEP)) != 0);
> print_hex_dump(KERN_ERR, "presciv@"__stringify(__LINE__)": ",
> DUMP_PREFIX_ADDRESS, 16, 4, req->info,
> ivsize, 1);
> - print_hex_dump(KERN_ERR, "src @"__stringify(__LINE__)": ",
> - DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
> - edesc->src_nents ? 100 : req->nbytes, 1);
> + printk(KERN_ERR "asked=%d, nbytes%d\n", (int)edesc->src_nents ? 100 : req->nbytes, req->nbytes);
> + dbg_dump_sg(KERN_ERR, "src @"__stringify(__LINE__)": ",
> + DUMP_PREFIX_ADDRESS, 16, 4, req->src,
> + edesc->src_nents ? 100 : req->nbytes, 1, may_sleep);
> #endif
>
> len = desc_len(sh_desc);
> @@ -2228,12 +2267,14 @@ static void init_ablkcipher_giv_job(u32 *sh_desc, dma_addr_t ptr,
> int len, sec4_sg_index = 0;
>
> #ifdef DEBUG
> + bool may_sleep = ((req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
> + CRYPTO_TFM_REQ_MAY_SLEEP)) != 0);
> print_hex_dump(KERN_ERR, "presciv@" __stringify(__LINE__) ": ",
> DUMP_PREFIX_ADDRESS, 16, 4, req->info,
> ivsize, 1);
> - print_hex_dump(KERN_ERR, "src @" __stringify(__LINE__) ": ",
> - DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
> - edesc->src_nents ? 100 : req->nbytes, 1);
> + dbg_dump_sg(KERN_ERR, "src @" __stringify(__LINE__) ": ",
> + DUMP_PREFIX_ADDRESS, 16, 4, req->src,
> + edesc->src_nents ? 100 : req->nbytes, 1, may_sleep);
> #endif
>
> len = desc_len(sh_desc);
> @@ -2503,18 +2544,20 @@ static int aead_decrypt(struct aead_request *req)
> u32 *desc;
> int ret = 0;
>
> +#ifdef DEBUG
> + bool may_sleep = ((req->base.flags & (CRYPTO_TFM_REQ_MAY_BACKLOG |
> + CRYPTO_TFM_REQ_MAY_SLEEP)) != 0);
> + dbg_dump_sg(KERN_ERR, "dec src@"__stringify(__LINE__)": ",
> + DUMP_PREFIX_ADDRESS, 16, 4, req->src,
> + req->assoclen + req->cryptlen, 1, may_sleep);
> +#endif
> +
The logic / flow shouldn't change (in this patch at least), leave the
print in the same place.
> /* allocate extended descriptor */
> edesc = aead_edesc_alloc(req, AUTHENC_DESC_JOB_IO_LEN,
> &all_contig, false);
> if (IS_ERR(edesc))
> return PTR_ERR(edesc);
>
> -#ifdef DEBUG
> - print_hex_dump(KERN_ERR, "dec src@"__stringify(__LINE__)": ",
> - DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->src),
> - req->assoclen + req->cryptlen, 1);
> -#endif
> -
> /* Create and submit job descriptor*/
> init_authenc_job(req, edesc, all_contig, false);
> #ifdef DEBUG
>
Catalin Vasile <[email protected]> wrote:
> Ensure scatterlists have a virtual memory mapping before dumping.
>
> Signed-off-by: Catalin Vasile <[email protected]>
Patch applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Horia Geanta Neag <[email protected]> wrote:
>
>> +
>> +static void dbg_dump_sg(const char *level, const char *prefix_str,
>> + int prefix_type, int rowsize, int groupsize,
>> + struct scatterlist *sg, size_t tlen, bool ascii,
>> + bool may_sleep)
> may_sleep is no longer needed.
As I've already applied this patch, please send any improvements
as incremental patches.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 9/22/2016 1:55 PM, Herbert Xu wrote:
> Horia Geanta Neag <[email protected]> wrote:
>>
>>> +
>>> +static void dbg_dump_sg(const char *level, const char *prefix_str,
>>> + int prefix_type, int rowsize, int groupsize,
>>> + struct scatterlist *sg, size_t tlen, bool ascii,
>>> + bool may_sleep)
>> may_sleep is no longer needed.
>
> As I've already applied this patch, please send any improvements
> as incremental patches.
I am trying to understand the process here...
Would adding an official maintainer for the driver avoid applying
patches without an ack?
(This is not the first time changes are applied without getting a chance
to comment. OTOH I admit there were also cases when there was absolute
silence.)
Thanks,
Horia
On Thu, Sep 22, 2016 at 11:47:49AM +0000, Horia Geanta Neag wrote:
>
> I am trying to understand the process here...
> Would adding an official maintainer for the driver avoid applying
> patches without an ack?
> (This is not the first time changes are applied without getting a chance
> to comment. OTOH I admit there were also cases when there was absolute
> silence.)
Sure, it wouldn't hurt.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt