mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: William Breathitt Gray <vilhelm.gray@gmail.com>
To: gregkh@linuxfoundation.org
Cc: david@lechnology.com, linux-iio@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	William Breathitt Gray <vilhelm.gray@gmail.com>
Subject: [PATCH] counter: Fix use-after-free race condition for events_queue_size write
Date: Thu, 21 Oct 2021 19:35:40 +0900	[thread overview]
Message-ID: <20211021103540.955639-1-vilhelm.gray@gmail.com> (raw)

A race condition is possible when writing to events_queue_size where the
events kfifo is freed during the execution of a kfifo_in(), resulting in
a use-after-free. This patch prevents such a scenario by protecting the
events queue in operation with a spinlock and locking before performing
the events queue size adjustment.

The existing events_lock mutex is renamed to events_out_lock to reflect
that it only protects events queue out operations. Because the events
queue in operations can occur in an interrupt context, a new
events_in_lock spinlock is introduced and utilized.

Fixes: feff17a550c7 ("counter: Implement events_queue_size sysfs attribute")
Cc: David Lechner <david@lechnology.com>
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
---
 drivers/counter/counter-chrdev.c | 10 ++++++----
 drivers/counter/counter-sysfs.c  |  7 +++++++
 include/linux/counter.h          |  6 ++++--
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/counter/counter-chrdev.c b/drivers/counter/counter-chrdev.c
index b747dc81c..afa41c517 100644
--- a/drivers/counter/counter-chrdev.c
+++ b/drivers/counter/counter-chrdev.c
@@ -82,10 +82,10 @@ static ssize_t counter_chrdev_read(struct file *filp, char __user *buf,
 				return -ENODEV;
 		}
 
-		if (mutex_lock_interruptible(&counter->events_lock))
+		if (mutex_lock_interruptible(&counter->events_out_lock))
 			return -ERESTARTSYS;
 		err = kfifo_to_user(&counter->events, buf, len, &copied);
-		mutex_unlock(&counter->events_lock);
+		mutex_unlock(&counter->events_out_lock);
 		if (err < 0)
 			return err;
 	} while (!copied);
@@ -437,7 +437,8 @@ int counter_chrdev_add(struct counter_device *const counter)
 	spin_lock_init(&counter->events_list_lock);
 	mutex_init(&counter->n_events_list_lock);
 	init_waitqueue_head(&counter->events_wait);
-	mutex_init(&counter->events_lock);
+	spin_lock_init(&counter->events_in_lock);
+	mutex_init(&counter->events_out_lock);
 
 	/* Initialize character device */
 	cdev_init(&counter->chrdev, &counter_fops);
@@ -560,7 +561,8 @@ void counter_push_event(struct counter_device *const counter, const u8 event,
 		ev.watch.component = comp_node->component;
 		ev.status = -counter_get_data(counter, comp_node, &ev.value);
 
-		copied += kfifo_in(&counter->events, &ev, 1);
+		copied += kfifo_in_spinlocked_noirqsave(&counter->events, &ev,
+							1, &counter->events_in_lock);
 	}
 
 exit_early:
diff --git a/drivers/counter/counter-sysfs.c b/drivers/counter/counter-sysfs.c
index 8c2d7c29e..03564c3af 100644
--- a/drivers/counter/counter-sysfs.c
+++ b/drivers/counter/counter-sysfs.c
@@ -12,6 +12,8 @@
 #include <linux/kfifo.h>
 #include <linux/kstrtox.h>
 #include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
 #include <linux/string.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
@@ -797,6 +799,7 @@ static int counter_events_queue_size_write(struct counter_device *counter,
 {
 	DECLARE_KFIFO_PTR(events, struct counter_event);
 	int err;
+	unsigned long flags;
 
 	/* Allocate new events queue */
 	err = kfifo_alloc(&events, val, GFP_KERNEL);
@@ -804,8 +807,12 @@ static int counter_events_queue_size_write(struct counter_device *counter,
 		return err;
 
 	/* Swap in new events queue */
+	mutex_lock(&counter->events_out_lock);
+	spin_lock_irqsave(&counter->events_in_lock, flags);
 	kfifo_free(&counter->events);
 	counter->events.kfifo = events.kfifo;
+	spin_unlock_irqrestore(&counter->events_in_lock, flags);
+	mutex_unlock(&counter->events_out_lock);
 
 	return 0;
 }
diff --git a/include/linux/counter.h b/include/linux/counter.h
index 0fd99e255..b7d0a00a6 100644
--- a/include/linux/counter.h
+++ b/include/linux/counter.h
@@ -296,7 +296,8 @@ struct counter_ops {
  * @n_events_list_lock:	lock to protect Counter next events list operations
  * @events:		queue of detected Counter events
  * @events_wait:	wait queue to allow blocking reads of Counter events
- * @events_lock:	lock to protect Counter events queue read operations
+ * @events_in_lock:	lock to protect Counter events queue in operations
+ * @events_out_lock:	lock to protect Counter events queue out operations
  * @ops_exist_lock:	lock to prevent use during removal
  */
 struct counter_device {
@@ -323,7 +324,8 @@ struct counter_device {
 	struct mutex n_events_list_lock;
 	DECLARE_KFIFO_PTR(events, struct counter_event);
 	wait_queue_head_t events_wait;
-	struct mutex events_lock;
+	spinlock_t events_in_lock;
+	struct mutex events_out_lock;
 	struct mutex ops_exist_lock;
 };
 

base-commit: 8135cc5b270b3f224615bdee8bd7d66afee87991
-- 
2.33.1


                 reply	other threads:[~2021-10-21 10:35 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211021103540.955639-1-vilhelm.gray@gmail.com \
    --to=vilhelm.gray@gmail.com \
    --cc=david@lechnology.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®