Messages in this thread | | | From | Jason Andryuk <> | Date | Thu, 2 Jun 2022 08:22:40 -0400 | Subject | Re: [PATCH] xen-blkfront: Handle NULL gendisk |
| |
On Thu, Jun 2, 2022 at 2:02 AM Christoph Hellwig <hch@infradead.org> wrote: > > On Wed, Jun 01, 2022 at 03:53:41PM -0400, Jason Andryuk wrote: > > When a VBD is not fully created and then closed, the kernel can have a > > NULL pointer dereference: > >
> > > > info->rq and info->gd are only set in blkfront_connect(), which is > > called for state 4 (XenbusStateConnected). Guard against using NULL > > variables in blkfront_closing() to avoid the issue. > > > > The rest of blkfront_closing looks okay. If info->nr_rings is 0, then > > for_each_rinfo won't do anything. > > > > blkfront_remove also needs to check for non-NULL pointers before > > cleaning up the gendisk and request queue. > > > > Fixes: 05d69d950d9d "xen-blkfront: sanitize the removal state machine" > > Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> > > Signed-off-by: Jason Andryuk <jandryuk@gmail.com> > > Tis looks ok, but do we have anything that prevents races between > blkfront_connect, blkfront_closing and blkfront_remove?
Thanks for taking a look, Christoph.
blkfront_connect and blkfront_closing are called by the state machine in blkback_changed. blkback_changed is the xenbus_driver .otherend_changed callback. The xenwatch kthread calls callbacks synchronously and one at a time, so that seems okay today.
blkfront_remove is the xenbus_driver .remove callback, so it is tied to the life cycle of the device. It's called after the otherend_changed callback is unregistered, so those won't run when blkfront_remove is running.
Given that, I think it's okay.
Regards, Jason
| |