On Thu, Dec 3, 2020 at 8:18 AM Ravi Kumar Siddojigari
<rsiddoji@xxxxxxxxxxxxxx> wrote:
>
> Sorry, Resending the patch for comments with dm-devel added .
>
> -----Original Message-----
> From: Ravi Kumar Siddojigari <rsiddoji@xxxxxxxxxxxxxx>
> Sent: Friday, November 20, 2020 6:37 PM
> To: 'linux-block@xxxxxxxxxxxxxxx' <linux-block@xxxxxxxxxxxxxxx>
> Cc: 'dm-devel@xxxxxxxxxx' <dm-devel@xxxxxxxxxx>
> Subject: RE: [PATCH] dm verity: correcting logic used with corrupted_errs
> counter
>
> One more question :
> Current code has DM_VERITY_MAX_CORRUPTED_ERRS set to 100 can we
> reduce this ? or is there any data that made us to keep this 100 ?
> Regards,
> Ravi
>
> -----Original Message-----
> From: Ravi Kumar Siddojigari <rsiddoji@xxxxxxxxxxxxxx>
> Sent: Wednesday, November 18, 2020 6:17 PM
> To: 'linux-block@xxxxxxxxxxxxxxx' <linux-block@xxxxxxxxxxxxxxx>
> Subject: [PATCH] dm verity: correcting logic used with corrupted_errs
> counter
>
> In verity_handle_err we see that the "corrupted_errs" is never going to be
> more than one as the code will fall through "out" label and hit
> panic/kernel_restart on the first error which is not as expected..
> Following patch will make sure that corrupted_errs are incremented and only
> panic/kernel_restart once it reached DM_VERITY_MAX_CORRUPTED_ERRS.
>
> Signed-off-by: Ravi Kumar Siddojigari <rsiddoji@xxxxxxxxxxxxxx>
> ---
> drivers/md/dm-verity-target.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index f74982dcbea0..d86900a2a8d7 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -221,8 +221,10 @@ static int verity_handle_err(struct dm_verity *v, enum
> verity_block_type type,
> /* Corruption should be visible in device status in all modes */
> v->hash_failed = 1;
>
> - if (v->corrupted_errs >= DM_VERITY_MAX_CORRUPTED_ERRS)
> + if (v->corrupted_errs >= DM_VERITY_MAX_CORRUPTED_ERRS) {
> + DMERR("%s: reached maximum errors", v->data_dev->name);
> goto out;
> + }
>
> v->corrupted_errs++;
>
> @@ -240,13 +242,13 @@ static int verity_handle_err(struct dm_verity *v, enum
> verity_block_type type,
> DMERR_LIMIT("%s: %s block %llu is corrupted", v->data_dev->name,
> type_str, block);
>
> - if (v->corrupted_errs == DM_VERITY_MAX_CORRUPTED_ERRS)
> - DMERR("%s: reached maximum errors", v->data_dev->name);
>
> snprintf(verity_env, DM_VERITY_ENV_LENGTH, "%s=%d,%llu",
> DM_VERITY_ENV_VAR_NAME, type, block);
>
> kobject_uevent_env(&disk_to_dev(dm_disk(md))->kobj, KOBJ_CHANGE,
> envp);
> + /* DM_VERITY_MAX_CORRUPTED_ERRS limit not reached yet */
> + return 0;
No. This would allow invalid blocks to be returned to userspace when
dm-verity is NOT in logging mode, which is unacceptable.
DM_VERITY_MAX_CORRUPTED_ERRS is only used to limit the number of error
messages printed out, we cannot let the first N corrupt blocks to just
slip through.
Sami
