Re: [PATCH v7 00/39] btrfs: zoned block device support

From: Johannes Thumshirn <Johannes.Thumshirn@wdc.com>
To: "dsterba@suse.cz" <dsterba@suse.cz>, Naohiro Aota <Naohiro.Aota@wdc.com>
Cc: "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
	David Sterba <dsterba@suse.com>, Chris Mason <clm@fb.com>,
	Josef Bacik <josef@toxicpanda.com>,
	Hannes Reinecke <hare@suse.com>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v7 00/39] btrfs: zoned block device support
Date: Wed, 16 Sep 2020 17:42:50 +0000	[thread overview]
Message-ID: <SN4PR0401MB359839054A125BF64641B4E89B210@SN4PR0401MB3598.namprd04.prod.outlook.com> (raw)
In-Reply-To: 20200915080927.GF1791@twin.jikos.cz

On 15/09/2020 10:25, David Sterba wrote:
> On Fri, Sep 11, 2020 at 09:32:20PM +0900, Naohiro Aota wrote:
>> Changelog
>> v6:
>>  - Use bitmap helpers (Johannes)
>>  - Code cleanup (Johannes)
>>  - Rebased on kdave/for-5.5
>>  - Enable the tree-log feature.
>>  - Treat conventional zones as sequential zones, so we can now allow
>>    mixed allocation of conventional zone and sequential write required
>>    zone to construct a block group.
>>  - Implement log-structured superblock
>>    - No need for one conventional zone at the beginning of a device.
>>  - Fix deadlock of direct IO writing
>>  - Fix building with !CONFIG_BLK_DEV_ZONED (Johannes)
>>  - Fix leak of zone_info (Johannes)
> 
> I did a quick check to see if the patchset passes the default VM tests
> and there's use after free short after the fstests start. No zoned
> devices or such. I had to fix some conflicts when rebasing on misc-next
> but I tried to base it on the last iomap-dio patch ("btrfs: switch to
> iomap for direct IO"), same result so it's something in the zoned
> patches.
> 
> The reported pointer 0x6b6b6b6b6d1918eb contains the use-after-free
> poison (0x6b) (CONFIG_PAGE_POISONING=y).
> 
> MKFS_OPTIONS  -- -f -K --csum xxhash /dev/vdb
> MOUNT_OPTIONS -- -o discard /dev/vdb /tmp/scratch

Hi David,

Can you check if this on top of the series fixes the issue? According
to Keith we can't call bio_iovec() from endio() as the iterator is already
advanced (see req_bio_endio()).

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index bda4e02b5eab..311956697682 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2753,10 +2753,6 @@ static void end_bio_extent_writepage(struct bio *bio)
        u64 end;
        struct bvec_iter_all iter_all;
 
-       btrfs_record_physical_zoned(bio_iovec(bio).bv_page->mapping->host,
-                                   page_offset(bio_iovec(bio).bv_page) + bio_iovec(bio).bv_offset,
-                                   bio);
-
        ASSERT(!bio_flagged(bio, BIO_CLONED));
        bio_for_each_segment_all(bvec, bio, iter_all) {
                struct page *page = bvec->bv_page;
@@ -2782,6 +2778,7 @@ static void end_bio_extent_writepage(struct bio *bio)
                start = page_offset(page);
                end = start + bvec->bv_offset + bvec->bv_len - 1;
 
+               btrfs_record_physical_zoned(inode, start, bio);
                end_extent_writepage(page, error, start, end);
                end_page_writeback(page);
        }
diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 576f8e333f16..6fdb21029ea9 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1086,8 +1086,7 @@ void btrfs_record_physical_zoned(struct inode *inode, u64 file_offset,
 {
        struct btrfs_ordered_extent *ordered;
        struct bio_vec bvec = bio_iovec(bio);
-       u64 physical = ((u64)bio->bi_iter.bi_sector << SECTOR_SHIFT) +
-               bvec.bv_offset;
+       u64 physical = (u64)bio->bi_iter.bi_sector << SECTOR_SHIFT;
 
        if (bio_op(bio) != REQ_OP_ZONE_APPEND)
                return;
