lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Wed, 15 Jul 2020 07:16:29 +0300
From:   Alexandru Ardelean <alexandru.ardelean@...log.com>
To:     <linux-iio@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC:     <jic23@...nel.org>, <lars@...afoo.de>,
        Alexandru Ardelean <alexandru.ardelean@...log.com>
Subject: [PATCH] iio: buffer: fix attach/detach pollfunc order

The original patch was error-ed by the submitter (me) and not by the author
(Lars).
After looking through the discussion logs (on email), it seems that this
order was wrong for the start, even though the order implemented in the
drivers was correct.

Discussions:
- first RFC: https://lore.kernel.org/linux-iio/20180622135322.3459-1-alexandru.ardelean@analog.com/
- 2nd patch: https://lore.kernel.org/linux-iio/20181219140912.22582-1-alexandru.ardelean@analog.com/
- final patch-sets:
  https://lore.kernel.org/linux-iio/20200522104632.517470-1-alexandru.ardelean@analog.com/
  https://lore.kernel.org/linux-iio/20200525113855.178821-1-alexandru.ardelean@analog.com/

The last one was applied.

The idea is that pollfunc should be attached before calling the
'indio_dev->setup_ops->postenable' hook and should be detached after
calling the 'indio_dev->setup_ops->predisable' hook.

While the drivers were updated to take this into account, the change to the
IIO core was somehow omitted and was made wrong.

This change fixes the order to the proper form.

Fixes f11d59d87b862: ("iio: Move attach/detach of the poll func to the core")
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@...log.com>
---
 drivers/iio/industrialio-buffer.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/industrialio-buffer.c b/drivers/iio/industrialio-buffer.c
index 2aec8b85f40d..a7d7e5143ed2 100644
--- a/drivers/iio/industrialio-buffer.c
+++ b/drivers/iio/industrialio-buffer.c
@@ -971,24 +971,29 @@ static int iio_enable_buffers(struct iio_dev *indio_dev,
 			goto err_disable_buffers;
 	}
 
+	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
+		ret = iio_trigger_attach_poll_func(indio_dev->trig,
+						   indio_dev->pollfunc);
+		if (ret)
+			goto err_disable_buffers;
+	}
+
 	if (indio_dev->setup_ops->postenable) {
 		ret = indio_dev->setup_ops->postenable(indio_dev);
 		if (ret) {
 			dev_dbg(&indio_dev->dev,
 			       "Buffer not started: postenable failed (%d)\n", ret);
-			goto err_disable_buffers;
+			goto err_detach_pollfunc;
 		}
 	}
 
-	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
-		ret = iio_trigger_attach_poll_func(indio_dev->trig,
-						   indio_dev->pollfunc);
-		if (ret)
-			goto err_disable_buffers;
-	}
-
 	return 0;
 
+err_detach_pollfunc:
+	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
+		iio_trigger_detach_poll_func(indio_dev->trig,
+					     indio_dev->pollfunc);
+	}
 err_disable_buffers:
 	list_for_each_entry_continue_reverse(buffer, &iio_dev_opaque->buffer_list,
 					     buffer_list)
@@ -1014,11 +1019,6 @@ static int iio_disable_buffers(struct iio_dev *indio_dev)
 	if (list_empty(&iio_dev_opaque->buffer_list))
 		return 0;
 
-	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
-		iio_trigger_detach_poll_func(indio_dev->trig,
-					     indio_dev->pollfunc);
-	}
-
 	/*
 	 * If things go wrong at some step in disable we still need to continue
 	 * to perform the other steps, otherwise we leave the device in a
@@ -1032,6 +1032,11 @@ static int iio_disable_buffers(struct iio_dev *indio_dev)
 			ret = ret2;
 	}
 
+	if (indio_dev->currentmode == INDIO_BUFFER_TRIGGERED) {
+		iio_trigger_detach_poll_func(indio_dev->trig,
+					     indio_dev->pollfunc);
+	}
+
 	list_for_each_entry(buffer, &iio_dev_opaque->buffer_list, buffer_list) {
 		ret2 = iio_buffer_disable(buffer, indio_dev);
 		if (ret2 && !ret)
-- 
2.17.1

Powered by blists - more mailing lists