On Thu, Jun 2, 2022 at 2:02 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxx> > > Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx> > > 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
