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@xxxxxxx +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
