Skip to contentSkip to conversation

staging: iio: Ensure mutex is correctly unlocked in __iio_push_event

PATCH New Jonathan Cameron 1 participant 2 messages Jan 9, 2010

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/2 staging: iio: Ensure mutex is correctly unlocked in __iio_push_event

---

This error was picked up by running the smatch static checker over all the IIO subsytem.

Whilst there are some false positives in some of the drivers, it picked up two real errors in the core. I was very impressed at the quality of the reporting and so Dan you can add me to your list of users as I'll be making frequent use of it in the future!

 drivers/staging/iio/industrialio-core.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
drivers/staging/iio/industrialio-core.c +4 -1
index 768f448..87799b2 100644
@@ -79,11 +79,14 @@ EXPORT_SYMBOL(__iio_change_event);
 	/* Does anyone care? */
 	mutex_lock(&ev_int->event_list_lock);
 	if (test_bit(IIO_BUSY_BIT_POS, &ev_int->handler.flags)) {
-		if (ev_int->current_events == ev_int->max_events)
+		if (ev_int->current_events == ev_int->max_events) {
+			mutex_unlock(&ev_int->event_list_lock);
 			return 0;
+		}
 		ev = kmalloc(sizeof(*ev), GFP_KERNEL);
 		if (ev == NULL) {
 			ret = -ENOMEM;
+			mutex_unlock(&ev_int->event_list_lock);
 			goto error_ret;
 		}
 		ev->ev.id = ev_code;
Signature · 1 line
1.6.4.4
#1 lore raw

PATCH 2/2 staging: iio: Fix incorrect existence check for a shared event pointer.

---
A second smatch detected error. First part fixes in a typo in the comment directly above that I noticed whilst trying to remember what this code actually does. Second part is the actual fix. I'm fairly amazed this one never caused trouble in testing as it is in one of the most common paths.

 drivers/staging/iio/industrialio-core.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)
drivers/staging/iio/industrialio-core.c +8 -8
index 87799b2..4ff683a 100644
@@ -292,16 +292,16 @@ ssize_t iio_event_chrdev_read(struct file *filep,
 	mutex_unlock(&ev_int->event_list_lock);
 	/*
 	 * Possible concurency issue if an update of this event is on its way
-	 * through. May lead to new even being removed whilst the reported event
-	 * was the unescalated event. In typical use case this is not a problem
-	 * as userspace will say read half the buffer due to a 50% full event
-	 * which would make the correct 100% full incorrect anyway.
+	 * through. May lead to new event being removed whilst the reported
+	 * event was the unescalated event. In typical use case this is not a
+	 * problem as userspace will say read half the buffer due to a 50%
+	 * full event which would make the correct 100% full incorrect anyway.
 	 */
-	spin_lock(&el->shared_pointer->lock);
-	if (el->shared_pointer)
+	if (el->shared_pointer) {
+		spin_lock(&el->shared_pointer->lock);
 		(el->shared_pointer->ev_p) = NULL;
-	spin_unlock(&el->shared_pointer->lock);
-
+		spin_unlock(&el->shared_pointer->lock);
+	}
 	kfree(el);
 
 	return len;
Signature · 1 line
1.6.4.4