Re: [bug report] dm crypt: add cryptographic data integrity protection (authenticated encryption)

[Mike Snitzer <snitzer@redhat.com>]

On Mon, Mar 13 2017 at 10:19am -0400,
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> Hello Milan Broz,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 858516b7881e: "dm crypt: add cryptographic data integrity
> protection (authenticated encryption)" from Jan 4, 2017, leads to the
> following Smatch complaint:
> 
> drivers/md/dm-crypt.c:1390 crypt_alloc_buffer()
> 	 error: we previously assumed 'clone' could be null (see line 1362)
> 
> drivers/md/dm-crypt.c
>   1361		clone = bio_alloc_bioset(GFP_NOIO, nr_iovecs, cc->bs);
>   1362		if (!clone)
>                     ^^^^^^
> We can be NULL on this path.
> 
>   1363			goto return_clone;
>   1364	
>   1365		clone_init(io, clone);
>   1366	
>   1367		remaining_size = size;
>   1368	
>   1369		for (i = 0; i < nr_iovecs; i++) {
>   1370			page = mempool_alloc(cc->page_pool, gfp_mask);
>   1371			if (!page) {
>   1372				crypt_free_buffer_pages(cc, clone);
>   1373				bio_put(clone);
>   1374				gfp_mask |= __GFP_DIRECT_RECLAIM;
>   1375				goto retry;
>   1376			}
>   1377	
>   1378			len = (remaining_size > PAGE_SIZE) ? PAGE_SIZE : remaining_size;
>   1379	
>   1380			bio_add_page(clone, page, len, 0);
>   1381	
>   1382			remaining_size -= len;
>   1383		}
>   1384	
>   1385	return_clone:
>   1386		if (unlikely(gfp_mask & __GFP_DIRECT_RECLAIM))
>   1387			mutex_unlock(&cc->bio_alloc_lock);
>   1388	
>   1389		/* Allocate space for integrity tags */
>   1390		if (dm_crypt_integrity_io_alloc(io, clone)) {
>                                                     ^^^^^
> Oops inside new function call.
> 
>   1391			crypt_free_buffer_pages(cc, clone);
>   1392			bio_put(clone);

Thanks for the report.  This code makes no sense as is.

Milan, please help me understand why you're allocating memory needed for
integrity tags in this 'return_clone' error path.