[dm-devel] [PATCH] dm-crypt: Fix per-bio data alignment

[Mikulas Patocka]

Hi

I would like to see the explanation, why does this patch fix it. i686 
allows unaligned access for most instructions, so I wonder how could 
adding an alignment fix it.

What is the exact cipher mode that crashes it? How can I reproduce it with 
cryptsetup?

Is it possible that something shoots beyond the end of cc->iv_size and the 
alignment just masks this bug?

Mikulas



On Mon, 18 Aug 2014, Milan Broz wrote:

> The commit
>   298a9fa08a1577211d42a75e8fc073baef61e0d9
>   dm crypt: use per-bio data
> causes OOPS on 32bit i686 architecture
> 
>   BUG: unable to handle kernel paging request at 20000000
>   IP: [<e0fe2433>] clone_endio+0x13/0xe0 [dm_mod]
>   ...
> 
>  [<c1257b61>] bio_endio+0x61/0x90
>  [<e142476c>] crypt_dec_pending+0x8c/0xd0 [dm_crypt]
>  [<e142666f>] kcryptd_crypt+0x4bf/0x4f0 [dm_crypt]
> 
> This patch fixes the issue by aligning per-bio alocated structure size.
> 
> Reported-by: Krzysztof Kolasa <kkolasa at winsoft.pl>
> Signed-off-by: Milan Broz <gmazyland at gmail.com>
> ---
>  drivers/md/dm-crypt.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 2785007..33f26a2 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -1735,9 +1735,10 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>  		goto bad;
>  	}
>  
> -	cc->per_bio_data_size = ti->per_bio_data_size =
> -				sizeof(struct dm_crypt_io) + cc->dmreq_start +
> -				sizeof(struct dm_crypt_request) + cc->iv_size;
> +	cc->per_bio_data_size = ALIGN(sizeof(struct dm_crypt_io) + cc->dmreq_start +
> +				      sizeof(struct dm_crypt_request) + cc->iv_size,
> +				      ARCH_KMALLOC_MINALIGN);
> +	ti->per_bio_data_size = cc->per_bio_data_size;
>  
>  	cc->page_pool = mempool_create_page_pool(MIN_POOL_PAGES, 0);
>  	if (!cc->page_pool) {
> -- 
> 2.1.0
>