From: Christoph Hellwig <hch@lst.de> To: Qian Cai <quic_qiancai@quicinc.com> Cc: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>, Tejun Heo <tj@kernel.org>, Jan Kara <jack@suse.cz>, linux-block@vger.kernel.org, Andrew Morton <akpm@linux-foundation.org>, cgroups@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org Subject: Re: move the bdi from the request_queue to the gendisk Date: Tue, 10 Aug 2021 22:02:56 +0200 [thread overview] Message-ID: <20210810200256.GA30809@lst.de> (raw) In-Reply-To: <e5e19d15-7efd-31f4-941a-a5eb2f94b898@quicinc.com> On Tue, Aug 10, 2021 at 03:36:39PM -0400, Qian Cai wrote: > > > On 8/9/2021 10:17 AM, Christoph Hellwig wrote: > > Hi Jens, > > > > this series moves the pointer to the bdi from the request_queue > > to the bdi, better matching the life time rules of the different > > objects. > > Reverting this series fixed an use-after-free in bdev_evict_inode(). Please try the patch below as a band-aid. Although the proper fix is that non-default bdi_writeback structures grab a reference to the bdi, as this was a landmine that might have already caused spurious issues before. diff --git a/block/genhd.c b/block/genhd.c index f8def1129501..2e4a9d187196 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -1086,7 +1086,6 @@ static void disk_release(struct device *dev) might_sleep(); - bdi_put(disk->bdi); if (MAJOR(dev->devt) == BLOCK_EXT_MAJOR) blk_free_ext_minor(MINOR(dev->devt)); disk_release_events(disk); diff --git a/fs/block_dev.c b/fs/block_dev.c index 7c969f81327a..c6087dbae6cf 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -849,11 +849,15 @@ static void init_once(void *data) static void bdev_evict_inode(struct inode *inode) { + struct block_device *bdev = I_BDEV(inode); + truncate_inode_pages_final(&inode->i_data); invalidate_inode_buffers(inode); /* is it needed here? */ clear_inode(inode); /* Detach inode from wb early as bdi_put() may free bdi->wb */ inode_detach_wb(inode); + if (!bdev_is_partition(bdev)) + bdi_put(bdev->bd_disk->bdi); } static const struct super_operations bdev_sops = {
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> To: Qian Cai <quic_qiancai-jfJNa2p1gH1BDgjK7y7TUQ@public.gmane.org> Cc: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>, Jens Axboe <axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org>, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Jan Kara <jack-AlSwsSmVLrQ@public.gmane.org>, linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org Subject: Re: move the bdi from the request_queue to the gendisk Date: Tue, 10 Aug 2021 22:02:56 +0200 [thread overview] Message-ID: <20210810200256.GA30809@lst.de> (raw) In-Reply-To: <e5e19d15-7efd-31f4-941a-a5eb2f94b898-jfJNa2p1gH1BDgjK7y7TUQ@public.gmane.org> On Tue, Aug 10, 2021 at 03:36:39PM -0400, Qian Cai wrote: > > > On 8/9/2021 10:17 AM, Christoph Hellwig wrote: > > Hi Jens, > > > > this series moves the pointer to the bdi from the request_queue > > to the bdi, better matching the life time rules of the different > > objects. > > Reverting this series fixed an use-after-free in bdev_evict_inode(). Please try the patch below as a band-aid. Although the proper fix is that non-default bdi_writeback structures grab a reference to the bdi, as this was a landmine that might have already caused spurious issues before. diff --git a/block/genhd.c b/block/genhd.c index f8def1129501..2e4a9d187196 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -1086,7 +1086,6 @@ static void disk_release(struct device *dev) might_sleep(); - bdi_put(disk->bdi); if (MAJOR(dev->devt) == BLOCK_EXT_MAJOR) blk_free_ext_minor(MINOR(dev->devt)); disk_release_events(disk); diff --git a/fs/block_dev.c b/fs/block_dev.c index 7c969f81327a..c6087dbae6cf 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -849,11 +849,15 @@ static void init_once(void *data) static void bdev_evict_inode(struct inode *inode) { + struct block_device *bdev = I_BDEV(inode); + truncate_inode_pages_final(&inode->i_data); invalidate_inode_buffers(inode); /* is it needed here? */ clear_inode(inode); /* Detach inode from wb early as bdi_put() may free bdi->wb */ inode_detach_wb(inode); + if (!bdev_is_partition(bdev)) + bdi_put(bdev->bd_disk->bdi); } static const struct super_operations bdev_sops = {
next prev parent reply other threads:[~2021-08-10 20:03 UTC|newest] Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-09 14:17 move the bdi from the request_queue to the gendisk Christoph Hellwig 2021-08-09 14:17 ` [PATCH 1/5] mm: hide laptop_mode_wb_timer entirely behind the BDI API Christoph Hellwig 2021-08-09 14:17 ` Christoph Hellwig 2021-08-09 14:33 ` Johannes Thumshirn 2021-08-09 14:33 ` Johannes Thumshirn 2021-08-09 15:10 ` Jan Kara 2021-08-09 15:10 ` Jan Kara 2021-08-10 21:56 ` Guenter Roeck 2021-08-10 21:56 ` Guenter Roeck 2021-08-11 5:22 ` Christoph Hellwig 2021-08-11 5:22 ` Christoph Hellwig 2021-08-09 14:17 ` [PATCH 2/5] block: pass a gendisk to blk_queue_update_readahead Christoph Hellwig 2021-08-09 14:17 ` Christoph Hellwig 2021-08-09 14:35 ` Johannes Thumshirn 2021-08-09 15:17 ` Jan Kara 2021-08-09 14:17 ` [PATCH 3/5] block: add a queue_has_disk helper Christoph Hellwig 2021-08-09 14:17 ` Christoph Hellwig 2021-08-09 14:37 ` Johannes Thumshirn 2021-08-09 15:18 ` Jan Kara 2021-08-09 14:17 ` [PATCH 4/5] block: move the bdi from the request_queue to the gendisk Christoph Hellwig 2021-08-09 14:38 ` Johannes Thumshirn 2021-08-09 14:38 ` Johannes Thumshirn 2021-08-09 15:47 ` Jan Kara 2021-08-09 17:57 ` Jens Axboe 2021-08-09 17:57 ` Jens Axboe 2021-08-09 21:29 ` Jan Kara 2021-08-10 16:44 ` Christoph Hellwig 2021-10-14 14:31 ` [sparc64] kernel OOPS (was: [PATCH 4/5] block: move the bdi from the request_queue to the gendisk) Anatoly Pugachev 2021-10-14 14:32 ` Christoph Hellwig 2021-10-14 14:32 ` Christoph Hellwig 2021-10-14 20:27 ` Anatoly Pugachev 2021-08-09 14:17 ` [PATCH 5/5] block: remove the bd_bdi in struct block_device Christoph Hellwig 2021-08-09 14:17 ` Christoph Hellwig 2021-08-09 14:55 ` Johannes Thumshirn 2021-08-09 15:49 ` Jan Kara 2021-08-09 21:42 ` move the bdi from the request_queue to the gendisk Jens Axboe 2021-08-09 21:42 ` Jens Axboe 2021-08-10 19:36 ` Qian Cai 2021-08-10 19:36 ` Qian Cai 2021-08-10 20:02 ` Christoph Hellwig [this message] 2021-08-10 20:02 ` Christoph Hellwig 2021-08-11 2:28 ` Qian Cai 2021-08-11 2:28 ` Qian Cai 2021-08-11 11:25 ` Jan Kara 2021-08-11 11:51 ` Christoph Hellwig 2021-08-11 11:51 ` Christoph Hellwig 2021-08-11 12:47 ` Jan Kara
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=20210810200256.GA30809@lst.de \ --to=hch@lst.de \ --cc=akpm@linux-foundation.org \ --cc=axboe@kernel.dk \ --cc=cgroups@vger.kernel.org \ --cc=jack@suse.cz \ --cc=linux-block@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=quic_qiancai@quicinc.com \ --cc=tj@kernel.org \ /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: linkBe 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.