Re: [BUGFIX] dm-crypt: Fix a bug of async cryption complete function

Commit Message

Milan Broz Feb. 27, 2009, 3:02 p.m. UTC

(resend, forgot cc: dm-devel for properly queue in patchwork)

-------- Original Message --------
Subject: Re: [BUGFIX] dm-crypt: Fix a bug of async cryption complete	function
Date: Fri, 27 Feb 2009 14:51:15 +0100
From: Milan Broz <[e-mail]>
To: Herbert Xu <[e-mail]>,  Alasdair G Kergon <[e-mail]>
CC: Huang Ying <[e-mail]>, [e-mail],  [e-mail]
References: <[e-mail]> <[e-mail]> <[e-mail]> <[e-mail]> <[e-mail]> <[e-mail]>

Herbert Xu wrote:
> On Fri, Feb 27, 2009 at 01:28:46PM +0100, Milan Broz wrote:
>> Like this?
>>
>> struct ablkcipher_request *req = (char *)dmreq - cc->dmreq_start;
>> mempool_free(req, cc->req_pool);
>
> Exactly.  You could also embed the ablkcipher_request at the
> end of dmreq, as in
>
> struct dm_crypt_request {
> 	struct scatterlist sg_in;
> 	struct scatterlist sg_out;
> 	struct ablkcipher_request req;
> };
>
> Then you can use container_of.

Hm, I better keep explicitly this pointer retyping as reminder that
the structures need some revision in future...

Is the attached and reworked patch ok?

Alasdair, please can we queue this for 2.6.29-rc as urgent bugfix?

Milan

Patch

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 35bda49..ebab49f 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;