[PATCH] ipmi: bt-bmc: keep device alive across remove

[Qing Ming]

misc_deregister() prevents new opens but does not close existing files.
bt_bmc and its MMIO mapping are devm-managed, so platform device removal
releases them while an open /dev/ipmi-bt-host file can still reach them
through poll and release.

Keeping /dev/ipmi-bt-host open across platform removal and then polling the
old file triggers:

  BUG: KASAN: slab-use-after-free in bt_bmc_poll

Allocate bt_bmc explicitly and hold a reference for the successful open.
Mark the device dead before deregistering the miscdevice, wake blocked
operations, and reject old-file MMIO access after removal. Serialize
process-context MMIO with the disconnect state and use a spinlock for wait
conditions and the IRQ handler.

Shut down the self-rearming poll timer, or release and synchronize the IRQ,
before dropping the probe reference. Leave the final file release to free
the software context when necessary.

Fixes: 54f9c4d0778b ("ipmi: add an Aspeed BT IPMI BMC driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Qing Ming <a0yami@xxxxxxxxxxx>
---
 drivers/char/ipmi/bt-bmc.c | 141 ++++++++++++++++++++++++++++++++-----
 1 file changed, 122 insertions(+), 19 deletions(-)

diff --git a/drivers/char/ipmi/bt-bmc.c b/drivers/char/ipmi/bt-bmc.c
index a179d4797011..481c83875908 100644
--- a/drivers/char/ipmi/bt-bmc.c
+++ b/drivers/char/ipmi/bt-bmc.c
@@ -8,12 +8,14 @@
 #include <linux/errno.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/kref.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/poll.h>
 #include <linux/sched.h>
+#include <linux/slab.h>
 #include <linux/timer.h>
 
 /*
@@ -55,6 +57,7 @@
 #define BT_BMC_BUFFER_SIZE 256
 
 struct bt_bmc {
+	struct kref		refcount;
 	struct device		dev;
 	struct miscdevice	miscdev;
 	void __iomem		*base;
@@ -62,6 +65,9 @@ struct bt_bmc {
 	wait_queue_head_t	queue;
 	struct timer_list	poll_timer;
 	struct mutex		mutex;
+	/* Protects the disconnect state against MMIO access. */
+	spinlock_t		state_lock;
+	bool			dead;
 };
 
 static atomic_t open_count = ATOMIC_INIT(0);
@@ -143,20 +149,61 @@ static void set_sms_atn(struct bt_bmc *bt_bmc)
 
 static struct bt_bmc *file_bt_bmc(struct file *file)
 {
-	return container_of(file->private_data, struct bt_bmc, miscdev);
+	return file->private_data;
+}
+
+static void bt_bmc_free(struct kref *refcount)
+{
+	struct bt_bmc *bt_bmc;
+
+	bt_bmc = container_of(refcount, struct bt_bmc, refcount);
+	kfree(bt_bmc);
+}
+
+static bool bt_bmc_request_ready(struct bt_bmc *bt_bmc)
+{
+	unsigned long flags;
+	bool ready;
+
+	spin_lock_irqsave(&bt_bmc->state_lock, flags);
+	ready = bt_bmc->dead ||
+		(bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN);
+	spin_unlock_irqrestore(&bt_bmc->state_lock, flags);
+	return ready;
+}
+
+static bool bt_bmc_response_ready(struct bt_bmc *bt_bmc)
+{
+	unsigned long flags;
+	bool ready;
+
+	spin_lock_irqsave(&bt_bmc->state_lock, flags);
+	ready = bt_bmc->dead ||
+		!(bt_inb(bt_bmc, BT_CTRL) &
+		  (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN));
+	spin_unlock_irqrestore(&bt_bmc->state_lock, flags);
+	return ready;
 }
 
 static int bt_bmc_open(struct inode *inode, struct file *file)
 {
-	struct bt_bmc *bt_bmc = file_bt_bmc(file);
+	struct bt_bmc *bt_bmc;
+	int ret = 0;
 
-	if (atomic_inc_return(&open_count) == 1) {
+	bt_bmc = container_of(file->private_data, struct bt_bmc, miscdev);
+	mutex_lock(&bt_bmc->mutex);
+	if (bt_bmc->dead) {
+		ret = -ENODEV;
+	} else if (atomic_inc_return(&open_count) == 1) {
+		kref_get(&bt_bmc->refcount);
+		file->private_data = bt_bmc;
 		clr_b_busy(bt_bmc);
-		return 0;
+	} else {
+		atomic_dec(&open_count);
+		ret = -EBUSY;
 	}
-
-	atomic_dec(&open_count);
-	return -EBUSY;
+	mutex_unlock(&bt_bmc->mutex);
+	return ret;
 }
 
 /*
@@ -185,10 +232,14 @@ static ssize_t bt_bmc_read(struct file *file, char __user *buf,
 	WARN_ON(*ppos);
 
 	if (wait_event_interruptible(bt_bmc->queue,
-				     bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN))
+				     bt_bmc_request_ready(bt_bmc)))
 		return -ERESTARTSYS;
 
 	mutex_lock(&bt_bmc->mutex);
+	if (bt_bmc->dead) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
 
 	if (unlikely(!(bt_inb(bt_bmc, BT_CTRL) & BT_CTRL_H2B_ATN))) {
 		ret = -EIO;
@@ -259,11 +310,14 @@ static ssize_t bt_bmc_write(struct file *file, const char __user *buf,
 	 * poll
 	 */
 	if (wait_event_interruptible(bt_bmc->queue,
-				     !(bt_inb(bt_bmc, BT_CTRL) &
-				       (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))))
+				     bt_bmc_response_ready(bt_bmc)))
 		return -ERESTARTSYS;
 
 	mutex_lock(&bt_bmc->mutex);
+	if (bt_bmc->dead) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
 
 	if (unlikely(bt_inb(bt_bmc, BT_CTRL) &
 		     (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN))) {
@@ -298,21 +352,35 @@ static long bt_bmc_ioctl(struct file *file, unsigned int cmd,
 			 unsigned long param)
 {
 	struct bt_bmc *bt_bmc = file_bt_bmc(file);
+	long ret = -EINVAL;
 
+	mutex_lock(&bt_bmc->mutex);
+	if (bt_bmc->dead) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
 	switch (cmd) {
 	case BT_BMC_IOCTL_SMS_ATN:
 		set_sms_atn(bt_bmc);
-		return 0;
+		ret = 0;
+		break;
 	}
-	return -EINVAL;
+
+out_unlock:
+	mutex_unlock(&bt_bmc->mutex);
+	return ret;
 }
 
 static int bt_bmc_release(struct inode *inode, struct file *file)
 {
 	struct bt_bmc *bt_bmc = file_bt_bmc(file);
 
+	mutex_lock(&bt_bmc->mutex);
 	atomic_dec(&open_count);
-	set_b_busy(bt_bmc);
+	if (!bt_bmc->dead)
+		set_b_busy(bt_bmc);
+	mutex_unlock(&bt_bmc->mutex);
+	kref_put(&bt_bmc->refcount, bt_bmc_free);
 	return 0;
 }
 
@@ -324,6 +392,11 @@ static __poll_t bt_bmc_poll(struct file *file, poll_table *wait)
 
 	poll_wait(file, &bt_bmc->queue, wait);
 
+	mutex_lock(&bt_bmc->mutex);
+	if (bt_bmc->dead) {
+		mutex_unlock(&bt_bmc->mutex);
+		return EPOLLERR | EPOLLHUP;
+	}
 	ctrl = bt_inb(bt_bmc, BT_CTRL);
 
 	if (ctrl & BT_CTRL_H2B_ATN)
@@ -331,6 +404,7 @@ static __poll_t bt_bmc_poll(struct file *file, poll_table *wait)
 
 	if (!(ctrl & (BT_CTRL_H_BUSY | BT_CTRL_B2H_ATN)))
 		mask |= EPOLLOUT;
+	mutex_unlock(&bt_bmc->mutex);
 
 	return mask;
 }
@@ -357,16 +431,25 @@ static void poll_timer(struct timer_list *t)
 static irqreturn_t bt_bmc_irq(int irq, void *arg)
 {
 	struct bt_bmc *bt_bmc = arg;
+	unsigned long flags;
 	u32 reg;
 
+	spin_lock_irqsave(&bt_bmc->state_lock, flags);
+	if (bt_bmc->dead) {
+		spin_unlock_irqrestore(&bt_bmc->state_lock, flags);
+		return IRQ_NONE;
+	}
 	reg = readl(bt_bmc->base + BT_CR2);
 
 	reg &= BT_CR2_IRQ_H2B | BT_CR2_IRQ_HBUSY;
-	if (!reg)
+	if (!reg) {
+		spin_unlock_irqrestore(&bt_bmc->state_lock, flags);
 		return IRQ_NONE;
+	}
 
 	/* ack pending IRQs */
 	writel(reg, bt_bmc->base + BT_CR2);
+	spin_unlock_irqrestore(&bt_bmc->state_lock, flags);
 
 	wake_up(&bt_bmc->queue);
 	return IRQ_HANDLED;
@@ -413,17 +496,21 @@ static int bt_bmc_probe(struct platform_device *pdev)
 	dev = &pdev->dev;
 	dev_info(dev, "Found bt bmc device\n");
 
-	bt_bmc = devm_kzalloc(dev, sizeof(*bt_bmc), GFP_KERNEL);
+	bt_bmc = kzalloc_obj(struct bt_bmc);
 	if (!bt_bmc)
 		return -ENOMEM;
+	kref_init(&bt_bmc->refcount);
 
 	dev_set_drvdata(&pdev->dev, bt_bmc);
 
 	bt_bmc->base = devm_platform_ioremap_resource(pdev, 0);
-	if (IS_ERR(bt_bmc->base))
-		return PTR_ERR(bt_bmc->base);
+	if (IS_ERR(bt_bmc->base)) {
+		rc = PTR_ERR(bt_bmc->base);
+		goto err_free;
+	}
 
 	mutex_init(&bt_bmc->mutex);
+	spin_lock_init(&bt_bmc->state_lock);
 	init_waitqueue_head(&bt_bmc->queue);
 
 	bt_bmc->miscdev.minor	= MISC_DYNAMIC_MINOR;
@@ -433,7 +520,7 @@ static int bt_bmc_probe(struct platform_device *pdev)
 	rc = misc_register(&bt_bmc->miscdev);
 	if (rc) {
 		dev_err(dev, "Unable to register misc device\n");
-		return rc;
+		goto err_free;
 	}
 
 	bt_bmc_config_irq(bt_bmc, pdev);
@@ -457,15 +544,31 @@ static int bt_bmc_probe(struct platform_device *pdev)
 	clr_b_busy(bt_bmc);
 
 	return 0;
+
+err_free:
+	dev_set_drvdata(dev, NULL);
+	kref_put(&bt_bmc->refcount, bt_bmc_free);
+	return rc;
 }
 
 static void bt_bmc_remove(struct platform_device *pdev)
 {
 	struct bt_bmc *bt_bmc = dev_get_drvdata(&pdev->dev);
+	unsigned long flags;
 
+	mutex_lock(&bt_bmc->mutex);
+	spin_lock_irqsave(&bt_bmc->state_lock, flags);
+	bt_bmc->dead = true;
+	spin_unlock_irqrestore(&bt_bmc->state_lock, flags);
+	mutex_unlock(&bt_bmc->mutex);
 	misc_deregister(&bt_bmc->miscdev);
+	wake_up_all(&bt_bmc->queue);
 	if (bt_bmc->irq < 0)
-		timer_delete_sync(&bt_bmc->poll_timer);
+		timer_shutdown_sync(&bt_bmc->poll_timer);
+	else
+		devm_free_irq(&pdev->dev, bt_bmc->irq, bt_bmc);
+	dev_set_drvdata(&pdev->dev, NULL);
+	kref_put(&bt_bmc->refcount, bt_bmc_free);
 }
 
 static const struct of_device_id bt_bmc_match[] = {
-- 
2.53.0