nbd: fix crash when unmaping nbd device with fs still mounted
Subsystems (2)
cover nbd: fix crash when unmaping nbd device with fs still mounted
Hi all,
I run into a BUG_ON(!buffer_mapped(bh)) crash with below script.
$ rbd-nbd map mypool/myimg $ mkfs.ext4 /dev/nbd0 $ mount /dev/nbd0 /mnt/ $ rbd-nbd unmap /dev/nbd0 $ umount /mnt
[ 1248.870131] kernel BUG at /home/mlin/linux/fs/buffer.c:3103! [ 1248.871214] invalid opcode: 0000 [#1] SMP [ 1248.879468] CPU: 0 PID: 2450 Comm: umount Tainted: G E 4.11.0-rc2+ #2 [ 1248.896579] Call Trace: [ 1248.897056] __sync_dirty_buffer+0x6e/0xe0 [ 1248.897870] ext4_commit_super+0x1eb/0x290 [ext4] [ 1248.898795] ext4_put_super+0x2fa/0x3c0 [ext4] [ 1248.899662] generic_shutdown_super+0x6f/0x100 [ 1248.900525] kill_block_super+0x27/0x70 [ 1248.901257] deactivate_locked_super+0x43/0x70 [ 1248.902112] deactivate_super+0x46/0x60 [ 1248.902869] cleanup_mnt+0x3f/0x80 [ 1248.903526] __cleanup_mnt+0x12/0x20 [ 1248.904218] task_work_run+0x83/0xb0 [ 1248.904941] exit_to_usermode_loop+0x59/0x7b [ 1248.905769] do_syscall_64+0x165/0x180 [ 1248.907603] entry_SYSCALL64_slow_path+0x25/0x25
Last year, Ratna posted a patch to fix it.
https://lkml.org/lkml/2016/4/20/257
Ratna's script to reproduce the bug.
$ qemu-img create -f qcow2 f.img 1G $ mkfs.ext4 f.img $ qemu-nbd -c /dev/nbd0 f.img $ mount /dev/nbd0 dir $ killall -KILL qemu-nbd $ sleep 1 $ ls dir $ umount dir
I ported Rantna's patch to 4.11-rc2 and confirmed that it fixes the crash.
Jan Kara had some comments about this bug:
http://www.kernelhub.org/?p=2&msg=361407
I hope to fix this bug in the upstream kernel first and then back port it to our production system.
Please see "PATCH 1/1" for detail.
Thanks,
Ming
RFC PATCH 1/1 nbd: replace kill_bdev() with __invalidate_device()
From: Ratna Manoj Bolla <[email protected]>
When a filesystem is mounted on a nbd device and on a disconnect, because of kill_bdev(), and resetting bdev size to zero, buffer_head mappings are getting destroyed under mounted filesystem.
After a bdev size reset(i.e bdev->bd_inode->i_size = 0) on a disconnect, followed by a sys_umount(),
generic_shutdown_super()->...
->__sync_blockdev()->...
-blkdev_writepages()->...
->do_invalidatepage()->...
-discard_buffer() is discarding superblock buffer_head assumedto be in mapped state by ext4_commit_super().
--- drivers/block/nbd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/block/nbd.c +6 -2
@@ -125,7 +125,8 @@ static const char *nbdcmd_to_ascii(int cmd)static int nbd_size_clear (struct nbd_device *nbd,struct block_device *bdev) { -bd_set_size (bdev,0 ); +if (bdev->bd_openers <=1 ) +bd_set_size (bdev,0 );set_capacity (nbd->disk,0 );kobject_uevent (&nbd_to_dev (nbd)->kobj,KOBJ_CHANGE );
@@ -603,6 +604,8 @@ static void nbd_reset(struct nbd_device *nbd)static void nbd_bdev_reset (struct block_device *bdev) { +if (bdev->bd_openers >1 ) +return ;set_device_ro (bdev, false); bdev->bd_inode->i_size =0 ;if (max_part >0 ) {
@@ -666,7 +669,8 @@ static int nbd_clear_sock(struct nbd_device *nbd, struct block_device *bdev) {sock_shutdown (nbd);nbd_clear_que (nbd); -kill_bdev (bdev); + +__invalidate_device (bdev, true);nbd_bdev_reset (bdev);/* * We want to give the run thread a chance to wait for everybody
Signature · 1 line
1.8.3.1
Re: [Nbd] [RFC PATCH 1/1] nbd: replace kill_bdev() with __invalidate_device()
Quoted 55 lines · Ming Lin
From: Ratna Manoj Bolla <[email protected]>
When a filesystem is mounted on a nbd device and on a disconnect, because of
kill_bdev(), and resetting bdev size to zero,buffer_headmappings are getting destroyed under mounted filesystem.After a bdev size
reset(i.e bdev->bd_inode->i_size = 0)on a disconnect, followed by asys_umount(),generic_shutdown_super()->... ->__sync_blockdev()->... -blkdev_writepages()->... ->do_invalidatepage()->... -discard_buffer() is discarding superblock buffer_head assumedto be in mapped state by
ext4_commit_super().[mlin: ported to 4.11-rc2]Signed-off-by Ratna Manoj Bolla <[email protected]--- drivers/block/nbd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)drivers/block/nbd.c +6
-3@@ -125,7 +125,8 @@ static const char *nbdcmd_to_ascii(int cmd)static int nbd_size_clear (struct nbd_device *nbd,struct block_device *bdev) { -bd_set_size (bdev,0 ); +if (bdev->bd_openers <=1 ) +bd_set_size (bdev,0 );set_capacity (nbd->disk,0 );kobject_uevent (&nbd_to_dev (nbd)->kobj,KOBJ_CHANGE );@@ -603,6 +604,8 @@ static void nbd_reset(struct nbd_device *nbd)static void nbd_bdev_reset (struct block_device *bdev) { +if (bdev->bd_openers >1 ) +return ;set_device_ro (bdev, false); bdev->bd_inode->i_size =0 ;if (max_part >0 ) {@@ -666,7 +669,8 @@ static int nbd_clear_sock(struct nbd_device *nbd, struct block_device *bdev) {sock_shutdown (nbd);nbd_clear_que (nbd); -kill_bdev (bdev); + +__invalidate_device (bdev, true);nbd_bdev_reset (bdev);/* * We want to give the run thread a chance to wait for everybody --
Quoted 1 line
1.8.3.1
Hi Josef,
Any comments?
Thanks,
Ming
Re: [Nbd] [RFC PATCH 1/1] nbd: replace kill_bdev() with __invalidate_device()
SGV5IHNvcnJ5IEkganVzdCBnb3QgYmFjayBmcm9tIExTRiwgSeKAmWxsIGxvb2sgYXQgdGhpcyBp biB0aGUgbW9ybmluZy4gIFRoYW5rcywNCg0KSm9zZWYNCg0KT24gMy8yMi8xNywgNDo0OCBQTSwg Ik1pbmcgTGluIiA8bWxpbkBrZXJuZWwub3JnPiB3cm90ZToNCg0KT24gTW9uLCBNYXIgMjAsIDIw MTcgYXQgMzo1OCBQTSwgTWluZyBMaW4gPG1saW5Aa2VybmVsLm9yZz4gd3JvdGU6DQo+IEZyb206 IFJhdG5hIE1hbm9qIEJvbGxhIDxtYW5vai5ickBnbWFpbC5jb20+DQo+DQo+IFdoZW4gYSBmaWxl c3lzdGVtIGlzIG1vdW50ZWQgb24gYSBuYmQgZGV2aWNlIGFuZCBvbiBhIGRpc2Nvbm5lY3QsIGJl Y2F1c2UNCj4gb2Yga2lsbF9iZGV2KCksIGFuZCByZXNldHRpbmcgYmRldiBzaXplIHRvIHplcm8s IGJ1ZmZlcl9oZWFkIG1hcHBpbmdzIGFyZQ0KPiBnZXR0aW5nIGRlc3Ryb3llZCB1bmRlciBtb3Vu dGVkIGZpbGVzeXN0ZW0uDQo+DQo+IEFmdGVyIGEgYmRldiBzaXplIHJlc2V0KGkuZSBiZGV2LT5i ZF9pbm9kZS0+aV9zaXplID0gMCkgb24gYSBkaXNjb25uZWN0LA0KPiBmb2xsb3dlZCBieSBhIHN5 c191bW91bnQoKSwNCj4gICAgICAgICBnZW5lcmljX3NodXRkb3duX3N1cGVyKCktPi4uLg0KPiAg ICAgICAgIC0+X19zeW5jX2Jsb2NrZGV2KCktPi4uLg0KPiAgICAgICAgIC1ibGtkZXZfd3JpdGVw YWdlcygpLT4uLi4NCj4gICAgICAgICAtPmRvX2ludmFsaWRhdGVwYWdlKCktPi4uLg0KPiAgICAg ICAgIC1kaXNjYXJkX2J1ZmZlcigpICAgaXMgZGlzY2FyZGluZyBzdXBlcmJsb2NrIGJ1ZmZlcl9o ZWFkIGFzc3VtZWQNCj4gdG8gYmUgaW4gbWFwcGVkIHN0YXRlIGJ5IGV4dDRfY29tbWl0X3N1cGVy KCkuDQo+DQo+IFttbGluOiBwb3J0ZWQgdG8gNC4xMS1yYzJdDQo+IFNpZ25lZC1vZmYtYnk6IFJh dG5hIE1hbm9qIEJvbGxhIDxtYW5vai5ickBnbWFpbC5jb20NCj4gLS0tDQo+ICBkcml2ZXJzL2Js b2NrL25iZC5jIHwgOCArKysrKystLQ0KPiAgMSBmaWxlIGNoYW5nZWQsIDYgaW5zZXJ0aW9ucygr KSwgMiBkZWxldGlvbnMoLSkNCj4NCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvYmxvY2svbmJkLmMg Yi9kcml2ZXJzL2Jsb2NrL25iZC5jDQo+IGluZGV4IGNiNGNjZmMuLmE2YTM2NDMgMTAwNjQ0DQo+ IC0tLSBhL2RyaXZlcnMvYmxvY2svbmJkLmMNCj4gKysrIGIvZHJpdmVycy9ibG9jay9uYmQuYw0K PiBAQCAtMTI1LDcgKzEyNSw4IEBAIHN0YXRpYyBjb25zdCBjaGFyICpuYmRjbWRfdG9fYXNjaWko aW50IGNtZCkNCj4NCj4gIHN0YXRpYyBpbnQgbmJkX3NpemVfY2xlYXIoc3RydWN0IG5iZF9kZXZp Y2UgKm5iZCwgc3RydWN0IGJsb2NrX2RldmljZSAqYmRldikNCj4gIHsNCj4gLSAgICAgICBiZF9z ZXRfc2l6ZShiZGV2LCAwKTsNCj4gKyAgICAgICBpZiAoYmRldi0+YmRfb3BlbmVycyA8PSAxKQ0K PiArICAgICAgICAgICAgICAgYmRfc2V0X3NpemUoYmRldiwgMCk7DQo+ICAgICAgICAgc2V0X2Nh cGFjaXR5KG5iZC0+ZGlzaywgMCk7DQo+ICAgICAgICAga29iamVjdF91ZXZlbnQoJm5iZF90b19k ZXYobmJkKS0+a29iaiwgS09CSl9DSEFOR0UpOw0KPg0KPiBAQCAtNjAzLDYgKzYwNCw4IEBAIHN0 YXRpYyB2b2lkIG5iZF9yZXNldChzdHJ1Y3QgbmJkX2RldmljZSAqbmJkKQ0KPg0KPiAgc3RhdGlj IHZvaWQgbmJkX2JkZXZfcmVzZXQoc3RydWN0IGJsb2NrX2RldmljZSAqYmRldikNCj4gIHsNCj4g KyAgICAgICBpZiAoYmRldi0+YmRfb3BlbmVycyA+IDEpDQo+ICsgICAgICAgICAgICAgICByZXR1 cm47DQo+ICAgICAgICAgc2V0X2RldmljZV9ybyhiZGV2LCBmYWxzZSk7DQo+ICAgICAgICAgYmRl di0+YmRfaW5vZGUtPmlfc2l6ZSA9IDA7DQo+ICAgICAgICAgaWYgKG1heF9wYXJ0ID4gMCkgew0K PiBAQCAtNjY2LDcgKzY2OSw4IEBAIHN0YXRpYyBpbnQgbmJkX2NsZWFyX3NvY2soc3RydWN0IG5i ZF9kZXZpY2UgKm5iZCwgc3RydWN0IGJsb2NrX2RldmljZSAqYmRldikNCj4gIHsNCj4gICAgICAg ICBzb2NrX3NodXRkb3duKG5iZCk7DQo+ICAgICAgICAgbmJkX2NsZWFyX3F1ZShuYmQpOw0KPiAt ICAgICAgIGtpbGxfYmRldihiZGV2KTsNCj4gKw0KPiArICAgICAgIF9faW52YWxpZGF0ZV9kZXZp Y2UoYmRldiwgdHJ1ZSk7DQo+ICAgICAgICAgbmJkX2JkZXZfcmVzZXQoYmRldik7DQo+ICAgICAg ICAgLyoNCj4gICAgICAgICAgKiBXZSB3YW50IHRvIGdpdmUgdGhlIHJ1biB0aHJlYWQgYSBjaGFu Y2UgdG8gd2FpdCBmb3IgZXZlcnlib2R5DQo+IC0tDQo+IDEuOC4zLjENCg0KSGkgSm9zZWYsDQoN CkFueSBjb21tZW50cz8NCg0KVGhhbmtzLA0KTWluZw0KDQoNCg==
Re: [RFC PATCH 1/1] nbd: replace kill_bdev() with __invalidate_device()
In reply to Ming LinWWVhaCBJIHRoaW5rIHRoaXMgaXMgb2ssIEnigJlsbCB0aHJvdyBpdCBvbiBteSBxdWV1ZSBmb3Ig Zml4ZXMgZm9yIHRoaXMgY3ljbGUuICBUaGFua3MsDQoNCkpvc2VmDQoNCk9uIDMvMjAvMTcsIDY6 NTggUE0sICJNaW5nIExpbiIgPG1saW5Aa2VybmVsLm9yZz4gd3JvdGU6DQoNCkZyb206IFJhdG5h IE1hbm9qIEJvbGxhIDxtYW5vai5ickBnbWFpbC5jb20+DQoNCldoZW4gYSBmaWxlc3lzdGVtIGlz IG1vdW50ZWQgb24gYSBuYmQgZGV2aWNlIGFuZCBvbiBhIGRpc2Nvbm5lY3QsIGJlY2F1c2UNCm9m IGtpbGxfYmRldigpLCBhbmQgcmVzZXR0aW5nIGJkZXYgc2l6ZSB0byB6ZXJvLCBidWZmZXJfaGVh ZCBtYXBwaW5ncyBhcmUNCmdldHRpbmcgZGVzdHJveWVkIHVuZGVyIG1vdW50ZWQgZmlsZXN5c3Rl bS4NCg0KQWZ0ZXIgYSBiZGV2IHNpemUgcmVzZXQoaS5lIGJkZXYtPmJkX2lub2RlLT5pX3NpemUg PSAwKSBvbiBhIGRpc2Nvbm5lY3QsDQpmb2xsb3dlZCBieSBhIHN5c191bW91bnQoKSwNCiAgICAg ICAgZ2VuZXJpY19zaHV0ZG93bl9zdXBlcigpLT4uLi4NCiAgICAgICAgLT5fX3N5bmNfYmxvY2tk ZXYoKS0+Li4uDQogICAgICAgIC1ibGtkZXZfd3JpdGVwYWdlcygpLT4uLi4NCiAgICAgICAgLT5k b19pbnZhbGlkYXRlcGFnZSgpLT4uLi4NCiAgICAgICAgLWRpc2NhcmRfYnVmZmVyKCkgICBpcyBk aXNjYXJkaW5nIHN1cGVyYmxvY2sgYnVmZmVyX2hlYWQgYXNzdW1lZA0KdG8gYmUgaW4gbWFwcGVk IHN0YXRlIGJ5IGV4dDRfY29tbWl0X3N1cGVyKCkuDQoNClttbGluOiBwb3J0ZWQgdG8gNC4xMS1y YzJdDQpTaWduZWQtb2ZmLWJ5OiBSYXRuYSBNYW5vaiBCb2xsYSA8bWFub2ouYnJAZ21haWwuY29t DQotLS0NCiBkcml2ZXJzL2Jsb2NrL25iZC5jIHwgOCArKysrKystLQ0KIDEgZmlsZSBjaGFuZ2Vk LCA2IGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pDQoNCmRpZmYgLS1naXQgYS9kcml2ZXJz L2Jsb2NrL25iZC5jIGIvZHJpdmVycy9ibG9jay9uYmQuYw0KaW5kZXggY2I0Y2NmYy4uYTZhMzY0 MyAxMDA2NDQNCi0tLSBhL2RyaXZlcnMvYmxvY2svbmJkLmMNCisrKyBiL2RyaXZlcnMvYmxvY2sv bmJkLmMNCkBAIC0xMjUsNyArMTI1LDggQEAgc3RhdGljIGNvbnN0IGNoYXIgKm5iZGNtZF90b19h c2NpaShpbnQgY21kKQ0KIA0KIHN0YXRpYyBpbnQgbmJkX3NpemVfY2xlYXIoc3RydWN0IG5iZF9k ZXZpY2UgKm5iZCwgc3RydWN0IGJsb2NrX2RldmljZSAqYmRldikNCiB7DQotCWJkX3NldF9zaXpl KGJkZXYsIDApOw0KKwlpZiAoYmRldi0+YmRfb3BlbmVycyA8PSAxKQ0KKwkJYmRfc2V0X3NpemUo YmRldiwgMCk7DQogCXNldF9jYXBhY2l0eShuYmQtPmRpc2ssIDApOw0KIAlrb2JqZWN0X3VldmVu dCgmbmJkX3RvX2RldihuYmQpLT5rb2JqLCBLT0JKX0NIQU5HRSk7DQogDQpAQCAtNjAzLDYgKzYw NCw4IEBAIHN0YXRpYyB2b2lkIG5iZF9yZXNldChzdHJ1Y3QgbmJkX2RldmljZSAqbmJkKQ0KIA0K IHN0YXRpYyB2b2lkIG5iZF9iZGV2X3Jlc2V0KHN0cnVjdCBibG9ja19kZXZpY2UgKmJkZXYpDQog ew0KKwlpZiAoYmRldi0+YmRfb3BlbmVycyA+IDEpDQorCQlyZXR1cm47DQogCXNldF9kZXZpY2Vf cm8oYmRldiwgZmFsc2UpOw0KIAliZGV2LT5iZF9pbm9kZS0+aV9zaXplID0gMDsNCiAJaWYgKG1h eF9wYXJ0ID4gMCkgew0KQEAgLTY2Niw3ICs2NjksOCBAQCBzdGF0aWMgaW50IG5iZF9jbGVhcl9z b2NrKHN0cnVjdCBuYmRfZGV2aWNlICpuYmQsIHN0cnVjdCBibG9ja19kZXZpY2UgKmJkZXYpDQog ew0KIAlzb2NrX3NodXRkb3duKG5iZCk7DQogCW5iZF9jbGVhcl9xdWUobmJkKTsNCi0Ja2lsbF9i ZGV2KGJkZXYpOw0KKw0KKwlfX2ludmFsaWRhdGVfZGV2aWNlKGJkZXYsIHRydWUpOw0KIAluYmRf YmRldl9yZXNldChiZGV2KTsNCiAJLyoNCiAJICogV2Ugd2FudCB0byBnaXZlIHRoZSBydW4gdGhy ZWFkIGEgY2hhbmNlIHRvIHdhaXQgZm9yIGV2ZXJ5Ym9keQ0KLS0gDQoxLjguMy4xDQoNCg0KDQo=
Re: [RFC PATCH 1/1] nbd: replace kill_bdev() with __invalidate_device()
Quoted 1 line · Josef Bacik
Yeah I think this is ok, I’ll throw it on my queue for fixes for this cycle. Thanks,
Great. Thanks.
Quoted 2 lines
Josef