[v2,3/7] bdi: protect device lifetime with RCU
diff mbox series

Message ID 20200226111851.55348-4-yuyufen@huawei.com
State New
Headers show
Series
  • bdi: fix use-after-free for bdi device
Related show

Commit Message

Yufen Yu Feb. 26, 2020, 11:18 a.m. UTC
We reported a kernel crash:

[201962.639350] Call trace:
[201962.644403]  string+0x28/0xa0
[201962.650501]  vsnprintf+0x5f0/0x748
[201962.657472]  seq_vprintf+0x70/0x98
[201962.664442]  seq_printf+0x7c/0xa0
[201962.671238]  __blkg_prfill_rwstat+0x84/0x128
[201962.679949]  blkg_prfill_rwstat_field+0x94/0xc0
[201962.689182]  blkcg_print_blkgs+0xcc/0x140
[201962.697370]  blkg_print_stat_bytes+0x4c/0x60
[201962.706083]  cgroup_seqfile_show+0x58/0xc0
[201962.714446]  kernfs_seq_show+0x44/0x50
[201962.722112]  seq_read+0xd4/0x4a8
[201962.728732]  kernfs_fop_read+0x16c/0x218
[201962.736748]  __vfs_read+0x60/0x188
[201962.743717]  vfs_read+0x94/0x150
[201962.750338]  ksys_read+0x6c/0xd8
[201962.756958]  __arm64_sys_read+0x24/0x30
[201962.764800]  el0_svc_common+0x78/0x130
[201962.772466]  el0_svc_handler+0x38/0x78
[201962.780131]  el0_svc+0x8/0xc

__blkg_prfill_rwstat() tries to get the device name by 'bdi->dev',
while the device and kobj->name has been freed by bdi_unregister().
Then, blkg_dev_name() will return an invalid name pointer, resulting
in crash on string(). The race as following:

CPU1                          CPU2
blkg_print_stat_bytes
                              __scsi_remove_device
                              del_gendisk
                                bdi_unregister

                                put_device(bdi->dev)
                                  kfree(bdi->dev)
__blkg_prfill_rwstat
  blkg_dev_name
    //use the freed bdi->dev
    dev_name(blkg->q->backing_dev_info->dev)
                                bdi->dev = NULL

We fix the bug by protecting the device lifetime with RCU.
call_rcu() will free the device until all of the readers complete.

Since both of blkg_dev_name() and device name have been protected
by rcu_read_lock/unlock(). Thus, we don't need to do it again.
Just use rcu_dereference() to fetch rcu_dev here.

Signed-off-by: Yufen Yu <yuyufen@huawei.com>
---
 block/bfq-iosched.c              |  2 +-
 block/blk-cgroup.c               |  8 ++++--
 block/genhd.c                    |  4 +--
 fs/ext4/super.c                  |  2 +-
 include/linux/backing-dev-defs.h |  8 +++++-
 include/linux/backing-dev.h      |  4 +--
 mm/backing-dev.c                 | 59 +++++++++++++++++++++++++++++++++-------
 7 files changed, 68 insertions(+), 19 deletions(-)

Comments

Tejun Heo March 4, 2020, 5:05 p.m. UTC | #1
Hello,

It might be better to put this patch at the end rather than in the
middle so that when this patch is applied things are actually fixed.

> +struct bdi_rcu_device {
> +	struct device dev;
> +	struct rcu_head rcu_head;
> +};

(cc'ing Greg)

Greg, block layer switches association between backing_device_info and
its struct device and needs to protect it with RCU. Yufen did so by
introducing a wrapping struct around struct device like above. Do you
think it'd make sense to just embed rcu_head into struct device and
let put_device() to RCU release by default?

Thanks.
Greg Kroah-Hartman March 4, 2020, 5:22 p.m. UTC | #2
On Wed, Mar 04, 2020 at 12:05:43PM -0500, Tejun Heo wrote:
> Hello,
> 
> It might be better to put this patch at the end rather than in the
> middle so that when this patch is applied things are actually fixed.
> 
> > +struct bdi_rcu_device {
> > +	struct device dev;
> > +	struct rcu_head rcu_head;
> > +};
> 
> (cc'ing Greg)
> 
> Greg, block layer switches association between backing_device_info and
> its struct device and needs to protect it with RCU. Yufen did so by
> introducing a wrapping struct around struct device like above. Do you
> think it'd make sense to just embed rcu_head into struct device and
> let put_device() to RCU release by default?

Ugh, I was dreading the fact that this day might sometime come...

In theory, the reference counting for struct device shouldn't need to
use rcu at all, right?  what is driving the need to use rcu for
backing_device_info?  Are these being destroyed/used so often that rcu
really is the best solution and the existing reference counting doesn't
work properly?

Some context is needed here.

thanks,

greg k-h
Greg Kroah-Hartman March 4, 2020, 5:23 p.m. UTC | #3
On Wed, Mar 04, 2020 at 06:22:21PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Mar 04, 2020 at 12:05:43PM -0500, Tejun Heo wrote:
> > Hello,
> > 
> > It might be better to put this patch at the end rather than in the
> > middle so that when this patch is applied things are actually fixed.
> > 
> > > +struct bdi_rcu_device {
> > > +	struct device dev;
> > > +	struct rcu_head rcu_head;
> > > +};
> > 
> > (cc'ing Greg)
> > 
> > Greg, block layer switches association between backing_device_info and
> > its struct device and needs to protect it with RCU. Yufen did so by
> > introducing a wrapping struct around struct device like above. Do you
> > think it'd make sense to just embed rcu_head into struct device and
> > let put_device() to RCU release by default?
> 
> Ugh, I was dreading the fact that this day might sometime come...
> 
> In theory, the reference counting for struct device shouldn't need to
> use rcu at all, right?  what is driving the need to use rcu for
> backing_device_info?  Are these being destroyed/used so often that rcu
> really is the best solution and the existing reference counting doesn't
> work properly?
> 
> Some context is needed here.

Ah, would help if I just read the whole patch series, that helps...

Something is odd if kobj->name is the problem here, let me look at the
patches in full.

thanks,

greg k-h
Tejun Heo March 4, 2020, 6:50 p.m. UTC | #4
Hello,

On Wed, Mar 04, 2020 at 06:22:21PM +0100, Greg Kroah-Hartman wrote:
> Ugh, I was dreading the fact that this day might sometime come...
> 
> In theory, the reference counting for struct device shouldn't need to
> use rcu at all, right?  what is driving the need to use rcu for

Lifetime rules in block layer are kinda nebulous. Some of it comes
from the fact that some objects are reused. Instead of the usual,
create-use-release, they get repurposed to be associated with
something else. When looking at such an object from some paths, we
don't necessarily have ownership of all of the members.

> backing_device_info?  Are these being destroyed/used so often that rcu
> really is the best solution and the existing reference counting doesn't
> work properly?

It's more that there are entry points which can only ensure that just
the top level object is valid and the member objects might be going or
coming as we're looking at it.

Thanks.
Theodore Y. Ts'o March 4, 2020, 7:10 p.m. UTC | #5
On Wed, Mar 04, 2020 at 01:50:56PM -0500, Tejun Heo wrote:
> 
> Lifetime rules in block layer are kinda nebulous. Some of it comes
> from the fact that some objects are reused. Instead of the usual,
> create-use-release, they get repurposed to be associated with
> something else. When looking at such an object from some paths, we
> don't necessarily have ownership of all of the members.

I wonder if the current rules should be better documented, and that
perhaps we should revisit some of them so we can tighten them down?
For things that are likely to be long-lived, such as anything
corresponding to a bdi or block device, perhaps it would be better if
the lifetime rules can be made tighter?  The cost of needing to
release and reallocate longer lived objects is going to be negligible,
and benefits of improving code readability, reliability, and
robuestness might be well worth it.

     		  	       	   	       - Ted
Tejun Heo March 4, 2020, 7:15 p.m. UTC | #6
Hello,

On Wed, Mar 04, 2020 at 02:10:26PM -0500, Theodore Y. Ts'o wrote:
> On Wed, Mar 04, 2020 at 01:50:56PM -0500, Tejun Heo wrote:
> > Lifetime rules in block layer are kinda nebulous. Some of it comes
> > from the fact that some objects are reused. Instead of the usual,
> > create-use-release, they get repurposed to be associated with
> > something else. When looking at such an object from some paths, we
> > don't necessarily have ownership of all of the members.
> 
> I wonder if the current rules should be better documented, and that
> perhaps we should revisit some of them so we can tighten them down?

Oh yeah, that'd be nice for sure. We've been papering over stuff
constantly for probably over a decade now. It'd be really nice if we
could clean the house and have sane nominal lifetime rules for block
objects.

> For things that are likely to be long-lived, such as anything
> corresponding to a bdi or block device, perhaps it would be better if
> the lifetime rules can be made tighter?  The cost of needing to
> release and reallocate longer lived objects is going to be negligible,
> and benefits of improving code readability, reliability, and
> robuestness might be well worth it.

I full-heartedly agree. It's just a lot of historical accumulation and
not a lot of manpower directed at cleaning it up.

Thanks.
Greg Kroah-Hartman March 4, 2020, 8:05 p.m. UTC | #7
On Wed, Mar 04, 2020 at 01:50:56PM -0500, Tejun Heo wrote:
> Hello,
> 
> On Wed, Mar 04, 2020 at 06:22:21PM +0100, Greg Kroah-Hartman wrote:
> > Ugh, I was dreading the fact that this day might sometime come...
> > 
> > In theory, the reference counting for struct device shouldn't need to
> > use rcu at all, right?  what is driving the need to use rcu for
> 
> Lifetime rules in block layer are kinda nebulous. Some of it comes
> from the fact that some objects are reused. Instead of the usual,
> create-use-release, they get repurposed to be associated with
> something else. When looking at such an object from some paths, we
> don't necessarily have ownership of all of the members.

That's horrid, it's not like block devices are on some "fast path" for
tear-down, we should do it correctly.

> > backing_device_info?  Are these being destroyed/used so often that rcu
> > really is the best solution and the existing reference counting doesn't
> > work properly?
> 
> It's more that there are entry points which can only ensure that just
> the top level object is valid and the member objects might be going or
> coming as we're looking at it.

That's not ok, a "member object" can only be valid if you have a
reference to it.  If you remove the object, you then drop the reference,
shouldn't that be the correct thing to do?

thanks,

greg k-h
Tejun Heo March 5, 2020, 1:22 a.m. UTC | #8
Hello,

On Wed, Mar 04, 2020 at 09:05:59PM +0100, Greg Kroah-Hartman wrote:
> > Lifetime rules in block layer are kinda nebulous. Some of it comes
> > from the fact that some objects are reused. Instead of the usual,
> > create-use-release, they get repurposed to be associated with
> > something else. When looking at such an object from some paths, we
> > don't necessarily have ownership of all of the members.
> 
> That's horrid, it's not like block devices are on some "fast path" for
> tear-down, we should do it correctly.

Yeah, it got retrofitted umpteenth times from the really early days. I
don't think much of it is intentionally designed to be this way.

> > > backing_device_info?  Are these being destroyed/used so often that rcu
> > > really is the best solution and the existing reference counting doesn't
> > > work properly?
> > 
> > It's more that there are entry points which can only ensure that just
> > the top level object is valid and the member objects might be going or
> > coming as we're looking at it.
> 
> That's not ok, a "member object" can only be valid if you have a
> reference to it.  If you remove the object, you then drop the reference,
> shouldn't that be the correct thing to do?

I mean, it depends. There are two layers of objects and the top level
object has two stacked lifetime rules. The "active" usage pins
everything as usual. The "shallower" usage only has full access to the
top level and when it reaches down into members it needs a different
mechanism to ensure its validity. Given a clean slate, I don't think
we'd go for this design for these objects but the usage isn't
fundamentally broken.

Idk, for the problem at hand, the choice is between patching it up by
copying the name and RCU protecting ->dev access at least for now.
Both are nasty in their own ways but copying does have a smaller blast
radius. So, copy for now?

Thanks.
Greg Kroah-Hartman March 6, 2020, 4:25 p.m. UTC | #9
On Wed, Mar 04, 2020 at 08:22:11PM -0500, Tejun Heo wrote:
> Hello,
> 
> On Wed, Mar 04, 2020 at 09:05:59PM +0100, Greg Kroah-Hartman wrote:
> > > Lifetime rules in block layer are kinda nebulous. Some of it comes
> > > from the fact that some objects are reused. Instead of the usual,
> > > create-use-release, they get repurposed to be associated with
> > > something else. When looking at such an object from some paths, we
> > > don't necessarily have ownership of all of the members.
> > 
> > That's horrid, it's not like block devices are on some "fast path" for
> > tear-down, we should do it correctly.
> 
> Yeah, it got retrofitted umpteenth times from the really early days. I
> don't think much of it is intentionally designed to be this way.
> 
> > > > backing_device_info?  Are these being destroyed/used so often that rcu
> > > > really is the best solution and the existing reference counting doesn't
> > > > work properly?
> > > 
> > > It's more that there are entry points which can only ensure that just
> > > the top level object is valid and the member objects might be going or
> > > coming as we're looking at it.
> > 
> > That's not ok, a "member object" can only be valid if you have a
> > reference to it.  If you remove the object, you then drop the reference,
> > shouldn't that be the correct thing to do?
> 
> I mean, it depends. There are two layers of objects and the top level
> object has two stacked lifetime rules. The "active" usage pins
> everything as usual. The "shallower" usage only has full access to the
> top level and when it reaches down into members it needs a different
> mechanism to ensure its validity. Given a clean slate, I don't think
> we'd go for this design for these objects but the usage isn't
> fundamentally broken.
> 
> Idk, for the problem at hand, the choice is between patching it up by
> copying the name and RCU protecting ->dev access at least for now.
> Both are nasty in their own ways but copying does have a smaller blast
> radius. So, copy for now?

Yes, copy for now, don't mess with RCU and the struct device lifetime,
that is not going to solve anything.

I'll put the "fix the lifetime rules in the block layer" on my todo
list, at the bottom :(

thanks,

greg k-h
Yufen Yu March 7, 2020, 9:13 a.m. UTC | #10
On 2020/3/7 0:25, Greg Kroah-Hartman wrote:
> On Wed, Mar 04, 2020 at 08:22:11PM -0500, Tejun Heo wrote:
>> Hello,
>>
>> On Wed, Mar 04, 2020 at 09:05:59PM +0100, Greg Kroah-Hartman wrote:
>>>> Lifetime rules in block layer are kinda nebulous. Some of it comes
>>>> from the fact that some objects are reused. Instead of the usual,
>>>> create-use-release, they get repurposed to be associated with
>>>> something else. When looking at such an object from some paths, we
>>>> don't necessarily have ownership of all of the members.
>>>
>>> That's horrid, it's not like block devices are on some "fast path" for
>>> tear-down, we should do it correctly.
>>
>> Yeah, it got retrofitted umpteenth times from the really early days. I
>> don't think much of it is intentionally designed to be this way.
>>
>>>>> backing_device_info?  Are these being destroyed/used so often that rcu
>>>>> really is the best solution and the existing reference counting doesn't
>>>>> work properly?
>>>>
>>>> It's more that there are entry points which can only ensure that just
>>>> the top level object is valid and the member objects might be going or
>>>> coming as we're looking at it.
>>>
>>> That's not ok, a "member object" can only be valid if you have a
>>> reference to it.  If you remove the object, you then drop the reference,
>>> shouldn't that be the correct thing to do?
>>
>> I mean, it depends. There are two layers of objects and the top level
>> object has two stacked lifetime rules. The "active" usage pins
>> everything as usual. The "shallower" usage only has full access to the
>> top level and when it reaches down into members it needs a different
>> mechanism to ensure its validity. Given a clean slate, I don't think
>> we'd go for this design for these objects but the usage isn't
>> fundamentally broken.
>>
>> Idk, for the problem at hand, the choice is between patching it up by
>> copying the name and RCU protecting ->dev access at least for now.
>> Both are nasty in their own ways but copying does have a smaller blast
>> radius. So, copy for now?
> 
> Yes, copy for now, don't mess with RCU and the struct device lifetime,
> that is not going to solve anything.

Okay, I will try to rework the fix by copying the name. Thanks so much for all
response and suggestion.

Thanks,
Yufen

Patch
diff mbox series

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 8c436abfaf14..00904611b8e4 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -4978,7 +4978,7 @@  bfq_set_next_ioprio_data(struct bfq_queue *bfqq, struct bfq_io_cq *bic)
 	ioprio_class = IOPRIO_PRIO_CLASS(bic->ioprio);
 	switch (ioprio_class) {
 	default:
-		dev_err(bfqq->bfqd->queue->backing_dev_info->dev,
+		dev_err(&bfqq->bfqd->queue->backing_dev_info->rcu_dev->dev,
 			"bfq: bad prio class %d\n", ioprio_class);
 		/* fall through */
 	case IOPRIO_CLASS_NONE:
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index a229b94d5390..7ab1af3925c5 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -494,9 +494,13 @@  static int blkcg_reset_stats(struct cgroup_subsys_state *css,
 
 const char *blkg_dev_name(struct blkcg_gq *blkg)
 {
+	struct bdi_rcu_device *rcu_dev;
+
+	rcu_dev = rcu_dereference(blkg->q->backing_dev_info->rcu_dev);
+
 	/* some drivers (floppy) instantiate a queue w/o disk registered */
-	if (blkg->q->backing_dev_info->dev)
-		return dev_name(blkg->q->backing_dev_info->dev);
+	if (rcu_dev)
+		return dev_name(&rcu_dev->dev);
 	return NULL;
 }
 
diff --git a/block/genhd.c b/block/genhd.c
index ff6268970ddc..bd406b0fb471 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -668,9 +668,9 @@  static void register_disk(struct device *parent, struct gendisk *disk,
 		kobject_uevent(&part_to_dev(part)->kobj, KOBJ_ADD);
 	disk_part_iter_exit(&piter);
 
-	if (disk->queue->backing_dev_info->dev) {
+	if (disk->queue->backing_dev_info->rcu_dev) {
 		err = sysfs_create_link(&ddev->kobj,
-			  &disk->queue->backing_dev_info->dev->kobj,
+			  &disk->queue->backing_dev_info->rcu_dev->dev.kobj,
 			  "bdi");
 		WARN_ON(err);
 	}
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index ff1b764b0c0e..7781773f9f77 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -388,7 +388,7 @@  static int block_device_ejected(struct super_block *sb)
 	struct inode *bd_inode = sb->s_bdev->bd_inode;
 	struct backing_dev_info *bdi = inode_to_bdi(bd_inode);
 
-	return bdi->dev == NULL;
+	return bdi->rcu_dev == NULL;
 }
 
 static void ext4_journal_commit_callback(journal_t *journal, transaction_t *txn)
diff --git a/include/linux/backing-dev-defs.h b/include/linux/backing-dev-defs.h
index 4fc87dee005a..3d2294c428c1 100644
--- a/include/linux/backing-dev-defs.h
+++ b/include/linux/backing-dev-defs.h
@@ -13,6 +13,7 @@ 
 #include <linux/workqueue.h>
 #include <linux/kref.h>
 #include <linux/refcount.h>
+#include <linux/device.h>
 
 struct page;
 struct device;
@@ -185,6 +186,11 @@  struct bdi_writeback {
 #endif
 };
 
+struct bdi_rcu_device {
+	struct device dev;
+	struct rcu_head rcu_head;
+};
+
 struct backing_dev_info {
 	u64 id;
 	struct rb_node rb_node; /* keyed by ->id */
@@ -219,7 +225,7 @@  struct backing_dev_info {
 #endif
 	wait_queue_head_t wb_waitq;
 
-	struct device *dev;
+	struct bdi_rcu_device *rcu_dev;
 	struct device *owner;
 
 	struct timer_list laptop_mode_wb_timer;
diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index f88197c1ffc2..67e429b203a1 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -509,9 +509,9 @@  extern const char *bdi_unknown_name;
 
 static inline const char *bdi_dev_name(struct backing_dev_info *bdi)
 {
-	if (!bdi || !bdi->dev)
+	if (!bdi || !bdi->rcu_dev)
 		return bdi_unknown_name;
-	return dev_name(bdi->dev);
+	return dev_name(&bdi->rcu_dev->dev);
 }
 
 #endif	/* _LINUX_BACKING_DEV_H */
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 62f05f605fb5..b29c0ad43477 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -850,7 +850,7 @@  static int bdi_init(struct backing_dev_info *bdi)
 {
 	int ret;
 
-	bdi->dev = NULL;
+	bdi->rcu_dev = NULL;
 
 	kref_init(&bdi->refcnt);
 	bdi->min_ratio = 0;
@@ -930,20 +930,44 @@  struct backing_dev_info *bdi_get_by_id(u64 id)
 	return bdi;
 }
 
+static void bdi_device_release(struct device *dev)
+{
+	struct bdi_rcu_device *rcu_dev = container_of(dev,
+			struct bdi_rcu_device, dev);
+	kfree(rcu_dev);
+}
+
 int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
 {
 	struct device *dev;
 	struct rb_node *parent, **p;
+	struct bdi_rcu_device *rcu_dev;
+	int retval = -ENODEV;
 
-	if (bdi->dev)	/* The driver needs to use separate queues per device */
+	/* The driver needs to use separate queues per device */
+	if (bdi->rcu_dev)
 		return 0;
 
-	dev = device_create_vargs(bdi_class, NULL, MKDEV(0, 0), bdi, fmt, args);
-	if (IS_ERR(dev))
-		return PTR_ERR(dev);
+	rcu_dev = kzalloc(sizeof(struct bdi_rcu_device), GFP_KERNEL);
+	if (!rcu_dev)
+		return -ENOMEM;
+
+	/* initialize device */
+	dev = &rcu_dev->dev;
+	device_initialize(dev);
+	dev->class = bdi_class;
+	dev->release = bdi_device_release;
+	dev_set_drvdata(dev, (void *)bdi);
+	retval = kobject_set_name_vargs(&dev->kobj, fmt, args);
+	if (retval)
+		goto error;
+
+	retval = device_add(dev);
+	if (retval)
+		goto error;
 
 	cgwb_bdi_register(bdi);
-	bdi->dev = dev;
+	bdi->rcu_dev = rcu_dev;
 
 	bdi_debug_register(bdi, dev_name(dev));
 	set_bit(WB_registered, &bdi->wb.state);
@@ -962,6 +986,10 @@  int bdi_register_va(struct backing_dev_info *bdi, const char *fmt, va_list args)
 
 	trace_writeback_bdi_register(bdi);
 	return 0;
+
+error:
+	put_device(&rcu_dev->dev);
+	return retval;
 }
 EXPORT_SYMBOL(bdi_register_va);
 
@@ -1005,17 +1033,28 @@  static void bdi_remove_from_list(struct backing_dev_info *bdi)
 	synchronize_rcu_expedited();
 }
 
+static void bdi_put_device_rcu(struct rcu_head *rcu)
+{
+	struct bdi_rcu_device *rcu_dev = container_of(rcu,
+			struct bdi_rcu_device, rcu_head);
+	put_device(&rcu_dev->dev);
+}
+
 void bdi_unregister(struct backing_dev_info *bdi)
 {
+	struct bdi_rcu_device *rcu_dev = bdi->rcu_dev;
+
 	/* make sure nobody finds us on the bdi_list anymore */
 	bdi_remove_from_list(bdi);
 	wb_shutdown(&bdi->wb);
 	cgwb_bdi_unregister(bdi);
 
-	if (bdi->dev) {
+	if (rcu_dev) {
 		bdi_debug_unregister(bdi);
-		device_unregister(bdi->dev);
-		bdi->dev = NULL;
+		get_device(&rcu_dev->dev);
+		device_unregister(&rcu_dev->dev);
+		rcu_assign_pointer(bdi->rcu_dev, NULL);
+		call_rcu(&rcu_dev->rcu_head, bdi_put_device_rcu);
 	}
 
 	if (bdi->owner) {
@@ -1031,7 +1070,7 @@  static void release_bdi(struct kref *ref)
 
 	if (test_bit(WB_registered, &bdi->wb.state))
 		bdi_unregister(bdi);
-	WARN_ON_ONCE(bdi->dev);
+	WARN_ON_ONCE(bdi->rcu_dev);
 	wb_exit(&bdi->wb);
 	cgwb_bdi_exit(bdi);
 	kfree(bdi);