All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Yeongjin Gil <youngjin.gil@samsung.com>
Cc: snitzer@kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, dm-devel@redhat.com,
	Sungjong Seo <sj1557.seo@samsung.com>,
	agk@redhat.com
Subject: Re: [dm-devel] [PATCH v2] dm verity: fix error handling for check_at_most_once on FEC
Date: Mon, 20 Mar 2023 14:23:01 -0700	[thread overview]
Message-ID: <20230320212301.GD1434@sol.localdomain> (raw)
In-Reply-To: <20230320065932.28116-1-youngjin.gil@samsung.com>

On Mon, Mar 20, 2023 at 03:59:32PM +0900, Yeongjin Gil wrote:
> In verity_end_io(), if bi_status is not BLK_STS_OK, it can be return
> directly. But if FEC configured, it is desired to correct the data page
> through verity_verify_io. And the return value will be converted to
> blk_status and passed to verity_finish_io().
> 
> BTW, when a bit is set in v->validated_blocks, verity_verify_io() skips
> verification regardless of I/O error for the corresponding bio. In this
> case, the I/O error could not be returned properly, and as a result,
> there is a problem that abnormal data could be read for the
> corresponding block.
> 
> To fix this problem, when an I/O error occurs, do not skip verification
> even if the bit related is set in v->validated_blocks.
> 
> Fixes: 843f38d382b1 ("dm verity: add 'check_at_most_once' option to only validate hashes once")
> Cc: stable@vger.kernel.org
> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
> Signed-off-by: Yeongjin Gil <youngjin.gil@samsung.com>
> ---
> v2:
> -change commit message and tag
> ---
>  drivers/md/dm-verity-target.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index ade83ef3b439..9316399b920e 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -523,7 +523,7 @@ static int verity_verify_io(struct dm_verity_io *io)
>  		sector_t cur_block = io->block + b;
>  		struct ahash_request *req = verity_io_hash_req(v, io);
>  
> -		if (v->validated_blocks &&
> +		if (v->validated_blocks && bio->bi_status == BLK_STS_OK &&
>  		    likely(test_bit(cur_block, v->validated_blocks))) {
>  			verity_bv_skip_block(v, io, iter);
>  			continue;
> -- 

Looks good now, thanks!

Reviewed-by: Eric Biggers <ebiggers@google.com>

- Eric

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: Yeongjin Gil <youngjin.gil@samsung.com>
Cc: agk@redhat.com, snitzer@kernel.org, dm-devel@redhat.com,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Sungjong Seo <sj1557.seo@samsung.com>
Subject: Re: [PATCH v2] dm verity: fix error handling for check_at_most_once on FEC
Date: Mon, 20 Mar 2023 14:23:01 -0700	[thread overview]
Message-ID: <20230320212301.GD1434@sol.localdomain> (raw)
In-Reply-To: <20230320065932.28116-1-youngjin.gil@samsung.com>

On Mon, Mar 20, 2023 at 03:59:32PM +0900, Yeongjin Gil wrote:
> In verity_end_io(), if bi_status is not BLK_STS_OK, it can be return
> directly. But if FEC configured, it is desired to correct the data page
> through verity_verify_io. And the return value will be converted to
> blk_status and passed to verity_finish_io().
> 
> BTW, when a bit is set in v->validated_blocks, verity_verify_io() skips
> verification regardless of I/O error for the corresponding bio. In this
> case, the I/O error could not be returned properly, and as a result,
> there is a problem that abnormal data could be read for the
> corresponding block.
> 
> To fix this problem, when an I/O error occurs, do not skip verification
> even if the bit related is set in v->validated_blocks.
> 
> Fixes: 843f38d382b1 ("dm verity: add 'check_at_most_once' option to only validate hashes once")
> Cc: stable@vger.kernel.org
> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
> Signed-off-by: Yeongjin Gil <youngjin.gil@samsung.com>
> ---
> v2:
> -change commit message and tag
> ---
>  drivers/md/dm-verity-target.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index ade83ef3b439..9316399b920e 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -523,7 +523,7 @@ static int verity_verify_io(struct dm_verity_io *io)
>  		sector_t cur_block = io->block + b;
>  		struct ahash_request *req = verity_io_hash_req(v, io);
>  
> -		if (v->validated_blocks &&
> +		if (v->validated_blocks && bio->bi_status == BLK_STS_OK &&
>  		    likely(test_bit(cur_block, v->validated_blocks))) {
>  			verity_bv_skip_block(v, io, iter);
>  			continue;
> -- 

Looks good now, thanks!

Reviewed-by: Eric Biggers <ebiggers@google.com>

- Eric

  reply	other threads:[~2023-03-20 21:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20230320070011epcas1p12f0fe9f9f417dd1a3441efdde55a4132@epcas1p1.samsung.com>
2023-03-20  6:59 ` [dm-devel] [PATCH v2] dm verity: fix error handling for check_at_most_once on FEC Yeongjin Gil
2023-03-20  6:59   ` Yeongjin Gil
2023-03-20 21:23   ` Eric Biggers [this message]
2023-03-20 21:23     ` Eric Biggers

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=20230320212301.GD1434@sol.localdomain \
    --to=ebiggers@kernel.org \
    --cc=agk@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sj1557.seo@samsung.com \
    --cc=snitzer@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=youngjin.gil@samsung.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.