virtio: implement remove callbacks

virtio parent device drivers (e.g. PCI and MMIO) create child devices
and free them on remove. The virtio drivers for the child devices (e.g.
block and console) however don't unregister with their respective
subsystems in the remove callbacks. So these subsystems may have stale
pointers pointing at removed devices. This is especially problematic for
the console driver, because the virtio console device_d will be removed,
but the console itself remains registered leading to a use-after-free
as soon as printf is invoked for the previously active console.

This leads to a crash when typing reset in

  https://www.barebox.org/jsbarebox/?graphic=0

Fix this for all virtio drivers.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Link: https://lore.barebox.org/20210916093458.21102-1-a.fatoum@pengutronix.de
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

5 files changed, 41 insertions, 2 deletions

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index b7a83cf686..87ab505f83 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -95,6 +95,7 @@ static int virtio_blk_probe(struct virtio_device *vdev)
 		return ret;
 
 	priv->vdev = vdev;
+	vdev->priv = priv;
 
 	devnum = cdev_find_free_index("virtioblk");
 	priv->blk.cdev.name = xasprintf("virtioblk%d", devnum);
@@ -115,8 +116,13 @@ static int virtio_blk_probe(struct virtio_device *vdev)
 
 static void virtio_blk_remove(struct virtio_device *vdev)
 {
+	struct virtio_blk_priv *priv = vdev->priv;
+
 	vdev->config->reset(vdev);
+	blockdevice_unregister(&priv->blk);
 	vdev->config->del_vqs(vdev);
+
+	free(priv);
 }
 
 static const struct virtio_device_id id_table[] = {
diff --git a/drivers/hw_random/core.c b/drivers/hw_random/core.c
index ee3d5a52dd..86214dc8ba 100644
--- a/drivers/hw_random/core.c
+++ b/drivers/hw_random/core.c
@@ -92,6 +92,12 @@ static int hwrng_register_cdev(struct hwrng *rng)
 	return devfs_create(&rng->cdev);
 }
 
+static void hwrng_unregister_cdev(struct hwrng *rng)
+{
+	devfs_remove(&rng->cdev);
+	free(rng->cdev.name);
+}
+
 struct hwrng *hwrng_get_first(void)
 {
 	if (list_empty(&hwrngs))
@@ -122,3 +128,9 @@ int hwrng_register(struct device_d *dev, struct hwrng *rng)
 
 	return err;
 }
+
+void hwrng_unregister(struct hwrng *rng)
+{
+	hwrng_unregister_cdev(rng);
+	free(rng->buf);
+}
diff --git a/drivers/hw_random/virtio-rng.c b/drivers/hw_random/virtio-rng.c
index 7bdacc976e..f0a3d3cb74 100644
--- a/drivers/hw_random/virtio-rng.c
+++ b/drivers/hw_random/virtio-rng.c
@@ -78,8 +78,14 @@ static int virtrng_probe(struct virtio_device *vdev)
 
 static void virtrng_remove(struct virtio_device *vdev)
 {
+	struct virtrng_info *vi = vdev->priv;
+
 	vdev->config->reset(vdev);
+	if (vi->hwrng_register_done)
+		hwrng_unregister(&vi->hwrng);
 	vdev->config->del_vqs(vdev);
+
+	kfree(vi);
 }
 
 static void virtrng_scan(struct virtio_device *vdev)
diff --git a/drivers/input/virtio_input.c b/drivers/input/virtio_input.c
index b354933209..b5430886ab 100644
--- a/drivers/input/virtio_input.c
+++ b/drivers/input/virtio_input.c
@@ -259,10 +259,11 @@ static void virtinput_remove(struct virtio_device *vdev)
 {
 	struct virtio_input *vi = vdev->priv;
 
-	poller_unregister(&vi->poller);
-
 	vdev->config->reset(vdev);
+	poller_unregister(&vi->poller);
+	input_device_unregister(&vi->idev);
 	vdev->config->del_vqs(vdev);
+
 	kfree(vi);
 }
 
diff --git a/drivers/serial/virtio_console.c b/drivers/serial/virtio_console.c
index a1331035d9..a4adb77610 100644
--- a/drivers/serial/virtio_console.c
+++ b/drivers/serial/virtio_console.c
@@ -134,6 +134,8 @@ static int virtcons_probe(struct virtio_device *vdev)
 
 	virtcons = xzalloc(sizeof(*virtcons));
 
+	vdev->priv = virtcons;
+
 	virtcons->in_vq = vqs[0];
 	virtcons->out_vq = vqs[1];
 
@@ -150,6 +152,17 @@ static int virtcons_probe(struct virtio_device *vdev)
 	return console_register(&virtcons->cdev);
 }
 
+static void virtcons_remove(struct virtio_device *vdev)
+{
+	struct virtio_console *virtcons = vdev->priv;
+
+	vdev->config->reset(vdev);
+	console_unregister(&virtcons->cdev);
+	vdev->config->del_vqs(vdev);
+
+	free(virtcons);
+}
+
 static struct virtio_device_id id_table[] = {
 	{ VIRTIO_ID_CONSOLE, VIRTIO_DEV_ANY_ID },
 	{ 0 },
@@ -159,6 +172,7 @@ static struct virtio_driver virtio_console = {
 	.driver.name =	"virtio_console",
 	.id_table =	id_table,
 	.probe =	virtcons_probe,
+	.remove =	virtcons_remove,
 };
 device_virtio_driver(virtio_console);