All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-ext4@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	Matthew Wilcox <willy@infradead.org>, Chao Yu <chao@kernel.org>
Subject: Re: [PATCH v4] fsverity: stop using PG_error to track error status
Date: Mon, 28 Nov 2022 23:14:25 -0800	[thread overview]
Message-ID: <Y4WxUZesKJ79mI9e@sol.localdomain> (raw)
In-Reply-To: <Y4WrSeIf+E6+tL1y@google.com>

On Mon, Nov 28, 2022 at 10:48:41PM -0800, Jaegeuk Kim wrote:
> >  static void f2fs_finish_read_bio(struct bio *bio, bool in_task)
> >  {
> >  	struct bio_vec *bv;
> >  	struct bvec_iter_all iter_all;
> > +	struct bio_post_read_ctx *ctx = bio->bi_private;
> >  
> > -	/*
> > -	 * Update and unlock the bio's pagecache pages, and put the
> > -	 * decompression context for any compressed pages.
> > -	 */
> >  	bio_for_each_segment_all(bv, bio, iter_all) {
> >  		struct page *page = bv->bv_page;
> >  
> >  		if (f2fs_is_compressed_page(page)) {
> > -			if (bio->bi_status)
> > +			if (!ctx->decompression_attempted)
> 
> If seems this causes a panic due to the ctx nullified by f2fs_verify_bio.
> 

Thanks for catching that!  I've sent out v5 that checks for 'ctx &&
!ctx->decompression_attempted' here.  That's the right thing to do, since if ctx
is NULL then decompression must have been attempted.

I'd like to get rid of freeing the bio_post_read_ctx in f2fs_verify_bio().
But I believe it's still needed, at least in theory.

Do you have a suggestion for testing f2fs compression + verity with xfstests?
I missed this because compression isn't covered by the "verity" group tests.
Maybe there should be an "f2fs/compress" config in xfstests-bld that uses mkfs
and mount options that cause all files to be automatically compressed, similar
to how f2fs/encrypt automatically encrypts all files with test_dummy_encryption.

- Eric

WARNING: multiple messages have this Message-ID (diff)
From: Eric Biggers <ebiggers@kernel.org>
To: Jaegeuk Kim <jaegeuk@kernel.org>
Cc: Matthew Wilcox <willy@infradead.org>,
	linux-f2fs-devel@lists.sourceforge.net,
	linux-fscrypt@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-ext4@vger.kernel.org
Subject: Re: [f2fs-dev] [PATCH v4] fsverity: stop using PG_error to track error status
Date: Mon, 28 Nov 2022 23:14:25 -0800	[thread overview]
Message-ID: <Y4WxUZesKJ79mI9e@sol.localdomain> (raw)
In-Reply-To: <Y4WrSeIf+E6+tL1y@google.com>

On Mon, Nov 28, 2022 at 10:48:41PM -0800, Jaegeuk Kim wrote:
> >  static void f2fs_finish_read_bio(struct bio *bio, bool in_task)
> >  {
> >  	struct bio_vec *bv;
> >  	struct bvec_iter_all iter_all;
> > +	struct bio_post_read_ctx *ctx = bio->bi_private;
> >  
> > -	/*
> > -	 * Update and unlock the bio's pagecache pages, and put the
> > -	 * decompression context for any compressed pages.
> > -	 */
> >  	bio_for_each_segment_all(bv, bio, iter_all) {
> >  		struct page *page = bv->bv_page;
> >  
> >  		if (f2fs_is_compressed_page(page)) {
> > -			if (bio->bi_status)
> > +			if (!ctx->decompression_attempted)
> 
> If seems this causes a panic due to the ctx nullified by f2fs_verify_bio.
> 

Thanks for catching that!  I've sent out v5 that checks for 'ctx &&
!ctx->decompression_attempted' here.  That's the right thing to do, since if ctx
is NULL then decompression must have been attempted.

I'd like to get rid of freeing the bio_post_read_ctx in f2fs_verify_bio().
But I believe it's still needed, at least in theory.

Do you have a suggestion for testing f2fs compression + verity with xfstests?
I missed this because compression isn't covered by the "verity" group tests.
Maybe there should be an "f2fs/compress" config in xfstests-bld that uses mkfs
and mount options that cause all files to be automatically compressed, similar
to how f2fs/encrypt automatically encrypts all files with test_dummy_encryption.

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2022-11-29  7:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-25 19:06 [PATCH v4] fsverity: stop using PG_error to track error status Eric Biggers
2022-11-25 19:06 ` [f2fs-dev] " Eric Biggers
2022-11-28 19:18 ` Jaegeuk Kim
2022-11-28 19:18   ` Jaegeuk Kim
2022-11-29  6:48 ` Jaegeuk Kim
2022-11-29  6:48   ` [f2fs-dev] " Jaegeuk Kim
2022-11-29  7:14   ` Eric Biggers [this message]
2022-11-29  7:14     ` Eric Biggers
2022-11-29 16:51     ` Jaegeuk Kim
2022-11-29 16:51       ` Jaegeuk Kim

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=Y4WxUZesKJ79mI9e@sol.localdomain \
    --to=ebiggers@kernel.org \
    --cc=chao@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=willy@infradead.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 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.