FAILED: patch "[PATCH] dm clone: Fix handling of partial region discards" failed to apply to 5.4-stable tree

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 4b5142905d4ff58a4b93f7c8eaa7ba829c0a53c9 Mon Sep 17 00:00:00 2001
From: Nikos Tsironis <[email protected]>
Date: Fri, 27 Mar 2020 16:01:08 +0200
Subject: [PATCH] dm clone: Fix handling of partial region discards

There is a bug in the way dm-clone handles discards, which can lead to discarding the wrong blocks or trying to discard blocks beyond the end of the device.

This could lead to data corruption, if the destination device indeed discards the underlying blocks, i.e., if the discard operation results in the original contents of a block to be lost.

The root of the problem is the code that calculates the range of regions covered by a discard request and decides which regions to discard.

Since dm-clone handles the device in units of regions, we don't discard parts of a region, only whole regions.

The range is calculated as:

   rs = dm_sector_div_up(bio->bi_iter.bi_sector, clone->region_size);
   re = bio_end_sector(bio) >> clone->region_shift;

, where 'rs' is the first region to discard and (re - rs) is the number of regions to discard.

The bug manifests when we try to discard part of a single region, i.e., when we try to discard a block with size < region_size, and the discard request both starts at an offset with respect to the beginning of that region and ends before the end of the region.

The root cause is the following comparison:

if (rs == re)
// skip discard and complete original bio immediately

, which doesn't take into account that 'rs' might be greater than 're'.

Thus, we then issue a discard request for the wrong blocks, instead of skipping the discard all together.

Fix the check to also take into account the above case, so we don't end up discarding the wrong blocks.

Also, add some range checks to dm_clone_set_region_hydrated() and dm_clone_cond_set_range(), which update dm-clone's region bitmap.

Note that the aforementioned bug doesn't cause invalid memory accesses, because dm_clone_is_range_hydrated() returns True for this case, so the checks are just precautionary.

Fixes 7431b7835f55 ("dm: add clone target")

Cc [email protected] # v5.4+

Signed-off-by Nikos Tsironis <[email protected]>

Signed-off-by Mike Snitzer <[email protected]>

Quoted 64 lines · On Wed, Apr 15, 2020 at 01:03:09PM +0200, [email protected] wrote:

The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 4b5142905d4ff58a4b93f7c8eaa7ba829c0a53c9 Mon Sep 17 00:00:00 2001
From: Nikos Tsironis <[email protected]>
Date: Fri, 27 Mar 2020 16:01:08 +0200
Subject: [PATCH] dm clone: Fix handling of partial region discards

There is a bug in the way dm-clone handles discards, which can lead to discarding the wrong blocks or trying to discard blocks beyond the end of the device.

This could lead to data corruption, if the destination device indeed discards the underlying blocks, i.e., if the discard operation results in the original contents of a block to be lost.

The root of the problem is the code that calculates the range of regions covered by a discard request and decides which regions to discard.

Since dm-clone handles the device in units of regions, we don't discard parts of a region, only whole regions.

The range is calculated as:

   rs = dm_sector_div_up(bio->bi_iter.bi_sector, clone->region_size);
   re = bio_end_sector(bio) >> clone->region_shift;

, where 'rs' is the first region to discard and (re - rs) is the number of regions to discard.

The bug manifests when we try to discard part of a single region, i.e., when we try to discard a block with size < region_size, and the discard request both starts at an offset with respect to the beginning of that region and ends before the end of the region.

The root cause is the following comparison:

if (rs == re)
// skip discard and complete original bio immediately

, which doesn't take into account that 'rs' might be greater than 're'.

Thus, we then issue a discard request for the wrong blocks, instead of skipping the discard all together.

Fix the check to also take into account the above case, so we don't end up discarding the wrong blocks.

Also, add some range checks to dm_clone_set_region_hydrated() and dm_clone_cond_set_range(), which update dm-clone's region bitmap.

Note that the aforementioned bug doesn't cause invalid memory accesses, because dm_clone_is_range_hydrated() returns True for this case, so the checks are just precautionary.

Quoted 4 lines

Fixes 7431b7835f55 ("dm: add clone target")

Cc [email protected] # v5.4+

Signed-off-by Nikos Tsironis <[email protected]>

Signed-off-by Mike Snitzer <[email protected]>

I've also grabbed 6ca43ed8376a ("dm clone: replace spin_lock_irqsave with spin_lock_irq") to deal with this conflict.