diff mbox series

[v11,4/6] xen/blkback: Protect 'reclaim_memory()' with 'reclaim_lock'

Message ID 20191217160748.693-5-sjpark@amazon.com (mailing list archive)
State New, archived
Headers show
Series xenbus/backend: Add a memory pressure handler callback | expand

Commit Message

SeongJae Park Dec. 17, 2019, 4:07 p.m. UTC 
From: SeongJae Park <sjpark@amazon.de>

The 'reclaim_memory()' callback of blkback could race with
'xen_blkbk_probe()' and 'xen_blkbk_remove()'.  In the case, incompletely
linked 'backend_info' and 'blkif' might be exposed to the callback, thus
result in bad results including NULL dereference.  This commit fixes the
problem by applying the 'reclaim_lock' protection to those.

Note that this commit is separated for review purpose only.  As the
previous commit might result in race condition and might make bisect
confuse, please squash this commit into previous commit if possible.

Signed-off-by: SeongJae Park <sjpark@amazon.de>

---
 drivers/block/xen-blkback/xenbus.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Jürgen Groß Dec. 17, 2019, 4:15 p.m. UTC | #1 
On 17.12.19 17:07, SeongJae Park wrote:
> From: SeongJae Park <sjpark@amazon.de>
> 
> The 'reclaim_memory()' callback of blkback could race with
> 'xen_blkbk_probe()' and 'xen_blkbk_remove()'.  In the case, incompletely
> linked 'backend_info' and 'blkif' might be exposed to the callback, thus
> result in bad results including NULL dereference.  This commit fixes the
> problem by applying the 'reclaim_lock' protection to those.
> 
> Note that this commit is separated for review purpose only.  As the
> previous commit might result in race condition and might make bisect
> confuse, please squash this commit into previous commit if possible.
> 
> Signed-off-by: SeongJae Park <sjpark@amazon.de>

Please merge this patch into patch 2.


Juergen
diff mbox series

Patch

diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index 4f6ea4feca79..20045827a391 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -492,6 +492,7 @@  static int xen_vbd_create(struct xen_blkif *blkif, blkif_vdev_t handle,
 static int xen_blkbk_remove(struct xenbus_device *dev)
 {
 	struct backend_info *be = dev_get_drvdata(&dev->dev);
+	unsigned long flags;
 
 	pr_debug("%s %p %d\n", __func__, dev, dev->otherend_id);
 
@@ -504,6 +505,7 @@  static int xen_blkbk_remove(struct xenbus_device *dev)
 		be->backend_watch.node = NULL;
 	}
 
+	spin_lock_irqsave(&dev->reclaim_lock, flags);
 	dev_set_drvdata(&dev->dev, NULL);
 
 	if (be->blkif) {
@@ -512,6 +514,7 @@  static int xen_blkbk_remove(struct xenbus_device *dev)
 		/* Put the reference we set in xen_blkif_alloc(). */
 		xen_blkif_put(be->blkif);
 	}
+	spin_unlock_irqrestore(&dev->reclaim_lock, flags);
 
 	return 0;
 }
@@ -597,6 +600,7 @@  static int xen_blkbk_probe(struct xenbus_device *dev,
 	int err;
 	struct backend_info *be = kzalloc(sizeof(struct backend_info),
 					  GFP_KERNEL);
+	unsigned long flags;
 
 	/* match the pr_debug in xen_blkbk_remove */
 	pr_debug("%s %p %d\n", __func__, dev, dev->otherend_id);
@@ -607,6 +611,7 @@  static int xen_blkbk_probe(struct xenbus_device *dev,
 		return -ENOMEM;
 	}
 	be->dev = dev;
+	spin_lock_irqsave(&dev->reclaim_lock, flags);
 	dev_set_drvdata(&dev->dev, be);
 
 	be->blkif = xen_blkif_alloc(dev->otherend_id);
@@ -614,8 +619,10 @@  static int xen_blkbk_probe(struct xenbus_device *dev,
 		err = PTR_ERR(be->blkif);
 		be->blkif = NULL;
 		xenbus_dev_fatal(dev, err, "creating block interface");
+		spin_unlock_irqrestore(&dev->reclaim_lock, flags);
 		goto fail;
 	}
+	spin_unlock_irqrestore(&dev->reclaim_lock, flags);
 
 	err = xenbus_printf(XBT_NIL, dev->nodename,
 			    "feature-max-indirect-segments", "%u",
@@ -838,6 +845,10 @@  static void reclaim_memory(struct xenbus_device *dev)
 {
 	struct backend_info *be = dev_get_drvdata(&dev->dev);
 
+	/* Device is registered but not probed yet */
+	if (!be)
+		return;
+
 	be->blkif->buffer_squeeze_end = jiffies +
 		msecs_to_jiffies(buffer_squeeze_duration_ms);
 }