linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: gregkh@linuxfoundation.org
To: linux-kernel@vger.kernel.org
Cc: "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, "Milan Broz" <gmazyland@gmail.com>,
	"Jérôme Carretero" <cJ-ko@zougloub.eu>,
	"Sami Tolvanen" <samitolvanen@google.com>,
	"Mike Snitzer" <snitzer@redhat.com>
Subject: [PATCH 5.11 24/44] dm verity: fix FEC for RS roots unaligned to block size
Date: Mon,  8 Mar 2021 13:35:02 +0100	[thread overview]
Message-ID: <20210308122719.767429188@linuxfoundation.org> (raw)
In-Reply-To: <20210308122718.586629218@linuxfoundation.org>

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

From: Milan Broz <gmazyland@gmail.com>

commit df7b59ba9245c4a3115ebaa905e3e5719a3810da upstream.

Optional Forward Error Correction (FEC) code in dm-verity uses
Reed-Solomon code and should support roots from 2 to 24.

The error correction parity bytes (of roots lengths per RS block) are
stored on a separate device in sequence without any padding.

Currently, to access FEC device, the dm-verity-fec code uses dm-bufio
client with block size set to verity data block (usually 4096 or 512
bytes).

Because this block size is not divisible by some (most!) of the roots
supported lengths, data repair cannot work for partially stored parity
bytes.

This fix changes FEC device dm-bufio block size to "roots << SECTOR_SHIFT"
where we can be sure that the full parity data is always available.
(There cannot be partial FEC blocks because parity must cover whole
sectors.)

Because the optional FEC starting offset could be unaligned to this
new block size, we have to use dm_bufio_set_sector_offset() to
configure it.

The problem is easily reproduced using veritysetup, e.g. for roots=13:

  # create verity device with RS FEC
  dd if=/dev/urandom of=data.img bs=4096 count=8 status=none
  veritysetup format data.img hash.img --fec-device=fec.img --fec-roots=13 | awk '/^Root hash/{ print $3 }' >roothash

  # create an erasure that should be always repairable with this roots setting
  dd if=/dev/zero of=data.img conv=notrunc bs=1 count=8 seek=4088 status=none

  # try to read it through dm-verity
  veritysetup open data.img test hash.img --fec-device=fec.img --fec-roots=13 $(cat roothash)
  dd if=/dev/mapper/test of=/dev/null bs=4096 status=noxfer
  # wait for possible recursive recovery in kernel
  udevadm settle
  veritysetup close test

With this fix, errors are properly repaired.
  device-mapper: verity-fec: 7:1: FEC 0: corrected 8 errors
  ...

Without it, FEC code usually ends on unrecoverable failure in RS decoder:
  device-mapper: verity-fec: 7:1: FEC 0: failed to correct: -74
  ...

This problem is present in all kernels since the FEC code's
introduction (kernel 4.5).

It is thought that this problem is not visible in Android ecosystem
because it always uses a default RS roots=2.

Depends-on: a14e5ec66a7a ("dm bufio: subtract the number of initial sectors in dm_bufio_get_device_size")
Signed-off-by: Milan Broz <gmazyland@gmail.com>
Tested-by: Jérôme Carretero <cJ-ko@zougloub.eu>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Cc: stable@vger.kernel.org # 4.5+
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/md/dm-verity-fec.c |   23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

--- a/drivers/md/dm-verity-fec.c
+++ b/drivers/md/dm-verity-fec.c
@@ -61,19 +61,18 @@ static int fec_decode_rs8(struct dm_veri
 static u8 *fec_read_parity(struct dm_verity *v, u64 rsb, int index,
 			   unsigned *offset, struct dm_buffer **buf)
 {
-	u64 position, block;
+	u64 position, block, rem;
 	u8 *res;
 
 	position = (index + rsb) * v->fec->roots;
-	block = position >> v->data_dev_block_bits;
-	*offset = (unsigned)(position - (block << v->data_dev_block_bits));
+	block = div64_u64_rem(position, v->fec->roots << SECTOR_SHIFT, &rem);
+	*offset = (unsigned)rem;
 
-	res = dm_bufio_read(v->fec->bufio, v->fec->start + block, buf);
+	res = dm_bufio_read(v->fec->bufio, block, buf);
 	if (IS_ERR(res)) {
 		DMERR("%s: FEC %llu: parity read failed (block %llu): %ld",
 		      v->data_dev->name, (unsigned long long)rsb,
-		      (unsigned long long)(v->fec->start + block),
-		      PTR_ERR(res));
+		      (unsigned long long)block, PTR_ERR(res));
 		*buf = NULL;
 	}
 
@@ -155,7 +154,7 @@ static int fec_decode_bufs(struct dm_ver
 
 		/* read the next block when we run out of parity bytes */
 		offset += v->fec->roots;
-		if (offset >= 1 << v->data_dev_block_bits) {
+		if (offset >= v->fec->roots << SECTOR_SHIFT) {
 			dm_bufio_release(buf);
 
 			par = fec_read_parity(v, rsb, block_offset, &offset, &buf);
@@ -674,7 +673,7 @@ int verity_fec_ctr(struct dm_verity *v)
 {
 	struct dm_verity_fec *f = v->fec;
 	struct dm_target *ti = v->ti;
-	u64 hash_blocks;
+	u64 hash_blocks, fec_blocks;
 	int ret;
 
 	if (!verity_fec_is_enabled(v)) {
@@ -744,15 +743,17 @@ int verity_fec_ctr(struct dm_verity *v)
 	}
 
 	f->bufio = dm_bufio_client_create(f->dev->bdev,
-					  1 << v->data_dev_block_bits,
+					  f->roots << SECTOR_SHIFT,
 					  1, 0, NULL, NULL);
 	if (IS_ERR(f->bufio)) {
 		ti->error = "Cannot initialize FEC bufio client";
 		return PTR_ERR(f->bufio);
 	}
 
-	if (dm_bufio_get_device_size(f->bufio) <
-	    ((f->start + f->rounds * f->roots) >> v->data_dev_block_bits)) {
+	dm_bufio_set_sector_offset(f->bufio, f->start << (v->data_dev_block_bits - SECTOR_SHIFT));
+
+	fec_blocks = div64_u64(f->rounds * f->roots, v->fec->roots << SECTOR_SHIFT);
+	if (dm_bufio_get_device_size(f->bufio) < fec_blocks) {
 		ti->error = "FEC device is too small";
 		return -E2BIG;
 	}



  parent reply	other threads:[~2021-03-08 12:37 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-08 12:34 [PATCH 5.11 00/44] 5.11.5-rc1 review gregkh
2021-03-08 12:34 ` [PATCH 5.11 01/44] ALSA: hda/realtek: Enable headset mic of Acer SWIFT with ALC256 gregkh
2021-03-08 12:34 ` [PATCH 5.11 02/44] ALSA: usb-audio: use Corsair Virtuoso mapping for Corsair Virtuoso SE gregkh
2021-03-08 12:34 ` [PATCH 5.11 03/44] ALSA: usb-audio: Dont abort even if the clock rate differs gregkh
2021-03-08 12:34 ` [PATCH 5.11 04/44] ALSA: usb-audio: Drop bogus dB range in too low level gregkh
2021-03-08 12:34 ` [PATCH 5.11 05/44] ALSA: usb-audio: Allow modifying parameters with succeeding hw_params calls gregkh
2021-03-08 12:34 ` [PATCH 5.11 06/44] tpm, tpm_tis: Decorate tpm_tis_gen_interrupt() with request_locality() gregkh
2021-03-08 12:34 ` [PATCH 5.11 07/44] tpm, tpm_tis: Decorate tpm_get_timeouts() " gregkh
2021-03-08 12:34 ` [PATCH 5.11 08/44] btrfs: avoid double put of block group when emptying cluster gregkh
2021-03-08 12:34 ` [PATCH 5.11 09/44] btrfs: fix raid6 qstripe kmap gregkh
2021-03-08 12:34 ` [PATCH 5.11 10/44] btrfs: fix race between writes to swap files and scrub gregkh
2021-03-08 12:34 ` [PATCH 5.11 11/44] btrfs: fix race between swap file activation and snapshot creation gregkh
2021-03-08 12:34 ` [PATCH 5.11 12/44] btrfs: fix stale data exposure after cloning a hole with NO_HOLES enabled gregkh
2021-03-08 12:34 ` [PATCH 5.11 13/44] btrfs: tree-checker: do not error out if extent ref hash doesnt match gregkh
2021-03-08 12:34 ` [PATCH 5.11 14/44] btrfs: fix race between extent freeing/allocation when using bitmaps gregkh
2021-03-08 12:34 ` [PATCH 5.11 15/44] btrfs: validate qgroup inherit for SNAP_CREATE_V2 ioctl gregkh
2021-03-08 12:34 ` [PATCH 5.11 16/44] btrfs: free correct amount of space in btrfs_delayed_inode_reserve_metadata gregkh
2021-03-08 12:34 ` [PATCH 5.11 17/44] btrfs: fix spurious free_space_tree remount warning gregkh
2021-03-08 12:34 ` [PATCH 5.11 18/44] btrfs: unlock extents in btrfs_zero_range in case of quota reservation errors gregkh
2021-03-08 12:34 ` [PATCH 5.11 19/44] btrfs: fix warning when creating a directory with smack enabled gregkh
2021-03-08 12:34 ` [PATCH 5.11 20/44] PM: runtime: Update device status before letting suppliers suspend gregkh
2021-03-08 12:34 ` [PATCH 5.11 21/44] ring-buffer: Force before_stamp and write_stamp to be different on discard gregkh
2021-03-08 12:35 ` [PATCH 5.11 22/44] io_uring: ignore double poll add on the same waitqueue head gregkh
2021-03-08 12:35 ` [PATCH 5.11 23/44] dm bufio: subtract the number of initial sectors in dm_bufio_get_device_size gregkh
2021-03-08 12:35 ` gregkh [this message]
2021-03-08 12:35 ` [PATCH 5.11 25/44] drm/amd/pm: correct Arcturus mmTHM_BACO_CNTL register address gregkh
2021-03-08 12:35 ` [PATCH 5.11 26/44] drm/amdgpu:disable VCN for Navi12 SKU gregkh
2021-03-08 12:35 ` [PATCH 5.11 27/44] drm/amdgpu: Only check for S0ix if AMD_PMC is configured gregkh
2021-03-08 12:35 ` [PATCH 5.11 28/44] drm/amdgpu: fix parameter error of RREG32_PCIE() in amdgpu_regs_pcie gregkh
2021-03-08 12:35 ` [PATCH 5.11 29/44] crypto - shash: reduce minimum alignment of shash_desc structure gregkh
2021-03-08 12:35 ` [PATCH 5.11 30/44] ALSA: ctxfi: cthw20k2: fix mask on conf to allow 4 bits gregkh
2021-03-08 12:35 ` [PATCH 5.11 31/44] ALSA: usb-audio: Fix Pioneer DJM devices URB_CONTROL request direction to set samplerate gregkh
2021-03-08 12:35 ` [PATCH 5.11 32/44] RDMA/cm: Fix IRQ restore in ib_send_cm_sidr_rep gregkh
2021-03-08 12:35 ` [PATCH 5.11 33/44] RDMA/rxe: Fix missing kconfig dependency on CRYPTO gregkh
2021-03-08 12:35 ` [PATCH 5.11 34/44] IB/mlx5: Add missing error code gregkh
2021-03-08 12:35 ` [PATCH 5.11 35/44] ALSA: hda: intel-nhlt: verify config type gregkh
2021-03-08 12:35 ` [PATCH 5.11 36/44] ftrace: Have recordmcount use w8 to read relp->r_info in arm64_is_fake_mcount gregkh
2021-03-08 12:35 ` [PATCH 5.11 37/44] ia64: dont call handle_signal() unless theres actually a signal queued gregkh
2021-03-08 12:35 ` [PATCH 5.11 38/44] rsxx: Return -EFAULT if copy_to_user() fails gregkh
2021-03-08 12:35 ` [PATCH 5.11 39/44] iommu/tegra-smmu: Fix mc errors on tegra124-nyan gregkh
2021-03-08 12:35 ` [PATCH 5.11 40/44] iommu: Dont use lazy flush for untrusted device gregkh
2021-03-08 12:35 ` [PATCH 5.11 41/44] iommu/vt-d: Fix status code for Allocate/Free PASID command gregkh
2021-03-08 12:35 ` [PATCH 5.11 42/44] btrfs: zoned: use sector_t for zone sectors gregkh
2021-03-08 12:35 ` [PATCH 5.11 43/44] tomoyo: recognize kernel threads correctly gregkh
2021-03-08 12:35 ` [PATCH 5.11 44/44] r8169: fix resuming from suspend on RTL8105e if machine runs on battery gregkh
2021-03-08 22:29 ` [PATCH 5.11 00/44] 5.11.5-rc1 review Guenter Roeck
2021-03-09 10:26   ` Greg KH
2021-03-09  4:22 ` Naresh Kamboju
2021-03-09 10:26   ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210308122719.767429188@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=cJ-ko@zougloub.eu \
    --cc=gmazyland@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=samitolvanen@google.com \
    --cc=snitzer@redhat.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).