Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-29 Thread Tejun Heo
Hello, On Mon, Aug 29, 2016 at 11:33:41PM +0200, Vegard Nossum wrote: > On 08/29/2016 09:55 PM, Tejun Heo wrote: > > I think the right thing to do there is doing blkdev_get() / > > blkdev_put() around func() invocation in iterate_bdevs() rather than > > holding bd_mutex across the callback. Can

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-29 Thread Tejun Heo
Hello, On Mon, Aug 29, 2016 at 11:33:41PM +0200, Vegard Nossum wrote: > On 08/29/2016 09:55 PM, Tejun Heo wrote: > > I think the right thing to do there is doing blkdev_get() / > > blkdev_put() around func() invocation in iterate_bdevs() rather than > > holding bd_mutex across the callback. Can

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-29 Thread Vegard Nossum
On 08/29/2016 09:55 PM, Tejun Heo wrote: On Sat, Aug 27, 2016 at 11:30:22AM +0200, Vegard Nossum wrote: If people who are more savvy in block/fs code could ack the locking bits I think we should apply the patch ASAP because it's an easy local DOS if you have (open/read) access to any block

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-29 Thread Vegard Nossum
On 08/29/2016 09:55 PM, Tejun Heo wrote: On Sat, Aug 27, 2016 at 11:30:22AM +0200, Vegard Nossum wrote: If people who are more savvy in block/fs code could ack the locking bits I think we should apply the patch ASAP because it's an easy local DOS if you have (open/read) access to any block

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-29 Thread Tejun Heo
Hello, On Mon, Aug 29, 2016 at 10:49:57PM +0200, Vegard Nossum wrote: > That didn't work at all. I guess bd_acquire() would just do a bdgrab() > and not touch ->bd_holders, whereas blkdev_get() would increment Yeah, bdev has two different refs - one for bdev struct itself and the other for the

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-29 Thread Tejun Heo
Hello, On Mon, Aug 29, 2016 at 10:49:57PM +0200, Vegard Nossum wrote: > That didn't work at all. I guess bd_acquire() would just do a bdgrab() > and not touch ->bd_holders, whereas blkdev_get() would increment Yeah, bdev has two different refs - one for bdev struct itself and the other for the

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-29 Thread Vegard Nossum
On 08/29/2016 09:55 PM, Tejun Heo wrote: Hello, On Sat, Aug 27, 2016 at 11:30:22AM +0200, Vegard Nossum wrote: Don't know what's the right fix, but I posted a slightly different one for the same crash some months ago: https://patchwork.kernel.org/patch/8556941/ Ah, I'm sorry, I didn't see

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-29 Thread Vegard Nossum
On 08/29/2016 09:55 PM, Tejun Heo wrote: Hello, On Sat, Aug 27, 2016 at 11:30:22AM +0200, Vegard Nossum wrote: Don't know what's the right fix, but I posted a slightly different one for the same crash some months ago: https://patchwork.kernel.org/patch/8556941/ Ah, I'm sorry, I didn't see

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-29 Thread Tejun Heo
Hello, On Sat, Aug 27, 2016 at 11:30:22AM +0200, Vegard Nossum wrote: > > Don't know what's the right fix, but I posted a slightly different one > > for the same crash some months ago: > > https://patchwork.kernel.org/patch/8556941/ > > > > Ah, I'm sorry, I didn't see that. > > Your patch is

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-29 Thread Tejun Heo
Hello, On Sat, Aug 27, 2016 at 11:30:22AM +0200, Vegard Nossum wrote: > > Don't know what's the right fix, but I posted a slightly different one > > for the same crash some months ago: > > https://patchwork.kernel.org/patch/8556941/ > > > > Ah, I'm sorry, I didn't see that. > > Your patch is

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-27 Thread Vegard Nossum
On 08/27/2016 11:03 AM, Rabin Vincent wrote: On Sat, Aug 27, 2016 at 09:07:28AM +0200, Vegard Nossum wrote: I got this with syzkaller: general protection fault: [#1] PREEMPT SMP KASAN Dumping ftrace buffer: (ftrace buffer empty) CPU: 0 PID: 11941 Comm: syz-executor Not

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-27 Thread Vegard Nossum
On 08/27/2016 11:03 AM, Rabin Vincent wrote: On Sat, Aug 27, 2016 at 09:07:28AM +0200, Vegard Nossum wrote: I got this with syzkaller: general protection fault: [#1] PREEMPT SMP KASAN Dumping ftrace buffer: (ftrace buffer empty) CPU: 0 PID: 11941 Comm: syz-executor Not

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-27 Thread Rabin Vincent
On Sat, Aug 27, 2016 at 09:07:28AM +0200, Vegard Nossum wrote: > I got this with syzkaller: > > general protection fault: [#1] PREEMPT SMP KASAN > Dumping ftrace buffer: >(ftrace buffer empty) > CPU: 0 PID: 11941 Comm: syz-executor Not tainted 4.8.0-rc2+ #169 >

Re: [PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-27 Thread Rabin Vincent
On Sat, Aug 27, 2016 at 09:07:28AM +0200, Vegard Nossum wrote: > I got this with syzkaller: > > general protection fault: [#1] PREEMPT SMP KASAN > Dumping ftrace buffer: >(ftrace buffer empty) > CPU: 0 PID: 11941 Comm: syz-executor Not tainted 4.8.0-rc2+ #169 >

[PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-27 Thread Vegard Nossum
I got this with syzkaller: general protection fault: [#1] PREEMPT SMP KASAN Dumping ftrace buffer: (ftrace buffer empty) CPU: 0 PID: 11941 Comm: syz-executor Not tainted 4.8.0-rc2+ #169 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS

[PATCH] bdev: fix NULL pointer dereference in sync()/close() race

2016-08-27 Thread Vegard Nossum
I got this with syzkaller: general protection fault: [#1] PREEMPT SMP KASAN Dumping ftrace buffer: (ftrace buffer empty) CPU: 0 PID: 11941 Comm: syz-executor Not tainted 4.8.0-rc2+ #169 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS