All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: "Matias Bjørling" <m@bjorling.me>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: BUG: KASAN: Use-after-free
Date: Mon, 23 Jan 2017 09:20:21 -0800	[thread overview]
Message-ID: <20170123172021.GA28446@infradead.org> (raw)
In-Reply-To: <0b458b92-62ee-bffc-8f3e-fbd6490b26fd@bjorling.me>

On Mon, Jan 23, 2017 at 05:07:52PM +0100, Matias Bj�rling wrote:
> Hi,
> 
> I could use some help verifying an use-after-free bug that I am seeing
> after the new direct I/O work went in.
> 
> When issuing a direct write io using libaio, a bio is referenced in the
> blkdev_direct_IO path, and then put again in the blkdev_bio_end_io path.
> However, there is a case where the bio is put twice, which leads to
> modules that rely on the bio after biodev_bio_end_io() to access it
> prematurely.

Can you reproduce anything like this with a normal block device?

> 
> The KASAN error report:
> 
> [   14.645916]
> ==================================================================
> [   14.648027] BUG: KASAN: use-after-free in
> blkdev_direct_IO+0x50c/0x600 at addr ffff8801ef30ea14

Can you resolve that address for me, please?

> Which boils down to the bio already being freed, when we hit the
> module's endio function.
> 
> The double bio_put() occurs when dio->should_dirty = 0, and dio->is_sync
> = 0. The flow follows:
> 
> Issuing:
>   blkdev_direct_IO
>      get_bio(bio)

bio refcounting in __blkdev_direct_IO is the following

first bio is allocated using bio_alloc_bioset to embedd the dio structure
into it.  We then grab a second reference to that bio and only that one.
Each other new bio just gets it's single reference from bio_alloc.

blkdev_bio_end_io then checks if we hit the last reference on the dio, in
which case it then drops the additional reference on the first bio after
calling the aio completion handler.  Once that is done it always drops the
reference for the current bio - either explicitly or through
bio_check_pages_dirty in the should_dirty case.

>      submit_io()
>        rrpc make_request(bio)
>          get_bio(bio)
> Completion:
>   blkdev_bio_end_io
>     bio_put(bio)
>     bio_put(bio) - bio is freed

Yes, and that's how it's intended.

>   rrpc_end_io
>     bio_put(bio) (use-after-free access)

Can you check this is the same bio that got submitted and it didn't
get bounced somewhere?

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: "Matias Bjørling" <m@bjorling.me>
Cc: LKML <linux-kernel@vger.kernel.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	Christoph Hellwig <hch@infradead.org>
Subject: Re: BUG: KASAN: Use-after-free
Date: Mon, 23 Jan 2017 09:20:21 -0800	[thread overview]
Message-ID: <20170123172021.GA28446@infradead.org> (raw)
In-Reply-To: <0b458b92-62ee-bffc-8f3e-fbd6490b26fd@bjorling.me>

On Mon, Jan 23, 2017 at 05:07:52PM +0100, Matias Bjørling wrote:
> Hi,
> 
> I could use some help verifying an use-after-free bug that I am seeing
> after the new direct I/O work went in.
> 
> When issuing a direct write io using libaio, a bio is referenced in the
> blkdev_direct_IO path, and then put again in the blkdev_bio_end_io path.
> However, there is a case where the bio is put twice, which leads to
> modules that rely on the bio after biodev_bio_end_io() to access it
> prematurely.

Can you reproduce anything like this with a normal block device?

> 
> The KASAN error report:
> 
> [   14.645916]
> ==================================================================
> [   14.648027] BUG: KASAN: use-after-free in
> blkdev_direct_IO+0x50c/0x600 at addr ffff8801ef30ea14

Can you resolve that address for me, please?

> Which boils down to the bio already being freed, when we hit the
> module's endio function.
> 
> The double bio_put() occurs when dio->should_dirty = 0, and dio->is_sync
> = 0. The flow follows:
> 
> Issuing:
>   blkdev_direct_IO
>      get_bio(bio)

bio refcounting in __blkdev_direct_IO is the following

first bio is allocated using bio_alloc_bioset to embedd the dio structure
into it.  We then grab a second reference to that bio and only that one.
Each other new bio just gets it's single reference from bio_alloc.

blkdev_bio_end_io then checks if we hit the last reference on the dio, in
which case it then drops the additional reference on the first bio after
calling the aio completion handler.  Once that is done it always drops the
reference for the current bio - either explicitly or through
bio_check_pages_dirty in the should_dirty case.

>      submit_io()
>        rrpc make_request(bio)
>          get_bio(bio)
> Completion:
>   blkdev_bio_end_io
>     bio_put(bio)
>     bio_put(bio) - bio is freed

Yes, and that's how it's intended.

>   rrpc_end_io
>     bio_put(bio) (use-after-free access)

Can you check this is the same bio that got submitted and it didn't
get bounced somewhere?

  reply	other threads:[~2017-01-23 17:20 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-23 16:07 BUG: KASAN: Use-after-free Matias Bjørling
2017-01-23 17:20 ` Christoph Hellwig [this message]
2017-01-23 17:20   ` Christoph Hellwig
2017-01-24  9:32   ` Matias Bjørling
2017-01-24  9:32     ` Matias Bjørling
2017-01-24  9:52     ` Christoph Hellwig
2017-01-24  9:52       ` Christoph Hellwig
2017-01-24 10:01       ` Matias Bjørling
2017-01-24 10:01         ` Matias Bjørling
2017-01-24 13:34         ` Christoph Hellwig
2017-01-24 13:34           ` Christoph Hellwig
2017-01-24 13:37           ` Matias Bjørling
2017-01-24 13:37             ` Matias Bjørling

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=20170123172021.GA28446@infradead.org \
    --to=hch@infradead.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m@bjorling.me \
    /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.