Messages in this thread Patch in this message | | | From | Hang Zhang <> | Subject | [PATCH] mailbox: mailbox-test: fix potential use-after-free issues | Date | Tue, 27 Dec 2022 19:46:17 -0800 |
| |
mbox_test_message_write() is the .write handler of the message debugfs interface, it operates on global pointers "tdev->signal" and "tdev->message" (e.g., allocation, dereference, free and nullification). However, these operations are not protected by any locks, making use-after-free possible in the concurrent setting. E.g., one invocation of the handler may have freed "tdev->signal" but being preempted before nullifying the pointer, then another invocation of the handler may dereference the now dangling pointer, causing use-after-free. Similarly, "tdev->message", as a shared pointer, may be manipulated by multiple invocations concurrently, resulting in unexpected issues such as use-after-free.
Fix these potential issues by protecting the above operations with the spinlock "tdev->lock", which has already been deployed in other handlers of the debugfs interface (e.g., .read). This patch introduces the same lock to the .write handler.
Signed-off-by: Hang Zhang <zh.nvgt@gmail.com> --- drivers/mailbox/mailbox-test.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c index 4555d678fadd..b2315261644a 100644 --- a/drivers/mailbox/mailbox-test.c +++ b/drivers/mailbox/mailbox-test.c @@ -97,6 +97,7 @@ static ssize_t mbox_test_message_write(struct file *filp, struct mbox_test_device *tdev = filp->private_data; void *data; int ret; + unsigned long flags; if (!tdev->tx_channel) { dev_err(tdev->dev, "Channel cannot do Tx\n"); @@ -110,9 +111,12 @@ static ssize_t mbox_test_message_write(struct file *filp, return -EINVAL; } + spin_lock_irqsave(&tdev->lock, flags); tdev->message = kzalloc(MBOX_MAX_MSG_LEN, GFP_KERNEL); - if (!tdev->message) - return -ENOMEM; + if (!tdev->message) { + ret = -ENOMEM; + goto out_1; + } ret = copy_from_user(tdev->message, userbuf, count); if (ret) { @@ -143,6 +147,8 @@ static ssize_t mbox_test_message_write(struct file *filp, kfree(tdev->signal); kfree(tdev->message); tdev->signal = NULL; +out_1: + spin_unlock_irqrestore(&tdev->lock, flags); return ret < 0 ? ret : count; } -- 2.39.0
| |