linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Thumshirn <jthumshirn@suse.de>
To: Christoph Hellwig <hch@infradead.org>
Cc: Goldwyn Rodrigues <rgoldwyn@suse.de>,
	linux-btrfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	darrick.wong@oracle.com, fdmanana@kernel.org, dsterba@suse.cz,
	nborisov@suse.com, Goldwyn Rodrigues <rgoldwyn@suse.com>
Subject: Re: [PATCH 3/8] btrfs: Switch to iomap_dio_rw() for dio
Date: Thu, 12 Dec 2019 11:38:34 +0100	[thread overview]
Message-ID: <035b1e2d-09bd-0cd9-3e32-ad47194f7812@suse.de> (raw)
In-Reply-To: <20191212095752.GA31597@infradead.org>

On 12/12/2019 10:57, Christoph Hellwig wrote:
> On Thu, Dec 12, 2019 at 10:56:48AM +0100, Johannes Thumshirn wrote:
>> On 12/12/2019 10:49, Christoph Hellwig wrote:
>>>> @@ -8230,9 +8228,8 @@ static void btrfs_endio_direct_read(struct bio *bio)
>>>>  	kfree(dip);
>>>>  
>>>>  	dio_bio->bi_status = err;
>>>> -	dio_end_io(dio_bio);
>>>> +	bio_endio(dio_bio);
>>>>  	btrfs_io_bio_free_csum(io_bio);
>>>> -	bio_put(bio);
>>>
>>> I'm not a btrfs export, but doesn't this introduce a use after free
>>> as bio_endio also frees io_bio?
>>
>> No that's ok, as the bio_endio() frees the dio_bio, while
>> btrfs_io_bio_free_csum() frees the csum of the io_bio (which is a struct
>> btrfs_io_bio).
> 
> So who frees the io_bio and its embedded bio?
> 

In the old code this was handled by the now removed bio_put(). But I
admit I'm confused now.

The bio embedded in io_bio is a btrfs_bio_clone() of dio_bio() (from
btrfs_submit_direct()).

So we're now freeing the dio_bio a.k.a the original one in bio_endio()
but the clone seems to be leaking.

What am I missing?

-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

  reply	other threads:[~2019-12-12 10:38 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-12  0:30 [PATCH 0/8 v5] btrfs direct-io using iomap Goldwyn Rodrigues
2019-12-12  0:30 ` [PATCH 1/8] fs: Export generic_file_buffered_read() Goldwyn Rodrigues
2019-12-12  9:46   ` Christoph Hellwig
2019-12-12  0:30 ` [PATCH 2/8] iomap: add a filesystem hook for direct I/O bio submission Goldwyn Rodrigues
2019-12-12  9:47   ` Christoph Hellwig
2019-12-12  0:30 ` [PATCH 3/8] btrfs: Switch to iomap_dio_rw() for dio Goldwyn Rodrigues
2019-12-12  9:49   ` Christoph Hellwig
2019-12-12  9:56     ` Johannes Thumshirn
2019-12-12  9:57       ` Christoph Hellwig
2019-12-12 10:38         ` Johannes Thumshirn [this message]
2019-12-12 19:42     ` Goldwyn Rodrigues
2019-12-12  0:30 ` [PATCH 4/8] iomap: Move lockdep_assert_held() to iomap_dio_rw() calls Goldwyn Rodrigues
2019-12-12  9:50   ` Christoph Hellwig
2019-12-12 22:24     ` Goldwyn Rodrigues
2019-12-12 22:46     ` Dave Chinner
2019-12-12  0:30 ` [PATCH 5/8] fs: Remove dio_end_io() Goldwyn Rodrigues
2019-12-12  9:50   ` Christoph Hellwig
2019-12-12  0:30 ` [PATCH 6/8] btrfs: Wait for extent bits to release page Goldwyn Rodrigues
2019-12-12  0:30 ` [PATCH 7/8] btrfs: Use iomap_end() instead of btrfs_dio_data Goldwyn Rodrigues
2019-12-12  9:52   ` Christoph Hellwig
2019-12-12  0:30 ` [PATCH 8/8] btrfs: remove BTRFS_INODE_READDIO_NEED_LOCK Goldwyn Rodrigues
2019-12-12  8:53   ` Johannes Thumshirn

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=035b1e2d-09bd-0cd9-3e32-ad47194f7812@suse.de \
    --to=jthumshirn@suse.de \
    --cc=darrick.wong@oracle.com \
    --cc=dsterba@suse.cz \
    --cc=fdmanana@kernel.org \
    --cc=hch@infradead.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=nborisov@suse.com \
    --cc=rgoldwyn@suse.com \
    --cc=rgoldwyn@suse.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).