On Mon, Mar 13 2017 at 10:19am -0400,
Dan Carpenter <[email protected]> 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.

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to