From b2174eebd1fadb76454dad09a1dacbc17081e6b0 Mon Sep 17 00:00:00 2001 From: Huang Ying Date: Mon, 16 Mar 2009 17:44:33 +0000 Subject: dm crypt: fix kcryptd_async_done parameter In the async encryption-complete function (kcryptd_async_done), the crypto_async_request passed in may be different from the one passed to crypto_ablkcipher_encrypt/decrypt. Only crypto_async_request->data is guaranteed to be same as the one passed in. The current kcryptd_async_done uses the passed-in crypto_async_request directly which may cause the AES-NI-based AES algorithm implementation to panic. This patch fixes this bug by only using crypto_async_request->data, which points to dm_crypt_request, the crypto_async_request passed in. The original data (convert_context) is gotten from dm_crypt_request. [mbroz@redhat.com: reworked] Cc: stable@kernel.org Signed-off-by: Huang Ying Cc: Herbert Xu Signed-off-by: Milan Broz Signed-off-by: Andrew Morton Signed-off-by: Alasdair G Kergon --- drivers/md/dm-crypt.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) (limited to 'drivers/md/dm-crypt.c') diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 35bda49796f..ebab49f8cc1 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -60,6 +60,7 @@ struct dm_crypt_io { }; struct dm_crypt_request { + struct convert_context *ctx; struct scatterlist sg_in; struct scatterlist sg_out; }; @@ -335,6 +336,18 @@ static void crypt_convert_init(struct crypt_config *cc, init_completion(&ctx->restart); } +static struct dm_crypt_request *dmreq_of_req(struct crypt_config *cc, + struct ablkcipher_request *req) +{ + return (struct dm_crypt_request *)((char *)req + cc->dmreq_start); +} + +static struct ablkcipher_request *req_of_dmreq(struct crypt_config *cc, + struct dm_crypt_request *dmreq) +{ + return (struct ablkcipher_request *)((char *)dmreq - cc->dmreq_start); +} + static int crypt_convert_block(struct crypt_config *cc, struct convert_context *ctx, struct ablkcipher_request *req) @@ -345,10 +358,11 @@ static int crypt_convert_block(struct crypt_config *cc, u8 *iv; int r = 0; - dmreq = (struct dm_crypt_request *)((char *)req + cc->dmreq_start); + dmreq = dmreq_of_req(cc, req); iv = (u8 *)ALIGN((unsigned long)(dmreq + 1), crypto_ablkcipher_alignmask(cc->tfm) + 1); + 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 + ctx->offset_in); @@ -395,8 +409,9 @@ static void crypt_alloc_req(struct crypt_config *cc, cc->req = mempool_alloc(cc->req_pool, GFP_NOIO); ablkcipher_request_set_tfm(cc->req, cc->tfm); ablkcipher_request_set_callback(cc->req, CRYPTO_TFM_REQ_MAY_BACKLOG | - CRYPTO_TFM_REQ_MAY_SLEEP, - kcryptd_async_done, ctx); + CRYPTO_TFM_REQ_MAY_SLEEP, + kcryptd_async_done, + dmreq_of_req(cc, cc->req)); } /* @@ -821,7 +836,8 @@ static void kcryptd_crypt_read_convert(struct dm_crypt_io *io) static void kcryptd_async_done(struct crypto_async_request *async_req, int error) { - struct convert_context *ctx = async_req->data; + struct dm_crypt_request *dmreq = async_req->data; + struct convert_context *ctx = dmreq->ctx; struct dm_crypt_io *io = container_of(ctx, struct dm_crypt_io, ctx); struct crypt_config *cc = io->target->private; @@ -830,7 +846,7 @@ static void kcryptd_async_done(struct crypto_async_request *async_req, return; } - mempool_free(ablkcipher_request_cast(async_req), cc->req_pool); + mempool_free(req_of_dmreq(cc, dmreq), cc->req_pool); if (!atomic_dec_and_test(&ctx->pending)) return; -- cgit v1.2.3 From b35f8caa0890169000fec22902290d9a15274cbd Mon Sep 17 00:00:00 2001 From: Milan Broz Date: Mon, 16 Mar 2009 17:44:36 +0000 Subject: dm crypt: wait for endio to complete before destruction The following oops has been reported when dm-crypt runs over a loop device. ... [ 70.381058] Process loop0 (pid: 4268, ti=cf3b2000 task=cf1cc1f0 task.ti=cf3b2000) ... [ 70.381058] Call Trace: [ 70.381058] [] ? crypt_dec_pending+0x5e/0x62 [dm_crypt] [ 70.381058] [] ? crypt_endio+0xa2/0xaa [dm_crypt] [ 70.381058] [] ? crypt_endio+0x0/0xaa [dm_crypt] [ 70.381058] [] ? bio_endio+0x2b/0x2e [ 70.381058] [] ? dec_pending+0x224/0x23b [dm_mod] [ 70.381058] [] ? clone_endio+0x79/0xa4 [dm_mod] [ 70.381058] [] ? clone_endio+0x0/0xa4 [dm_mod] [ 70.381058] [] ? bio_endio+0x2b/0x2e [ 70.381058] [] ? loop_thread+0x380/0x3b7 [ 70.381058] [] ? do_lo_send_aops+0x0/0x165 [ 70.381058] [] ? autoremove_wake_function+0x0/0x33 [ 70.381058] [] ? loop_thread+0x0/0x3b7 When a table is being replaced, it waits for I/O to complete before destroying the mempool, but the endio function doesn't call mempool_free() until after completing the bio. Fix it by swapping the order of those two operations. The same problem occurs in dm.c with md referenced after dec_pending. Again, we swap the order. Cc: stable@kernel.org Signed-off-by: Milan Broz Signed-off-by: Alasdair G Kergon --- drivers/md/dm-crypt.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) (limited to 'drivers/md/dm-crypt.c') diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index ebab49f8cc1..bfefd079a95 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -568,19 +568,22 @@ static void crypt_inc_pending(struct dm_crypt_io *io) static void crypt_dec_pending(struct dm_crypt_io *io) { struct crypt_config *cc = io->target->private; + struct bio *base_bio = io->base_bio; + struct dm_crypt_io *base_io = io->base_io; + int error = io->error; if (!atomic_dec_and_test(&io->pending)) return; - if (likely(!io->base_io)) - bio_endio(io->base_bio, io->error); + mempool_free(io, cc->io_pool); + + if (likely(!base_io)) + bio_endio(base_bio, error); else { - if (io->error && !io->base_io->error) - io->base_io->error = io->error; - crypt_dec_pending(io->base_io); + if (error && !base_io->error) + base_io->error = error; + crypt_dec_pending(base_io); } - - mempool_free(io, cc->io_pool); } /* -- cgit v1.2.3