From a5d60783df61fbb67b7596b8a0f6b4b2e05251d5 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Tue, 30 Aug 2016 16:11:53 -0400 Subject: [PATCH 1/6] dm log writes: move IO accounting earlier to fix error path Move log_one_block()'s atomic_inc(&lc->io_blocks) before bio_alloc() to fix a bug that the target hangs if bio_alloc() fails. The error path does put_io_block(lc), so atomic_inc(&lc->io_blocks) must occur before invoking the error path to avoid underflow of lc->io_blocks. Signed-off-by: Mikulas Patocka Reviewed-by: Josef Bacik Signed-off-by: Mike Snitzer Cc: stable@vger.kernel.org --- drivers/md/dm-log-writes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c index 4ab68033f9d1..4cc78aef9007 100644 --- a/drivers/md/dm-log-writes.c +++ b/drivers/md/dm-log-writes.c @@ -259,12 +259,12 @@ static int log_one_block(struct log_writes_c *lc, goto out; sector++; + atomic_inc(&lc->io_blocks); bio = bio_alloc(GFP_KERNEL, block->vec_cnt); if (!bio) { DMERR("Couldn't alloc log bio"); goto error; } - atomic_inc(&lc->io_blocks); bio->bi_iter.bi_size = 0; bio->bi_iter.bi_sector = sector; bio->bi_bdev = lc->logdev->bdev; From 7efb367320f56fc4d549875b6f3a6940018ef2e5 Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Tue, 30 Aug 2016 16:20:55 -0400 Subject: [PATCH 2/6] dm log writes: fix bug with too large bios bio_alloc() can allocate a bio with at most BIO_MAX_PAGES (256) vector entries. However, the incoming bio may have more vector entries if it was allocated by other means. For example, bcache submits bios with more than BIO_MAX_PAGES entries. This results in bio_alloc() failure. To avoid the failure, change the code so that it allocates bio with at most BIO_MAX_PAGES entries. If the incoming bio has more entries, bio_add_page() will fail and a new bio will be allocated - the code that handles bio_add_page() failure already exists in the dm-log-writes target. Signed-off-by: Mikulas Patocka Reviewed-by: Josef Bacik Signed-off-by: Mike Snitzer Cc: stable@vger.kernel.org # v4.1+ --- drivers/md/dm-log-writes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c index 4cc78aef9007..ba24f4f37efc 100644 --- a/drivers/md/dm-log-writes.c +++ b/drivers/md/dm-log-writes.c @@ -260,7 +260,7 @@ static int log_one_block(struct log_writes_c *lc, sector++; atomic_inc(&lc->io_blocks); - bio = bio_alloc(GFP_KERNEL, block->vec_cnt); + bio = bio_alloc(GFP_KERNEL, min(block->vec_cnt, BIO_MAX_PAGES)); if (!bio) { DMERR("Couldn't alloc log bio"); goto error; @@ -282,7 +282,7 @@ static int log_one_block(struct log_writes_c *lc, if (ret != block->vecs[i].bv_len) { atomic_inc(&lc->io_blocks); submit_bio(bio); - bio = bio_alloc(GFP_KERNEL, block->vec_cnt - i); + bio = bio_alloc(GFP_KERNEL, min(block->vec_cnt - i, BIO_MAX_PAGES)); if (!bio) { DMERR("Couldn't alloc log bio"); goto error; From 91e630d9ae6de6f740ef7c8176736eb55366833e Mon Sep 17 00:00:00 2001 From: Vladimir Zapolskiy Date: Thu, 10 Mar 2016 01:22:19 +0200 Subject: [PATCH 3/6] dm log writes: fix check of kthread_run() return value The kthread_run() function returns either a valid task_struct or ERR_PTR() value, check for NULL is invalid. This change fixes potential for oops, e.g. in OOM situation. Signed-off-by: Vladimir Zapolskiy Signed-off-by: Mike Snitzer Cc: stable@vger.kernel.org --- drivers/md/dm-log-writes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/md/dm-log-writes.c b/drivers/md/dm-log-writes.c index ba24f4f37efc..49e4d8d4558f 100644 --- a/drivers/md/dm-log-writes.c +++ b/drivers/md/dm-log-writes.c @@ -459,9 +459,9 @@ static int log_writes_ctr(struct dm_target *ti, unsigned int argc, char **argv) goto bad; } - ret = -EINVAL; lc->log_kthread = kthread_run(log_writes_kthread, lc, "log-write"); - if (!lc->log_kthread) { + if (IS_ERR(lc->log_kthread)) { + ret = PTR_ERR(lc->log_kthread); ti->error = "Couldn't alloc kthread"; dm_put_device(ti, lc->dev); dm_put_device(ti, lc->logdev); From 4e870e948fbabf62b78e8410f04c67703e7c816b Mon Sep 17 00:00:00 2001 From: Mikulas Patocka Date: Tue, 30 Aug 2016 16:38:42 -0400 Subject: [PATCH 4/6] dm crypt: fix error with too large bios When dm-crypt processes writes, it allocates a new bio in crypt_alloc_buffer(). The bio is allocated from a bio set and it can have at most BIO_MAX_PAGES vector entries, however the incoming bio can be larger (e.g. if it was allocated by bcache). If the incoming bio is larger, bio_alloc_bioset() fails and an error is returned. To avoid the error, we test for a too large bio in the function crypt_map() and use dm_accept_partial_bio() to split the bio. dm_accept_partial_bio() trims the current bio to the desired size and asks DM core to send another bio with the rest of the data. Signed-off-by: Mikulas Patocka Signed-off-by: Mike Snitzer Cc: stable@vger.kernel.org # v3.16+ --- drivers/md/dm-crypt.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index eedba67b0e3e..d609566c19b0 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1924,6 +1924,13 @@ static int crypt_map(struct dm_target *ti, struct bio *bio) return DM_MAPIO_REMAPPED; } + /* + * Check if bio is too large, split as needed. + */ + if (unlikely(bio->bi_iter.bi_size > (BIO_MAX_PAGES << PAGE_SHIFT)) && + bio_data_dir(bio) == WRITE) + dm_accept_partial_bio(bio, ((BIO_MAX_PAGES << PAGE_SHIFT) >> SECTOR_SHIFT)); + io = dm_per_bio_data(bio, cc->per_bio_data_size); crypt_io_init(io, cc, bio, dm_target_offset(ti, bio->bi_iter.bi_sector)); io->ctx.req = (struct skcipher_request *)(io + 1); From 5d0be84ec0cacfc7a6d6ea548afdd07d481324cd Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 30 Aug 2016 09:51:44 -0700 Subject: [PATCH 5/6] dm crypt: fix free of bad values after tfm allocation failure If crypt_alloc_tfms() had to allocate multiple tfms and it failed before the last allocation, then it would call crypt_free_tfms() and could free pointers from uninitialized memory -- due to the crypt_free_tfms() check for non-zero cc->tfms[i]. Fix by allocating zeroed memory. Signed-off-by: Eric Biggers Signed-off-by: Mike Snitzer Cc: stable@vger.kernel.org --- drivers/md/dm-crypt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index d609566c19b0..874295757caa 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1453,7 +1453,7 @@ static int crypt_alloc_tfms(struct crypt_config *cc, char *ciphermode) unsigned i; int err; - cc->tfms = kmalloc(cc->tfms_count * sizeof(struct crypto_skcipher *), + cc->tfms = kzalloc(cc->tfms_count * sizeof(struct crypto_skcipher *), GFP_KERNEL); if (!cc->tfms) return -ENOMEM; From edd1ea2a8a2549e4fe58e817d539445729491ecf Mon Sep 17 00:00:00 2001 From: Bhaktipriya Shridhar Date: Tue, 30 Aug 2016 22:19:11 +0530 Subject: [PATCH 6/6] dm bufio: remove use of deprecated create_singlethread_workqueue() The workqueue "dm_bufio_wq" queues a single work item &dm_bufio_work so it doesn't require execution ordering. Hence, alloc_workqueue() has been used to replace the deprecated create_singlethread_workqueue(). The WQ_MEM_RECLAIM flag has been set since DM requires forward progress under memory pressure. Since there are fixed number of work items, explicit concurrency limit is unnecessary here. Signed-off-by: Bhaktipriya Shridhar Signed-off-by: Mike Snitzer --- drivers/md/dm-bufio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/dm-bufio.c b/drivers/md/dm-bufio.c index 6571c81465e1..8625040bae92 100644 --- a/drivers/md/dm-bufio.c +++ b/drivers/md/dm-bufio.c @@ -1879,7 +1879,7 @@ static int __init dm_bufio_init(void) __cache_size_refresh(); mutex_unlock(&dm_bufio_clients_lock); - dm_bufio_wq = create_singlethread_workqueue("dm_bufio_cache"); + dm_bufio_wq = alloc_workqueue("dm_bufio_cache", WQ_MEM_RECLAIM, 0); if (!dm_bufio_wq) return -ENOMEM;