crypto: caam - fix DMA mapping of stack memory

Roland reports the following issue and provides a root cause analysis:

"On a v4.19 i.MX6 system with IMA and CONFIG_DMA_API_DEBUG enabled, a
warning is generated when accessing files on a filesystem for which IMA
measurement is enabled:

    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 1 at kernel/dma/debug.c:1181 check_for_stack.part.9+0xd0/0x120
    caam_jr 2101000.jr0: DMA-API: device driver maps memory from stack [addr=b668049e]
    Modules linked in:
    CPU: 0 PID: 1 Comm: switch_root Not tainted 4.19.0-20181214-1 #2
    Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
    Backtrace:
    [<c010efb8>] (dump_backtrace) from [<c010f2d0>] (show_stack+0x20/0x24)
    [<c010f2b0>] (show_stack) from [<c08b04f4>] (dump_stack+0xa0/0xcc)
    [<c08b0454>] (dump_stack) from [<c012b610>] (__warn+0xf0/0x108)
    [<c012b520>] (__warn) from [<c012b680>] (warn_slowpath_fmt+0x58/0x74)
    [<c012b62c>] (warn_slowpath_fmt) from [<c0199acc>] (check_for_stack.part.9+0xd0/0x120)
    [<c01999fc>] (check_for_stack.part.9) from [<c019a040>] (debug_dma_map_page+0x144/0x174)
    [<c0199efc>] (debug_dma_map_page) from [<c065f7f4>] (ahash_final_ctx+0x5b4/0xcf0)
    [<c065f240>] (ahash_final_ctx) from [<c065b3c4>] (ahash_final+0x1c/0x20)
    [<c065b3a8>] (ahash_final) from [<c03fe278>] (crypto_ahash_op+0x38/0x80)
    [<c03fe240>] (crypto_ahash_op) from [<c03fe2e0>] (crypto_ahash_final+0x20/0x24)
    [<c03fe2c0>] (crypto_ahash_final) from [<c03f19a8>] (ima_calc_file_hash+0x29c/0xa40)
    [<c03f170c>] (ima_calc_file_hash) from [<c03f2b24>] (ima_collect_measurement+0x1dc/0x240)
    [<c03f2948>] (ima_collect_measurement) from [<c03f0a60>] (process_measurement+0x4c4/0x6b8)
    [<c03f059c>] (process_measurement) from [<c03f0cdc>] (ima_file_check+0x88/0xa4)
    [<c03f0c54>] (ima_file_check) from [<c02d8adc>] (path_openat+0x5d8/0x1364)
    [<c02d8504>] (path_openat) from [<c02dad24>] (do_filp_open+0x84/0xf0)
    [<c02daca0>] (do_filp_open) from [<c02cf50c>] (do_open_execat+0x84/0x1b0)
    [<c02cf488>] (do_open_execat) from [<c02d1058>] (__do_execve_file+0x43c/0x890)
    [<c02d0c1c>] (__do_execve_file) from [<c02d1770>] (sys_execve+0x44/0x4c)
    [<c02d172c>] (sys_execve) from [<c0101000>] (ret_fast_syscall+0x0/0x28)
    ---[ end trace 3455789a10e3aefd ]---

The cause is that the struct ahash_request *req is created as a
stack-local variable up in the stack (presumably somewhere in the IMA
implementation), then passed down into the CAAM driver, which tries to
dma_single_map the req->result (indirectly via map_seq_out_ptr_result)
in order to make that buffer available for the CAAM to store the result
of the following hash operation.

The calling code doesn't know how req will be used by the CAAM driver,
and there could be other such occurrences where stack memory is passed
down to the CAAM driver. Therefore we should rather fix this issue in
the CAAM driver where the requirements are known."

Fix this problem by:
-instructing the crypto engine to write the final hash in state->caam_ctx
-subsequently memcpy-ing the final hash into req->result

Cc: <stable@vger.kernel.org> # v4.19+
Reported-by: Roland Hieber <rhi@pengutronix.de>
Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
Tested-by: Roland Hieber <rhi@pengutronix.de>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 5855d03f52d1943927d10bd470aad906fa75a75f..3bba3cb92f03f3b7ab80311dea17ec974501bc9b 100644 (file)
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -184,18 +184,6 @@ static inline int map_seq_out_ptr_ctx(u32 *desc, struct device *jrdev,
        return 0;
 }
 
-/* Map req->result, and append seq_out_ptr command that points to it */
-static inline dma_addr_t map_seq_out_ptr_result(u32 *desc, struct device *jrdev,
-                                               u8 *result, int digestsize)
-{
-       dma_addr_t dst_dma;
-
-       dst_dma = dma_map_single(jrdev, result, digestsize, DMA_FROM_DEVICE);
-       append_seq_out_ptr(desc, dst_dma, digestsize, 0);
-
-       return dst_dma;
-}
-
 /* Map current buffer in state (if length > 0) and put it in link table */
 static inline int buf_map_to_sec4_sg(struct device *jrdev,
                                     struct sec4_sg_entry *sec4_sg,
@@ -568,7 +556,6 @@ static int acmac_setkey(struct crypto_ahash *ahash, const u8 *key,
 
 /*
  * ahash_edesc - s/w-extended ahash descriptor
- * @dst_dma: physical mapped address of req->result
  * @sec4_sg_dma: physical mapped address of h/w link table
  * @src_nents: number of segments in input scatterlist
  * @sec4_sg_bytes: length of dma mapped sec4_sg space
@@ -576,7 +563,6 @@ static int acmac_setkey(struct crypto_ahash *ahash, const u8 *key,
  * @sec4_sg: h/w link table
  */
 struct ahash_edesc {
-       dma_addr_t dst_dma;
        dma_addr_t sec4_sg_dma;
        int src_nents;
        int sec4_sg_bytes;
@@ -592,8 +578,6 @@ static inline void ahash_unmap(struct device *dev,
 
        if (edesc->src_nents)
                dma_unmap_sg(dev, req->src, edesc->src_nents, DMA_TO_DEVICE);
-       if (edesc->dst_dma)
-               dma_unmap_single(dev, edesc->dst_dma, dst_len, DMA_FROM_DEVICE);
 
        if (edesc->sec4_sg_bytes)
                dma_unmap_single(dev, edesc->sec4_sg_dma,
@@ -628,9 +612,9 @@ static void ahash_done(struct device *jrdev, u32 *desc, u32 err,
        struct ahash_edesc *edesc;
        struct crypto_ahash *ahash = crypto_ahash_reqtfm(req);
        int digestsize = crypto_ahash_digestsize(ahash);
+       struct caam_hash_state *state = ahash_request_ctx(req);
 #ifdef DEBUG
        struct caam_hash_ctx *ctx = crypto_ahash_ctx(ahash);
-       struct caam_hash_state *state = ahash_request_ctx(req);
 
        dev_err(jrdev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
 #endif
@@ -639,17 +623,14 @@ static void ahash_done(struct device *jrdev, u32 *desc, u32 err,
        if (err)
                caam_jr_strstatus(jrdev, err);
 
-       ahash_unmap(jrdev, edesc, req, digestsize);
+       ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
+       memcpy(req->result, state->caam_ctx, digestsize);
        kfree(edesc);
 
 #ifdef DEBUG
        print_hex_dump(KERN_ERR, "ctx@"__stringify(__LINE__)": ",
                       DUMP_PREFIX_ADDRESS, 16, 4, state->caam_ctx,
                       ctx->ctx_len, 1);
-       if (req->result)
-               print_hex_dump(KERN_ERR, "result@"__stringify(__LINE__)": ",
-                              DUMP_PREFIX_ADDRESS, 16, 4, req->result,
-                              digestsize, 1);
 #endif
 
        req->base.complete(&req->base, err);
@@ -697,9 +678,9 @@ static void ahash_done_ctx_src(struct device *jrdev, u32 *desc, u32 err,
        struct ahash_edesc *edesc;
        struct crypto_ahash *ahash = crypto_ahash_reqtfm(req);
        int digestsize = crypto_ahash_digestsize(ahash);
+       struct caam_hash_state *state = ahash_request_ctx(req);
 #ifdef DEBUG
        struct caam_hash_ctx *ctx = crypto_ahash_ctx(ahash);
-       struct caam_hash_state *state = ahash_request_ctx(req);
 
        dev_err(jrdev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
 #endif
@@ -708,17 +689,14 @@ static void ahash_done_ctx_src(struct device *jrdev, u32 *desc, u32 err,
        if (err)
                caam_jr_strstatus(jrdev, err);
 
-       ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_TO_DEVICE);
+       ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_BIDIRECTIONAL);
+       memcpy(req->result, state->caam_ctx, digestsize);
        kfree(edesc);
 
 #ifdef DEBUG
        print_hex_dump(KERN_ERR, "ctx@"__stringify(__LINE__)": ",
                       DUMP_PREFIX_ADDRESS, 16, 4, state->caam_ctx,
                       ctx->ctx_len, 1);
-       if (req->result)
-               print_hex_dump(KERN_ERR, "result@"__stringify(__LINE__)": ",
-                              DUMP_PREFIX_ADDRESS, 16, 4, req->result,
-                              digestsize, 1);
 #endif
 
        req->base.complete(&req->base, err);
@@ -991,7 +969,7 @@ static int ahash_final_ctx(struct ahash_request *req)
        edesc->sec4_sg_bytes = sec4_sg_bytes;
 
        ret = ctx_map_to_sec4_sg(jrdev, state, ctx->ctx_len,
-                                edesc->sec4_sg, DMA_TO_DEVICE);
+                                edesc->sec4_sg, DMA_BIDIRECTIONAL);
        if (ret)
                goto unmap_ctx;
 
@@ -1011,14 +989,7 @@ static int ahash_final_ctx(struct ahash_request *req)
 
        append_seq_in_ptr(desc, edesc->sec4_sg_dma, ctx->ctx_len + buflen,
                          LDST_SGF);
-
-       edesc->dst_dma = map_seq_out_ptr_result(desc, jrdev, req->result,
-                                               digestsize);
-       if (dma_mapping_error(jrdev, edesc->dst_dma)) {
-               dev_err(jrdev, "unable to map dst\n");
-               ret = -ENOMEM;
-               goto unmap_ctx;
-       }
+       append_seq_out_ptr(desc, state->ctx_dma, digestsize, 0);
 
 #ifdef DEBUG
        print_hex_dump(KERN_ERR, "jobdesc@"__stringify(__LINE__)": ",
@@ -1031,7 +1002,7 @@ static int ahash_final_ctx(struct ahash_request *req)
 
        return -EINPROGRESS;
  unmap_ctx:
-       ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
+       ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_BIDIRECTIONAL);
        kfree(edesc);
        return ret;
 }
@@ -1085,7 +1056,7 @@ static int ahash_finup_ctx(struct ahash_request *req)
        edesc->src_nents = src_nents;
 
        ret = ctx_map_to_sec4_sg(jrdev, state, ctx->ctx_len,
-                                edesc->sec4_sg, DMA_TO_DEVICE);
+                                edesc->sec4_sg, DMA_BIDIRECTIONAL);
        if (ret)
                goto unmap_ctx;
 
@@ -1099,13 +1070,7 @@ static int ahash_finup_ctx(struct ahash_request *req)
        if (ret)
                goto unmap_ctx;
 
-       edesc->dst_dma = map_seq_out_ptr_result(desc, jrdev, req->result,
-                                               digestsize);
-       if (dma_mapping_error(jrdev, edesc->dst_dma)) {
-               dev_err(jrdev, "unable to map dst\n");
-               ret = -ENOMEM;
-               goto unmap_ctx;
-       }
+       append_seq_out_ptr(desc, state->ctx_dma, digestsize, 0);
 
 #ifdef DEBUG
        print_hex_dump(KERN_ERR, "jobdesc@"__stringify(__LINE__)": ",
@@ -1118,7 +1083,7 @@ static int ahash_finup_ctx(struct ahash_request *req)
 
        return -EINPROGRESS;
  unmap_ctx:
-       ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
+       ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_BIDIRECTIONAL);
        kfree(edesc);
        return ret;
 }
@@ -1177,10 +1142,8 @@ static int ahash_digest(struct ahash_request *req)
 
        desc = edesc->hw_desc;
 
-       edesc->dst_dma = map_seq_out_ptr_result(desc, jrdev, req->result,
-                                               digestsize);
-       if (dma_mapping_error(jrdev, edesc->dst_dma)) {
-               dev_err(jrdev, "unable to map dst\n");
+       ret = map_seq_out_ptr_ctx(desc, jrdev, state, digestsize);
+       if (ret) {
                ahash_unmap(jrdev, edesc, req, digestsize);
                kfree(edesc);
                return -ENOMEM;
@@ -1195,7 +1158,7 @@ static int ahash_digest(struct ahash_request *req)
        if (!ret) {
                ret = -EINPROGRESS;
        } else {
-               ahash_unmap(jrdev, edesc, req, digestsize);
+               ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
                kfree(edesc);
        }
 
@@ -1237,12 +1200,9 @@ static int ahash_final_no_ctx(struct ahash_request *req)
                append_seq_in_ptr(desc, state->buf_dma, buflen, 0);
        }
 
-       edesc->dst_dma = map_seq_out_ptr_result(desc, jrdev, req->result,
-                                               digestsize);
-       if (dma_mapping_error(jrdev, edesc->dst_dma)) {
-               dev_err(jrdev, "unable to map dst\n");
+       ret = map_seq_out_ptr_ctx(desc, jrdev, state, digestsize);
+       if (ret)
                goto unmap;
-       }
 
 #ifdef DEBUG
        print_hex_dump(KERN_ERR, "jobdesc@"__stringify(__LINE__)": ",
@@ -1253,7 +1213,7 @@ static int ahash_final_no_ctx(struct ahash_request *req)
        if (!ret) {
                ret = -EINPROGRESS;
        } else {
-               ahash_unmap(jrdev, edesc, req, digestsize);
+               ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
                kfree(edesc);
        }
 
@@ -1464,12 +1424,9 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
                goto unmap;
        }
 
-       edesc->dst_dma = map_seq_out_ptr_result(desc, jrdev, req->result,
-                                               digestsize);
-       if (dma_mapping_error(jrdev, edesc->dst_dma)) {
-               dev_err(jrdev, "unable to map dst\n");
+       ret = map_seq_out_ptr_ctx(desc, jrdev, state, digestsize);
+       if (ret)
                goto unmap;
-       }
 
 #ifdef DEBUG
        print_hex_dump(KERN_ERR, "jobdesc@"__stringify(__LINE__)": ",
@@ -1480,7 +1437,7 @@ static int ahash_finup_no_ctx(struct ahash_request *req)
        if (!ret) {
                ret = -EINPROGRESS;
        } else {
-               ahash_unmap(jrdev, edesc, req, digestsize);
+               ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);
                kfree(edesc);
        }