Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_NEOMUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 60997C282C8 for ; Mon, 28 Jan 2019 19:00:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 23A1020989 for ; Mon, 28 Jan 2019 19:00:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726877AbfA1TA6 (ORCPT ); Mon, 28 Jan 2019 14:00:58 -0500 Received: from metis.ext.pengutronix.de ([85.220.165.71]:33957 "EHLO metis.ext.pengutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726678AbfA1TA6 (ORCPT ); Mon, 28 Jan 2019 14:00:58 -0500 Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.89) (envelope-from ) id 1goC9K-0003Mv-Qn; Mon, 28 Jan 2019 20:00:50 +0100 Received: from rhi by ptx.hi.pengutronix.de with local (Exim 4.89) (envelope-from ) id 1goC9H-00055t-FI; Mon, 28 Jan 2019 20:00:47 +0100 Date: Mon, 28 Jan 2019 20:00:47 +0100 From: Roland Hieber To: Horia =?utf-8?Q?Geant=C4=83?= Cc: Herbert Xu , "David S. Miller" , Aymen Sghaier , linux-crypto@vger.kernel.org, Pengutronix Kernel Team Subject: Re: [PATCH v2 2/2] crypto: caam - fix DMA mapping of stack memory Message-ID: <20190128190047.7btanivlbj5jpy3u@pengutronix.de> References: <20190126180215.9842-1-horia.geanta@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20190126180215.9842-1-horia.geanta@nxp.com> X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-IRC: #ptxdist @freenode X-Accept-Language: de,en X-Accept-Content-Type: text/plain X-Uptime: 19:57:00 up 7 days, 6:10, 49 users, load average: 0.07, 0.08, 0.09 User-Agent: NeoMutt/20170113 (1.7.2) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: rhi@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-crypto@vger.kernel.org Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org Hi Horia, I didn't understand your patch thoroughly yet, but I tested it and it gets rid of my DMA-API warning, so: Tested-by: Roland Hieber Thanks! :) - Roland On Sat, Jan 26, 2019 at 08:02:15PM +0200, Horia Geantă wrote: > 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: > [] (dump_backtrace) from [] (show_stack+0x20/0x24) > [] (show_stack) from [] (dump_stack+0xa0/0xcc) > [] (dump_stack) from [] (__warn+0xf0/0x108) > [] (__warn) from [] (warn_slowpath_fmt+0x58/0x74) > [] (warn_slowpath_fmt) from [] (check_for_stack.part.9+0xd0/0x120) > [] (check_for_stack.part.9) from [] (debug_dma_map_page+0x144/0x174) > [] (debug_dma_map_page) from [] (ahash_final_ctx+0x5b4/0xcf0) > [] (ahash_final_ctx) from [] (ahash_final+0x1c/0x20) > [] (ahash_final) from [] (crypto_ahash_op+0x38/0x80) > [] (crypto_ahash_op) from [] (crypto_ahash_final+0x20/0x24) > [] (crypto_ahash_final) from [] (ima_calc_file_hash+0x29c/0xa40) > [] (ima_calc_file_hash) from [] (ima_collect_measurement+0x1dc/0x240) > [] (ima_collect_measurement) from [] (process_measurement+0x4c4/0x6b8) > [] (process_measurement) from [] (ima_file_check+0x88/0xa4) > [] (ima_file_check) from [] (path_openat+0x5d8/0x1364) > [] (path_openat) from [] (do_filp_open+0x84/0xf0) > [] (do_filp_open) from [] (do_open_execat+0x84/0x1b0) > [] (do_open_execat) from [] (__do_execve_file+0x43c/0x890) > [] (__do_execve_file) from [] (sys_execve+0x44/0x4c) > [] (sys_execve) from [] (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: # v4.19+ > Reported-by: Roland Hieber > Signed-off-by: Horia Geantă > --- > drivers/crypto/caam/caamhash.c | 85 +++++++++++------------------------------- > 1 file changed, 21 insertions(+), 64 deletions(-) > > diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c > index b65e2e54c562..89ecda28f87b 100644 > --- a/drivers/crypto/caam/caamhash.c > +++ b/drivers/crypto/caam/caamhash.c > @@ -178,18 +178,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, > @@ -426,7 +414,6 @@ static int ahash_setkey(struct crypto_ahash *ahash, > > /* > * 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 > @@ -434,7 +421,6 @@ static int ahash_setkey(struct crypto_ahash *ahash, > * @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; > @@ -450,8 +436,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, > @@ -486,9 +470,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 > @@ -497,17 +481,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); > @@ -555,9 +536,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 > @@ -566,17 +547,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); > @@ -837,7 +815,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; > > @@ -857,14 +835,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__)": ", > @@ -877,7 +848,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; > } > @@ -931,7 +902,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; > > @@ -945,13 +916,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__)": ", > @@ -964,7 +929,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; > } > @@ -1023,10 +988,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; > @@ -1041,7 +1004,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); > } > > @@ -1083,12 +1046,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__)": ", > @@ -1099,7 +1059,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); > } > > @@ -1298,12 +1258,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__)": ", > @@ -1314,7 +1271,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); > } > > -- > 2.16.2 > > -- Roland Hieber | r.hieber@pengutronix.de | Pengutronix e.K. | https://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim | Phone: +49-5121-206917-5086 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |