btrfs regression since 4.X kernel NULL pointer dereference

btrfs regression since 4.X kernel NULL pointer dereference

[Stefan Priebe]

Hello,

today i experienced the following btrfs bug:

Aug 20 11:59:18 debian-build kernel: [  325.170036] BUG: unable to 
handle kernel NULL pointer dereference at 0000000000000330
Aug 20 11:59:18 debian-build kernel: [  325.170144] IP: 
[<ffffffff813204c0>] blk_get_backing_dev_info+0x10/0x20
Aug 20 11:59:18 debian-build kernel: [  325.170216] PGD 74f57067 PUD 
74f51067 PMD 0
Aug 20 11:59:18 debian-build kernel: [  325.170282] Oops: 0000 [#1] SMP
Aug 20 11:59:18 debian-build kernel: [  325.170330] Modules linked in: 
dm_mod netconsole xt_multiport iptable_filter ip_tables x_tab
les cpufreq_userspace cpufreq_stats cpufreq_powersave 
cpufreq_conservative ext2 loop shpchp i2c_piix4 i2c_core virtio_balloon 
acpi_c
pufreq button btrfs xor lzo_compress usbhid raid6_pq ata_generic sg 
sd_mod virtio_net virtio_scsi floppy uhci_hcd ehci_hcd ata_piix
usbcore usb_common virtio_pci
Aug 20 11:59:18 debian-build kernel: [  325.170783] CPU: 4 PID: 13323 
Comm: btrfs Not tainted 4.1.6+17-ph #1
Aug 20 11:59:18 debian-build kernel: [  325.170842] Hardware name: QEMU 
Standard PC (i440FX + PIIX, 1996), BIOS 
rel-1.8.1-0-g4adadbd-20150316_085822-nilsson.home.kraxel.org 04/01/2014
Aug 20 11:59:18 debian-build kernel: [  325.170952] task: 
ffff88022d6bbae0 ti: ffff8800748e0000 task.ti: ffff8800748e0000
Aug 20 11:59:18 debian-build kernel: [  325.171017] RIP: 
0010:[<ffffffff813204c0>]  [<ffffffff813204c0>] 
blk_get_backing_dev_info+0x10/0x20
Aug 20 11:59:18 debian-build kernel: [  325.171096] RSP: 
0018:ffff8800748e39a8  EFLAGS: 00010202
Aug 20 11:59:18 debian-build kernel: [  325.171148] RAX: 
0000000000000000 RBX: ffff880234680770 RCX: 0000000000000001
Aug 20 11:59:18 debian-build kernel: [  325.171210] RDX: 
7fffffffffffffff RSI: 0000000000000000 RDI: ffff880234680680
Aug 20 11:59:18 debian-build kernel: [  325.171271] RBP: 
ffff8800748e39a8 R08: 7fffffffffffffff R09: 0000000000000246
Aug 20 11:59:18 debian-build kernel: [  325.171333] R10: 
ffffffffa0158bdc R11: 0000000000000000 R12: ffff880237019000
Aug 20 11:59:18 debian-build kernel: [  325.171393] R13: 
7fffffffffffffff R14: ffff880092df07fc R15: 7fffffffffffffff
Aug 20 11:59:18 debian-build kernel: [  325.171455] FS: 
00007fb05f0ba880(0000) GS:ffff88023fd00000(0000) knlGS:0000000000000000
Aug 20 11:59:18 debian-build kernel: [  325.171522] CS:  0010 DS: 0000 
ES: 0000 CR0: 0000000080050033
Aug 20 11:59:18 debian-build kernel: [  325.171577] CR2: 
0000000000000330 CR3: 0000000074ce4000 CR4: 00000000000006e0
Aug 20 11:59:18 debian-build kernel: [  325.171669] Stack:
Aug 20 11:59:18 debian-build kernel: [  325.171706]  ffff8800748e39c8 
ffffffff811e6d60 ffff8802346808c0 0000000000000000
Aug 20 11:59:18 debian-build kernel: [  325.171811]  ffff8800748e3a18 
ffffffff8114e232 ffff880212f93910 7fffffffffffffff
Aug 20 11:59:18 debian-build kernel: [  325.171923]  0000000000000000 
0000000000000000 7fffffffffffffff 0000000000000001
Aug 20 11:59:18 debian-build kernel: [  325.172078] Call Trace:
Aug 20 11:59:18 debian-build kernel: [  325.172132] 
[<ffffffff811e6d60>] inode_to_bdi+0x60/0x70
Aug 20 11:59:18 debian-build kernel: [  325.172221] 
[<ffffffff8114e232>] __filemap_fdatawrite_range+0x42/0x70
Aug 20 11:59:18 debian-build kernel: [  325.172319] 
[<ffffffff8114eea3>] filemap_fdatawrite_range+0x13/0x20
Aug 20 11:59:18 debian-build kernel: [  325.172418] 
[<ffffffffa0157c2b>] btrfs_fdatawrite_range+0x2b/0x70 [btrfs]
Aug 20 11:59:18 debian-build kernel: [  325.172493] 
[<ffffffffa015d57c>] btrfs_wait_ordered_range+0x4c/0x130 [btrfs]
Aug 20 11:59:18 debian-build kernel: [  325.174258] 
[<ffffffffa0155075>] ? btrfs_drop_extent_cache+0x355/0x420 [btrfs]
Aug 20 11:59:18 debian-build kernel: [  325.175688] 
[<ffffffffa014dde6>] btrfs_evict_inode+0x226/0x550 [btrfs]
Aug 20 11:59:18 debian-build kernel: [  325.177252] 
[<ffffffff811e726d>] ? __inode_wait_for_writeback+0x6d/0xc0
Aug 20 11:59:18 debian-build kernel: [  325.179214] 
[<ffffffff811d9058>] evict+0xb8/0x190
Aug 20 11:59:18 debian-build kernel: [  325.180619] 
[<ffffffff811d986b>] iput+0x18b/0x1f0
Aug 20 11:59:18 debian-build kernel: [  325.182034] 
[<ffffffff811d4f28>] __dentry_kill+0x198/0x200
Aug 20 11:59:18 debian-build kernel: [  325.183559] 
[<ffffffff811d50ad>] shrink_dentry_list+0x11d/0x2b0
Aug 20 11:59:18 debian-build kernel: [  325.184981] 
[<ffffffff811d56c8>] d_invalidate+0xd8/0x100
Aug 20 11:59:18 debian-build kernel: [  325.186394] 
[<ffffffffa017757b>] btrfs_ioctl_snap_destroy+0x50b/0x6e0 [btrfs]
Aug 20 11:59:18 debian-build kernel: [  325.187832] 
[<ffffffffa017abca>] btrfs_ioctl+0x131a/0x2a30 [btrfs]
Aug 20 11:59:18 debian-build kernel: [  325.189239] 
[<ffffffff8115ab2b>] ? lru_cache_add_active_or_unevictable+0x2b/0xa0
Aug 20 11:59:18 debian-build kernel: [  325.190668] 
[<ffffffff8117970a>] ? handle_mm_fault+0x2ba/0x1860
Aug 20 11:59:18 debian-build kernel: [  325.192062] 
[<ffffffff81181566>] ? mmap_region+0x316/0x630
Aug 20 11:59:18 debian-build kernel: [  325.193453] 
[<ffffffff81116ecc>] ? acct_account_cputime+0x1c/0x20
Aug 20 11:59:18 debian-build kernel: [  325.194851] 
[<ffffffff810ae3f9>] ? account_user_time+0x99/0xb0
Aug 20 11:59:18 debian-build kernel: [  325.196241] 
[<ffffffff811d0bd3>] do_vfs_ioctl+0x83/0x550
Aug 20 11:59:18 debian-build kernel: [  325.197584] 
[<ffffffff8114be23>] ? context_tracking_user_exit+0x13/0x20
Aug 20 11:59:18 debian-build kernel: [  325.198913] 
[<ffffffff81012558>] ? syscall_trace_enter_phase1+0xf8/0x160
Aug 20 11:59:18 debian-build kernel: [  325.200229] 
[<ffffffff811d10ec>] SyS_ioctl+0x4c/0x90
Aug 20 11:59:18 debian-build kernel: [  325.201548] 
[<ffffffff8163442e>] system_call_fastpath+0x12/0x71
Aug 20 11:59:18 debian-build kernel: [  325.202836] Code: e9 23 ff ff ff 
b8 01 00 00 00 45 31 e4 eb d5 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 
44 00 00 48 8b 87 98 00 00 00 55 48 89 e5 <48> 8b 80 30 03 00 00 5d 48 
05 98 01 00 00 c3 90 0f 1f 44 00 00
Aug 20 11:59:18 debian-build kernel: [  325.205694] RIP 
[<ffffffff813204c0>] blk_get_backing_dev_info+0x10/0x20
Aug 20 11:59:18 debian-build kernel: [  325.206983]  RSP <ffff8800748e39a8>
Aug 20 11:59:18 debian-build kernel: [  325.208233] CR2: 0000000000000330
Aug 20 11:59:18 debian-build kernel: [  325.209467] ---[ end trace 
9dd28134a31aacc4 ]---

It was introduced by:
| commit de1414a654e66b81b5348dbc5259ecf2fb61655e
| Author: Christoph Hellwig <hch@lst.de>
| Date:   Wed Jan 14 10:42:36 2015 +0100
|
|     fs: export inode_to_bdi and use it in favor of 
mapping->backing_dev_info

More details and a reproducer from a 3rd person can be found here:
https://bugzilla.kernel.org/show_bug.cgi?id=100911

Greets,
Stefan

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Christoph Hellwig]

I think this is btrfs using a struct block_device that doesn't have
a valid queue pointer in it's gendisk for ->s_bdev.  And there are
some fishy looking ->s_bdev assignments in the code which I suspect
are related to it:

fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev == src_device->bdev)
fs/btrfs/dev-replace.c:         fs_info->sb->s_bdev = tgt_device->bdev;
fs/btrfs/volumes.c:     if (device->bdev == root->fs_info->sb->s_bdev)
fs/btrfs/volumes.c:             root->fs_info->sb->s_bdev = next_device->bdev;
fs/btrfs/volumes.c:     if (tgtdev->bdev == fs_info->sb->s_bdev)
fs/btrfs/volumes.c:             fs_info->sb->s_bdev = next_device->bdev;

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Stefan Priebe - Profihost AG]

Am 25.08.2015 um 11:00 schrieb Christoph Hellwig:
> I think this is btrfs using a struct block_device that doesn't have
> a valid queue pointer in it's gendisk for ->s_bdev.  And there are
> some fishy looking ->s_bdev assignments in the code which I suspect
> are related to it:
> 
> fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev == src_device->bdev)
> fs/btrfs/dev-replace.c:         fs_info->sb->s_bdev = tgt_device->bdev;
> fs/btrfs/volumes.c:     if (device->bdev == root->fs_info->sb->s_bdev)
> fs/btrfs/volumes.c:             root->fs_info->sb->s_bdev = next_device->bdev;
> fs/btrfs/volumes.c:     if (tgtdev->bdev == fs_info->sb->s_bdev)
> fs/btrfs/volumes.c:             fs_info->sb->s_bdev = next_device->bdev;
> 

Would be very nice if anyone from btrfs can pick this up.

Stefan

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Chris Mason]

On Tue, Aug 25, 2015 at 11:00:30AM +0200, Christoph Hellwig wrote:
> I think this is btrfs using a struct block_device that doesn't have
> a valid queue pointer in it's gendisk for ->s_bdev.  And there are
> some fishy looking ->s_bdev assignments in the code which I suspect
> are related to it:
> 
> fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev == src_device->bdev)
> fs/btrfs/dev-replace.c:         fs_info->sb->s_bdev = tgt_device->bdev;
> fs/btrfs/volumes.c:     if (device->bdev == root->fs_info->sb->s_bdev)
> fs/btrfs/volumes.c:             root->fs_info->sb->s_bdev = next_device->bdev;
> fs/btrfs/volumes.c:     if (tgtdev->bdev == fs_info->sb->s_bdev)
> fs/btrfs/volumes.c:             fs_info->sb->s_bdev = next_device->bdev;

We've had trouble with this in the past, I'll take a look.

-chris

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Stefan Priebe - Profihost AG]

> Am 25.08.2015 um 15:51 schrieb Chris Mason <clm@fb.com>:
> 
>> On Tue, Aug 25, 2015 at 11:00:30AM +0200, Christoph Hellwig wrote:
>> I think this is btrfs using a struct block_device that doesn't have
>> a valid queue pointer in it's gendisk for ->s_bdev.  And there are
>> some fishy looking ->s_bdev assignments in the code which I suspect
>> are related to it:
>> 
>> fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev == src_device->bdev)
>> fs/btrfs/dev-replace.c:         fs_info->sb->s_bdev = tgt_device->bdev;
>> fs/btrfs/volumes.c:     if (device->bdev == root->fs_info->sb->s_bdev)
>> fs/btrfs/volumes.c:             root->fs_info->sb->s_bdev = next_device->bdev;
>> fs/btrfs/volumes.c:     if (tgtdev->bdev == fs_info->sb->s_bdev)
>> fs/btrfs/volumes.c:             fs_info->sb->s_bdev = next_device->bdev;
> 
> We've had trouble with this in the past, I'll take a look.

Any news?

> 
> -chris
> 

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Chris Mason]

On Mon, Aug 31, 2015 at 07:32:09PM +0200, Stefan Priebe - Profihost AG wrote:
> > Am 25.08.2015 um 15:51 schrieb Chris Mason <clm@fb.com>:
> > 
> >> On Tue, Aug 25, 2015 at 11:00:30AM +0200, Christoph Hellwig wrote:
> >> I think this is btrfs using a struct block_device that doesn't have
> >> a valid queue pointer in it's gendisk for ->s_bdev.  And there are
> >> some fishy looking ->s_bdev assignments in the code which I suspect
> >> are related to it:
> >> 
> >> fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev == src_device->bdev)
> >> fs/btrfs/dev-replace.c:         fs_info->sb->s_bdev = tgt_device->bdev;
> >> fs/btrfs/volumes.c:     if (device->bdev == root->fs_info->sb->s_bdev)
> >> fs/btrfs/volumes.c:             root->fs_info->sb->s_bdev = next_device->bdev;
> >> fs/btrfs/volumes.c:     if (tgtdev->bdev == fs_info->sb->s_bdev)
> >> fs/btrfs/volumes.c:             fs_info->sb->s_bdev = next_device->bdev;
> > 
> > We've had trouble with this in the past, I'll take a look.
> 
> Any news?

Haven't been able to reproduce yet, I'll try again in the morning.

-chris

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Stefan Priebe]

Am 01.09.2015 um 02:06 schrieb Chris Mason:
> On Mon, Aug 31, 2015 at 07:32:09PM +0200, Stefan Priebe - Profihost AG wrote:
>>> Am 25.08.2015 um 15:51 schrieb Chris Mason <clm@fb.com>:
>>>
>>>> On Tue, Aug 25, 2015 at 11:00:30AM +0200, Christoph Hellwig wrote:
>>>> I think this is btrfs using a struct block_device that doesn't have
>>>> a valid queue pointer in it's gendisk for ->s_bdev.  And there are
>>>> some fishy looking ->s_bdev assignments in the code which I suspect
>>>> are related to it:
>>>>
>>>> fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev == src_device->bdev)
>>>> fs/btrfs/dev-replace.c:         fs_info->sb->s_bdev = tgt_device->bdev;
>>>> fs/btrfs/volumes.c:     if (device->bdev == root->fs_info->sb->s_bdev)
>>>> fs/btrfs/volumes.c:             root->fs_info->sb->s_bdev = next_device->bdev;
>>>> fs/btrfs/volumes.c:     if (tgtdev->bdev == fs_info->sb->s_bdev)
>>>> fs/btrfs/volumes.c:             fs_info->sb->s_bdev = next_device->bdev;
>>>
>>> We've had trouble with this in the past, I'll take a look.
>>
>> Any news?
>
> Haven't been able to reproduce yet, I'll try again in the morning.

Thanks. We're using schroot like the user in this bugreport:
https://bugzilla.kernel.org/show_bug.cgi?id=100911

But he also claims he found another way to reproduce using vfcgbackup 
(last comment).

Stefan

>
> -chris
>

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Christoph Biedl]

Stefan Priebe wrote...

> Thanks. We're using schroot like the user in this bugreport:
> https://bugzilla.kernel.org/show_bug.cgi?id=100911
> 
> But he also claims he found another way to reproduce using vfcgbackup (last
> comment).

FWIW, this was still the same thing, just stripped down to get closer
to the actual cause. It began with building a private Debian package
that has lvm2 as build dependency. Daemons are disabled in the build
chroot (via policy-rc.d) but the vfcgbackup invocation in lvm2's
postinst is still run.

Also, a quick test of Jeff's update to the Bugzilla ticket looks very
good. More tests will follow.

    Christoph

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 8/31/15 8:06 PM, Chris Mason wrote:
> On Mon, Aug 31, 2015 at 07:32:09PM +0200, Stefan Priebe -
> Profihost AG wrote:
>>> Am 25.08.2015 um 15:51 schrieb Chris Mason <clm@fb.com>:
>>> 
>>>> On Tue, Aug 25, 2015 at 11:00:30AM +0200, Christoph Hellwig 
>>>> wrote: I think this is btrfs using a struct block_device
>>>> that doesn't have a valid queue pointer in it's gendisk for 
>>>> ->s_bdev.  And there are some fishy looking ->s_bdev 
>>>> assignments in the code which I suspect are related to it:
>>>> 
>>>> fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev == 
>>>> src_device->bdev) fs/btrfs/dev-replace.c: fs_info->sb->s_bdev
>>>> = tgt_device->bdev; fs/btrfs/volumes.c: if (device->bdev ==
>>>> root->fs_info->sb->s_bdev) fs/btrfs/volumes.c:
>>>> root->fs_info->sb->s_bdev = next_device->bdev;
>>>> fs/btrfs/volumes.c:     if (tgtdev->bdev ==
>>>> fs_info->sb->s_bdev) fs/btrfs/volumes.c: fs_info->sb->s_bdev
>>>> = next_device->bdev;
>>> 
>>> We've had trouble with this in the past, I'll take a look.
>> 
>> Any news?
> 
> Haven't been able to reproduce yet, I'll try again in the morning.

I'm unable to reproduce it as well and I'm hearing about people
hitting it in different forums.

Can you describe your storage configuration?  There's a gendisk
without a queue sneaking in somehow.  There are some spots in dm where
the mddev gets a new queue assigned to it but the old queue associated
with the gendisk is left behind so that's not it.  If you're able to
reproduce it reliably with this configuration, that's certainly
interesting.  I'm wondering if the case is that vgcfgbackup is
iterating over devices and causing something that might not have been
previously scanned to be scanned and added to the btrfs device list.
Are there any messages above the oops indicating when a device is
added?  Could you post your complete dmesg somewhere?

Thanks,

- -Jeff

- -- 
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.19 (Darwin)

iQIcBAEBAgAGBQJV8gJ2AAoJEB57S2MheeWyEP4QALvIXZGIqkXhUBZEYvLC+rnD
Q79chLpbt8x52xtFoav78i3nG24SMXV5NbhFlGNnr6xt/CQtKSzBuDyJ+nigkyy+
bTtHbwkMqNddrqKJ3r928nKwz1IOyU4J58l7hGyC0owSA3mDxyI7+yS16OXFwc1S
6lxtV4ljf6nqobA+rVyC3n3BU6CF1aL45V2FpM/m4OHL/Xqv97Xg2tGlysnk8RPD
dRvz5fs2T75B0eBsK4xWHKEgKKWdVYaKqlySRXUJQpnNJTlJltW5HzL8RnR47brt
LLaOCD4jcLlEtGhTi6wEN6BlEYbgDx/1yjQs5zxLG0vbTUi5ZcTeT82d5+Hh4WMD
HyLfE4pwFUZXTHvcIoQ51wPLS/tWMVJXnleQKBrDT+WcsKPr5yuwUZpwfrOBfjol
Qan13/mIcmvFsz2yVWnoZJWU5osnhe+frBsVFlIFLNXp50QVXk+WvruPDOsoY2eQ
J8L20LhY5e0dgJseXWkAAy68mJ0/rrkX0iAdiED/WEsBdjy7QuOomJqPyQQEPplm
d+tIkDR77Biajket2tNEXK1m14f7QvFUFojI0NwegcmZT3iJ7BBWD6wjxIvh3KqE
LqHIcTaECU9XSg9x9E3N416hCbclC9xeosxaJB1iN+EvpaD7CBA+V+onkNMN6Fu4
J0aVUYlZuLcqkEs/BuNL
=BRtR
-----END PGP SIGNATURE-----

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Stefan Priebe]

Am 11.09.2015 um 00:21 schrieb Jeff Mahoney:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 8/31/15 8:06 PM, Chris Mason wrote:
>> On Mon, Aug 31, 2015 at 07:32:09PM +0200, Stefan Priebe -
>> Profihost AG wrote:
>>>> Am 25.08.2015 um 15:51 schrieb Chris Mason <clm@fb.com>:
>>>>
>>>>> On Tue, Aug 25, 2015 at 11:00:30AM +0200, Christoph Hellwig
>>>>> wrote: I think this is btrfs using a struct block_device
>>>>> that doesn't have a valid queue pointer in it's gendisk for
>>>>> ->s_bdev.  And there are some fishy looking ->s_bdev
>>>>> assignments in the code which I suspect are related to it:
>>>>>
>>>>> fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev ==
>>>>> src_device->bdev) fs/btrfs/dev-replace.c: fs_info->sb->s_bdev
>>>>> = tgt_device->bdev; fs/btrfs/volumes.c: if (device->bdev ==
>>>>> root->fs_info->sb->s_bdev) fs/btrfs/volumes.c:
>>>>> root->fs_info->sb->s_bdev = next_device->bdev;
>>>>> fs/btrfs/volumes.c:     if (tgtdev->bdev ==
>>>>> fs_info->sb->s_bdev) fs/btrfs/volumes.c: fs_info->sb->s_bdev
>>>>> = next_device->bdev;
>>>>
>>>> We've had trouble with this in the past, I'll take a look.
>>>
>>> Any news?
>>
>> Haven't been able to reproduce yet, I'll try again in the morning.
>
> I'm unable to reproduce it as well and I'm hearing about people
> hitting it in different forums.
>
> Can you describe your storage configuration?   There's a gendisk
> without a queue sneaking in somehow.  There are some spots in dm where
> the mddev gets a new queue assigned to it but the old queue associated
> with the gendisk is left behind so that's not it.

I'm hitting this inside a debian buil VM using schroot. So the disk is a 
simple plain qemu emulated scsi disk (virtio-pci-scsi).

 >  If you're able to
> reproduce it reliably with this configuration, that's certainly
> interesting.  I'm wondering if the case is that vgcfgbackup is
> iterating over devices and causing something that might not have been
> previously scanned to be scanned and added to the btrfs device list.

I'm not the guy from the kernel bug report - so i'm only using debian 
schroot not vgcfgbackup.

I can trigger it 100% by using schroot with sbuild using debian or ubuntu.

> Are there any messages above the oops indicating when a device is
> added?  Could you post your complete dmesg somewhere?

Sure:
http://pastebin.com/raw.php?i=bG6bZUPA

Thanks!

Greets,
Stefan

> Thanks,
>
> - -Jeff
>
> - --
> Jeff Mahoney
> SUSE Labs
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.19 (Darwin)
>
> iQIcBAEBAgAGBQJV8gJ2AAoJEB57S2MheeWyEP4QALvIXZGIqkXhUBZEYvLC+rnD
> Q79chLpbt8x52xtFoav78i3nG24SMXV5NbhFlGNnr6xt/CQtKSzBuDyJ+nigkyy+
> bTtHbwkMqNddrqKJ3r928nKwz1IOyU4J58l7hGyC0owSA3mDxyI7+yS16OXFwc1S
> 6lxtV4ljf6nqobA+rVyC3n3BU6CF1aL45V2FpM/m4OHL/Xqv97Xg2tGlysnk8RPD
> dRvz5fs2T75B0eBsK4xWHKEgKKWdVYaKqlySRXUJQpnNJTlJltW5HzL8RnR47brt
> LLaOCD4jcLlEtGhTi6wEN6BlEYbgDx/1yjQs5zxLG0vbTUi5ZcTeT82d5+Hh4WMD
> HyLfE4pwFUZXTHvcIoQ51wPLS/tWMVJXnleQKBrDT+WcsKPr5yuwUZpwfrOBfjol
> Qan13/mIcmvFsz2yVWnoZJWU5osnhe+frBsVFlIFLNXp50QVXk+WvruPDOsoY2eQ
> J8L20LhY5e0dgJseXWkAAy68mJ0/rrkX0iAdiED/WEsBdjy7QuOomJqPyQQEPplm
> d+tIkDR77Biajket2tNEXK1m14f7QvFUFojI0NwegcmZT3iJ7BBWD6wjxIvh3KqE
> LqHIcTaECU9XSg9x9E3N416hCbclC9xeosxaJB1iN+EvpaD7CBA+V+onkNMN6Fu4
> J0aVUYlZuLcqkEs/BuNL
> =BRtR
> -----END PGP SIGNATURE-----
>

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 8/25/15 5:00 AM, Christoph Hellwig wrote:
> I think this is btrfs using a struct block_device that doesn't
> have a valid queue pointer in it's gendisk for ->s_bdev.  And there
> are some fishy looking ->s_bdev assignments in the code which I
> suspect are related to it:
> 
> fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev ==
> src_device->bdev) fs/btrfs/dev-replace.c:
> fs_info->sb->s_bdev = tgt_device->bdev; fs/btrfs/volumes.c:     if
> (device->bdev == root->fs_info->sb->s_bdev) fs/btrfs/volumes.c:
> root->fs_info->sb->s_bdev = next_device->bdev; fs/btrfs/volumes.c:
> if (tgtdev->bdev == fs_info->sb->s_bdev) fs/btrfs/volumes.c:
> fs_info->sb->s_bdev = next_device->bdev;

The report at https://bugzilla.kernel.org/show_bug.cgi?id=100911
tracks it down a bit further and it's bdev->bd_disk == NULL instead of
the queue in the gendisk. I don't think that the s_bdev stuff is
related, though I'd certainly love to see that bit go away.

If we're calling blk_get_backing_dev_info, that means we're already
using an inode that has blockdev_superblock and the btrfs superblock
isn't even involved.

We're getting there because btrfs_evict_inode ->
btrfs_wait_ordered_range -> btrfs_fdatawrite_range ->
filemap_fdatawrite_range gets called with inode->i_mapping.  That
mapping gets passed down through __filemap_fdatawrite_range to
wbc_attach_fdatawrite_inode where the inode passed is mapping->host --
which will be the block device inode rather than the btrfs device node
inode.  That inode is the one ultimately checked in inode_to_bdi.

So it looks like we're causing writeback on an unrelated block device
that was opened using a device node hosted on btrfs, which is
obviously wrong.

I don't think snapshot removal is even a requirement to trigger this.
 I expect it's possible to trigger with two device nodes for the same
block device where one is getting closed and cleaned up while the
eviction of the other happens.  The device nodes wouldn't even need to
be on the same fs.

Other file systems use &inode->i_data in eviction.  Is it that simple
here?

- -Jeff

- -- 
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.19 (Darwin)

iQIcBAEBAgAGBQJV8yOVAAoJEB57S2MheeWysvMP/0cIPCytKGzQqkNpzjfcBk4b
a4s3xM3xnxZ0BayvAWIpSrCLp/5OR5N30Eu326LNZKIEnC7jbkQHePFLIftnhtJ/
eGWlFe9kOsHGWtdA2HyZO9s6V/Nnh0t7vXKUBfqTjV71T66VL/FP9cfRVJ4Ov5Zb
99dK58glhDuF0tOQhePdfaqw4zym+3YHkD+CJjTUKO9YnpTgr4CQFJ+6v6itGbIt
QRY7qVY0S1nz0w/s8AsKu2g76thILtBvmwsEMik3TYSJI5gHxLgSpR0btk64o67+
N50AGsO/TMJs6u9p8Ad4zMFF8AfylAgTV3g8uH6v2QLI3ILVMhjtqgOwWlT78Aca
dmceWAfhBAdRizYqKQC6ZKq26Qf9GTSEoM0L/3TuBqN5scKtGYx0mvoDzj080i7p
nmPJ955pWwxa2tsmo8wRoPXVjvOXegIyguyHvqTg0wrwzfm4aPtZGTtr7RU65lp2
83fl2KJXan8V1vkOwmZ9n4e1G1g8Gggb+qCMAiv9cLWkfTus2HFdh5GNEZ+jSCJ1
2+QzIjFzLqx0N3wQmneBfkdDiWpQkAbQJjJLPdJykivo4WytV/6Vtvcqbv39JCJj
1awM2EpqB9rKV24BGDH86MiErvVT3HBLjSEEpIa41T8PlBXEsQOH1hsXTZSzyP9o
iO8qclZgSIIUgiN4feV3
=xPuq
-----END PGP SIGNATURE-----

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Jeff Mahoney]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 9/11/15 2:55 PM, Jeff Mahoney wrote:
> On 8/25/15 5:00 AM, Christoph Hellwig wrote:
>> I think this is btrfs using a struct block_device that doesn't 
>> have a valid queue pointer in it's gendisk for ->s_bdev.  And
>> there are some fishy looking ->s_bdev assignments in the code
>> which I suspect are related to it:
> 
>> fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev == 
>> src_device->bdev) fs/btrfs/dev-replace.c: fs_info->sb->s_bdev =
>> tgt_device->bdev; fs/btrfs/volumes.c:     if (device->bdev ==
>> root->fs_info->sb->s_bdev) fs/btrfs/volumes.c: 
>> root->fs_info->sb->s_bdev = next_device->bdev;
>> fs/btrfs/volumes.c: if (tgtdev->bdev == fs_info->sb->s_bdev)
>> fs/btrfs/volumes.c: fs_info->sb->s_bdev = next_device->bdev;
> 
> The report at https://bugzilla.kernel.org/show_bug.cgi?id=100911 
> tracks it down a bit further and it's bdev->bd_disk == NULL instead
> of the queue in the gendisk. I don't think that the s_bdev stuff
> is related, though I'd certainly love to see that bit go away.
> 
> If we're calling blk_get_backing_dev_info, that means we're
> already using an inode that has blockdev_superblock and the btrfs
> superblock isn't even involved.
> 
> We're getting there because btrfs_evict_inode -> 
> btrfs_wait_ordered_range -> btrfs_fdatawrite_range -> 
> filemap_fdatawrite_range gets called with inode->i_mapping.  That 
> mapping gets passed down through __filemap_fdatawrite_range to 
> wbc_attach_fdatawrite_inode where the inode passed is mapping->host
> -- which will be the block device inode rather than the btrfs
> device node inode.  That inode is the one ultimately checked in
> inode_to_bdi.
> 
> So it looks like we're causing writeback on an unrelated block
> device that was opened using a device node hosted on btrfs, which
> is obviously wrong.
> 
> I don't think snapshot removal is even a requirement to trigger
> this. I expect it's possible to trigger with two device nodes for
> the same block device where one is getting closed and cleaned up
> while the eviction of the other happens.  The device nodes wouldn't
> even need to be on the same fs.
> 
> Other file systems use &inode->i_data in eviction.  Is it that
> simple here?

Incidentally, this explanation also covers why I was unable to
reproduce it locally.  SLES systems use devtmpfs and I just bind
mounted it into my chroot environment like I normally would.  When I
cp'd /dev into the test environment, I was able to reproduce immediately
.

- -Jeff

- -- 
Jeff Mahoney
SUSE Labs
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.19 (Darwin)

iQIcBAEBAgAGBQJV8yYAAAoJEB57S2MheeWy2BwP+QGdpsErIfyHJcx95LLrvsxu
n0kBoI4Jd5yfNxp8m+Ll3xgUdsd6rKHJV2Muq8aRdNEdzf1E0DFrRcE0d1W5UrJy
lPzrA8QxCVaLf5jFysFp0xygKbLKHGmOAv2KnAGYFw6exIjb344UnZb6aiw5Uekm
DqrTmEq+0Yb/mE04GVpWMylK6pkDOhgkOzFVZa1Pff0eKY4E61G5GtmA2kNAUP9v
CsoZ0FO1WdF2Fc9ONSPjq7FdZLKH+OmIVakHnaELa8EEM3W7NU+mxLRabznBV25e
L/KPjr+awzkhV1ieyAAww/dddE3bN5nmDOq+OgvA9WPgaRvvwne2tHVTFaxHoiHg
d8oHDLkC1/Z1MqINLi5dZNsSuIWMvRhIMV9Th5F2rdWxrBCSRvID7N+Z2HHh6mJC
Q9rgSOyYKclTam6IF7yX8lDWIqkAnoA6OxvOKRccgr3hS/u4DzVtRmWHO9RblEi+
a9dF2FCP+v+Lgdb8C5n7XUixrtF5H6BWHhmArgjmxD6iyeXOmphyGrgqmSLdY1s9
sakvLrSB9i3O27CKoup2OHyF6MOdgsaa90FZLPLt6BDrCTWAscd0LDy8MbaKgKCR
kjfSTiwNydzZfkJixH71U/1mGbuB9nqf6jrNWCQdE5f57MSCEwiFqQvaD1KK+Uug
ZW2Bz1VQxkOvGbYiJ4HV
=ic4+
-----END PGP SIGNATURE-----

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Stefan Priebe]

Am 11.09.2015 um 21:05 schrieb Jeff Mahoney:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 9/11/15 2:55 PM, Jeff Mahoney wrote:
>> On 8/25/15 5:00 AM, Christoph Hellwig wrote:
>>> I think this is btrfs using a struct block_device that doesn't
>>> have a valid queue pointer in it's gendisk for ->s_bdev.  And
>>> there are some fishy looking ->s_bdev assignments in the code
>>> which I suspect are related to it:
>>
>>> fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev ==
>>> src_device->bdev) fs/btrfs/dev-replace.c: fs_info->sb->s_bdev =
>>> tgt_device->bdev; fs/btrfs/volumes.c:     if (device->bdev ==
>>> root->fs_info->sb->s_bdev) fs/btrfs/volumes.c:
>>> root->fs_info->sb->s_bdev = next_device->bdev;
>>> fs/btrfs/volumes.c: if (tgtdev->bdev == fs_info->sb->s_bdev)
>>> fs/btrfs/volumes.c: fs_info->sb->s_bdev = next_device->bdev;
>>
>> The report at https://bugzilla.kernel.org/show_bug.cgi?id=100911
>> tracks it down a bit further and it's bdev->bd_disk == NULL instead
>> of the queue in the gendisk. I don't think that the s_bdev stuff
>> is related, though I'd certainly love to see that bit go away.
>>
>> If we're calling blk_get_backing_dev_info, that means we're
>> already using an inode that has blockdev_superblock and the btrfs
>> superblock isn't even involved.
>>
>> We're getting there because btrfs_evict_inode ->
>> btrfs_wait_ordered_range -> btrfs_fdatawrite_range ->
>> filemap_fdatawrite_range gets called with inode->i_mapping.  That
>> mapping gets passed down through __filemap_fdatawrite_range to
>> wbc_attach_fdatawrite_inode where the inode passed is mapping->host
>> -- which will be the block device inode rather than the btrfs
>> device node inode.  That inode is the one ultimately checked in
>> inode_to_bdi.
>>
>> So it looks like we're causing writeback on an unrelated block
>> device that was opened using a device node hosted on btrfs, which
>> is obviously wrong.
>>
>> I don't think snapshot removal is even a requirement to trigger
>> this. I expect it's possible to trigger with two device nodes for
>> the same block device where one is getting closed and cleaned up
>> while the eviction of the other happens.  The device nodes wouldn't
>> even need to be on the same fs.
>>
>> Other file systems use &inode->i_data in eviction.  Is it that
>> simple here?

Your patch works fine here. Did some simple tests already.

Thanks!

Stefan

>
> Incidentally, this explanation also covers why I was unable to
> reproduce it locally.  SLES systems use devtmpfs and I just bind
> mounted it into my chroot environment like I normally would.  When I
> cp'd /dev into the test environment, I was able to reproduce immediately
> .
>
> - -Jeff
>
> - --
> Jeff Mahoney
> SUSE Labs
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG/MacGPG2 v2.0.19 (Darwin)
>
> iQIcBAEBAgAGBQJV8yYAAAoJEB57S2MheeWy2BwP+QGdpsErIfyHJcx95LLrvsxu
> n0kBoI4Jd5yfNxp8m+Ll3xgUdsd6rKHJV2Muq8aRdNEdzf1E0DFrRcE0d1W5UrJy
> lPzrA8QxCVaLf5jFysFp0xygKbLKHGmOAv2KnAGYFw6exIjb344UnZb6aiw5Uekm
> DqrTmEq+0Yb/mE04GVpWMylK6pkDOhgkOzFVZa1Pff0eKY4E61G5GtmA2kNAUP9v
> CsoZ0FO1WdF2Fc9ONSPjq7FdZLKH+OmIVakHnaELa8EEM3W7NU+mxLRabznBV25e
> L/KPjr+awzkhV1ieyAAww/dddE3bN5nmDOq+OgvA9WPgaRvvwne2tHVTFaxHoiHg
> d8oHDLkC1/Z1MqINLi5dZNsSuIWMvRhIMV9Th5F2rdWxrBCSRvID7N+Z2HHh6mJC
> Q9rgSOyYKclTam6IF7yX8lDWIqkAnoA6OxvOKRccgr3hS/u4DzVtRmWHO9RblEi+
> a9dF2FCP+v+Lgdb8C5n7XUixrtF5H6BWHhmArgjmxD6iyeXOmphyGrgqmSLdY1s9
> sakvLrSB9i3O27CKoup2OHyF6MOdgsaa90FZLPLt6BDrCTWAscd0LDy8MbaKgKCR
> kjfSTiwNydzZfkJixH71U/1mGbuB9nqf6jrNWCQdE5f57MSCEwiFqQvaD1KK+Uug
> ZW2Bz1VQxkOvGbYiJ4HV
> =ic4+
> -----END PGP SIGNATURE-----
>

Re: btrfs regression since 4.X kernel NULL pointer dereference

[Chris Mason]

On Fri, Sep 11, 2015 at 02:55:17PM -0400, Jeff Mahoney wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 8/25/15 5:00 AM, Christoph Hellwig wrote:
> > I think this is btrfs using a struct block_device that doesn't
> > have a valid queue pointer in it's gendisk for ->s_bdev.  And there
> > are some fishy looking ->s_bdev assignments in the code which I
> > suspect are related to it:
> > 
> > fs/btrfs/dev-replace.c: if (fs_info->sb->s_bdev ==
> > src_device->bdev) fs/btrfs/dev-replace.c:
> > fs_info->sb->s_bdev = tgt_device->bdev; fs/btrfs/volumes.c:     if
> > (device->bdev == root->fs_info->sb->s_bdev) fs/btrfs/volumes.c:
> > root->fs_info->sb->s_bdev = next_device->bdev; fs/btrfs/volumes.c:
> > if (tgtdev->bdev == fs_info->sb->s_bdev) fs/btrfs/volumes.c:
> > fs_info->sb->s_bdev = next_device->bdev;
> 
> The report at https://bugzilla.kernel.org/show_bug.cgi?id=100911
> tracks it down a bit further and it's bdev->bd_disk == NULL instead of
> the queue in the gendisk. I don't think that the s_bdev stuff is
> related, though I'd certainly love to see that bit go away.
> 
> If we're calling blk_get_backing_dev_info, that means we're already
> using an inode that has blockdev_superblock and the btrfs superblock
> isn't even involved.
> 
> We're getting there because btrfs_evict_inode ->
> btrfs_wait_ordered_range -> btrfs_fdatawrite_range ->
> filemap_fdatawrite_range gets called with inode->i_mapping.  That
> mapping gets passed down through __filemap_fdatawrite_range to
> wbc_attach_fdatawrite_inode where the inode passed is mapping->host --
> which will be the block device inode rather than the btrfs device node
> inode.  That inode is the one ultimately checked in inode_to_bdi.
> 
> So it looks like we're causing writeback on an unrelated block device
> that was opened using a device node hosted on btrfs, which is
> obviously wrong.
> 
> I don't think snapshot removal is even a requirement to trigger this.
>  I expect it's possible to trigger with two device nodes for the same
> block device where one is getting closed and cleaned up while the
> eviction of the other happens.  The device nodes wouldn't even need to
> be on the same fs.
> 
> Other file systems use &inode->i_data in eviction.  Is it that simple
> here?

Oh, ok I'm following now.  This really should explain it.  Jeff
mentioned that he's working on a patch to skip the wait_ordered_range
dance based on i_mode.  Thanks Jeff!

-chris