Re: [PATCH] nvme: fix use-after-free during booting

2020-09-23 Thread Tong Zhang
IMHO, WARN_ON_ONCE() looks ok to me. as long as it won't crash the system or lead to any memory corruption issue. We can talk about the reproducer offline if you are interested. - Tong On Wed, Sep 23, 2020 at 1:06 AM Christoph Hellwig wrote: > > On Tue, Sep 22, 2020 at 04:34:45PM -0400, Tong

Re: [PATCH] nvme: fix use-after-free during booting

2020-09-23 Thread Tong Zhang
here you go [0.00] Linux version 5.9.0-rc4+ (tong@tong-desktop) (gcc (Ubuntu 9.3.0-10ubuntu2) 9.3.0, GNU ld (GNU Binutils for Ubuntu) 2.34) #44 SMP Wed Sep 23 12:15:34 EDT 2020 [0.00] Command line: console=ttyS0 root=/dev/sda earlyprintk=serial biosdevname=0 net.ifnames=0

Re: [PATCH] nvme: fix use-after-free during booting

2020-09-22 Thread Christoph Hellwig
I suspect the patch below might be better. Can you send me a full dmesg with this one applied? Preferably on top of Jens' for-next branch? diff --git a/block/genhd.c b/block/genhd.c index 9d060e79eb31d8..ef2784c69d59ee 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -832,7 +832,9 @@ static

Re: [PATCH] nvme: fix use-after-free during booting

2020-09-22 Thread Christoph Hellwig
On Tue, Sep 22, 2020 at 04:34:45PM -0400, Tong Zhang wrote: > Hi Christoph, > I modified the patch a bit and now it works. So you're still hitting the WARN_ON_ONCE? I think we need to fix that as well, but all the ideas I have will turn into a bigger project, so I think I'll submit this one to

Re: [PATCH] nvme: fix use-after-free during booting

2020-09-22 Thread Tong Zhang
Hi Christoph, I modified the patch a bit and now it works. Best, - Tong --- block/genhd.c | 7 +-- include/linux/genhd.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/block/genhd.c b/block/genhd.c index 99c64641c314..8c432e5f97ea 100644 --- a/block/genhd.c +++

Re: [PATCH] nvme: fix use-after-free during booting

2020-09-22 Thread Tong Zhang
Thank you Christoph. I will do some testing with my setup and let you know. - Tong On Tue, Sep 22, 2020 at 9:59 AM Christoph Hellwig wrote: > > Hi Tong, > > can you test this patch? > > diff --git a/block/genhd.c b/block/genhd.c > index 99c64641c3148c..6473ae703789e4 100644 > --- a/block/genhd.c

Re: [PATCH] nvme: fix use-after-free during booting

2020-09-22 Thread Christoph Hellwig
Hi Tong, can you test this patch? diff --git a/block/genhd.c b/block/genhd.c index 99c64641c3148c..6473ae703789e4 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -836,6 +836,7 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk, * so that it sticks around as

Re: [PATCH] nvme: fix use-after-free during booting

2020-09-17 Thread Christoph Hellwig
On Thu, Sep 17, 2020 at 11:24:59AM -0400, Tong Zhang wrote: > Hmm.. OK. Any suggestions on how to fix this? Yes, we'll need to make sure the block layer doesn't double put for a not fully setup disk/queue. It has been on my todo list for a while, I'll plan to get back to in the next days.

Re: [PATCH] nvme: fix use-after-free during booting

2020-09-17 Thread Tong Zhang
Hmm.. OK. Any suggestions on how to fix this? On Thu, Sep 17, 2020 at 4:26 AM Christoph Hellwig wrote: > > On Wed, Sep 16, 2020 at 11:36:05AM -0400, Tong Zhang wrote: > > if a nvme controller is not responding during probing, a use-after-free > > condition could happen > > This now leaks the

Re: [PATCH] nvme: fix use-after-free during booting

2020-09-17 Thread Christoph Hellwig
On Wed, Sep 16, 2020 at 11:36:05AM -0400, Tong Zhang wrote: > if a nvme controller is not responding during probing, a use-after-free > condition could happen This now leaks the queue for the regular teardown path.

[PATCH] nvme: fix use-after-free during booting

2020-09-16 Thread Tong Zhang
if a nvme controller is not responding during probing, a use-after-free condition could happen [ 215.396884] nvme nvme0: Identify Controller failed (-4) [ 215.397239] nvme nvme0: Removing after probe failure status: -5 [ 215.409079] Buffer I/O error on dev nvme0n1, logical block 0, async page