[dm-devel] [bug report] dm crypt: add cryptographic data integrity protection (authenticated encryption)
Mike Snitzer
snitzer at redhat.com
Mon Mar 13 16:15:59 UTC 2017
On Mon, Mar 13 2017 at 12:08pm -0400,
Mike Snitzer <snitzer at redhat.com> wrote:
> On Mon, Mar 13 2017 at 10:19am -0400,
> Dan Carpenter <dan.carpenter at 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.
I folded in a fix for this, Milan please review crypt_alloc_buffer()
changes in this revised commit:
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.12&id=8896496efaef45ca1a734782a8d7f9082299fa5d
More information about the dm-devel
mailing list