Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 29 Nov 2022 08:51:05 -0800
From:   Jaegeuk Kim <jaegeuk@kernel.org>
To:     Eric Biggers <ebiggers@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
Message-ID: <Y4Y4eaMvCBZMgig0@google.com>
References: <20221125190642.12787-1-ebiggers@kernel.org>
 <Y4WrSeIf+E6+tL1y@google.com>
 <Y4WxUZesKJ79mI9e@sol.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y4WxUZesKJ79mI9e@sol.localdomain>
Precedence: bulk

On 11/28, Eric Biggers wrote:
> 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.

I used for fsstress+fault_injection+shutdown loop with compressed and
non-compressed directories with:

# mkfs.f2fs -f -O extra_attr -O project_quota -O compression -g android /dev/$DEV
# mount -t f2fs -o discard,fsync_mode=nobarrier,reserve_root=32768,checkpoint_merge,atgc,compress_cache /dev/$DEV $TESTDIR
# mkdir $TESTDIR/comp
# f2fs_io setflags compression $TESTDIR/comp
# fsstress [options] -d $TESTDIR/comp

I think you can simply mount with "-o compress_extension=*" to compress
everything.

> 
> - Eric