[PATCH] iio: trigger: free hrtimer and loop triggers on configfs release

[PATCH] iio: trigger: free hrtimer and loop triggers on configfs release

[Changyul Lee]

Removing an hrtimer or loop trigger through configfs causes a
use-after-free:

  BUG: KASAN: slab-use-after-free in config_item_put+0x1f/0x60
  Write of size 4 at addr ffff888001bf322c by task rmdir/71
  Call Trace:
   config_item_put+0x1f/0x60
   configfs_rmdir+0x7b7/0x9f0
   vfs_rmdir+0x291/0x7b0
   ...
  Allocated by task 70:
   iio_trig_hrtimer_probe+0x46/0x2b0
   iio_sw_trigger_create+0x11b/0x210
   trigger_make_group+0x2f/0x70
   configfs_mkdir+0x477/0xf00
   ...
  Freed by task 71:
   kfree+0x121/0x390
   iio_trig_hrtimer_remove+0x86/0xb0
   trigger_drop_group+0x78/0xd0
   configfs_rmdir+0x7b7/0x9f0
   ...

On rmdir, trigger_drop_group() calls iio_sw_trigger_destroy(), and the
->remove() callback of the trigger frees the structure that embeds the
config_item. The config_item_put() in trigger_drop_group() and the
final config_item_put() in configfs_rmdir() then access freed memory.

Free the structure from the ->release() callback of the
config_item_type instead, which configfs calls when the last reference
is dropped. Documentation/filesystems/configfs.rst requires this
callback for dynamically allocated items.

Fixes: ac5006a2a558 ("iio: trigger: Introduce IIO hrtimer based trigger")
Fixes: bc2e1126eccb ("iio:trigger: Experimental kthread tight loop trigger (thread only)")
Signed-off-by: Changyul Lee <lcy8047@gmail.com>
---
Found with KASAN.

Reproduced in QEMU (x86_64, KASAN) by running mkdir and then rmdir on
an hrtimer trigger and a loop trigger in configfs. Without this patch,
KASAN reports the use-after-free on the first rmdir of each. With it,
1000 rounds of mkdir and rmdir on both trigger types give no KASAN
report.

 drivers/iio/trigger/iio-trig-hrtimer.c | 13 ++++++++++++-
 drivers/iio/trigger/iio-trig-loop.c    | 17 ++++++++++++-----
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/trigger/iio-trig-hrtimer.c b/drivers/iio/trigger/iio-trig-hrtimer.c
index 57550b581593..469bd83331b2 100644
--- a/drivers/iio/trigger/iio-trig-hrtimer.c
+++ b/drivers/iio/trigger/iio-trig-hrtimer.c
@@ -29,7 +29,19 @@ struct iio_hrtimer_info {
 	ktime_t period;
 };
 
+static void iio_hrtimer_release(struct config_item *item)
+{
+	struct iio_sw_trigger *swt = to_iio_sw_trigger(item);
+
+	kfree(container_of(swt, struct iio_hrtimer_info, swt));
+}
+
+static const struct configfs_item_operations iio_hrtimer_item_ops = {
+	.release = iio_hrtimer_release,
+};
+
 static const struct config_item_type iio_hrtimer_type = {
+	.ct_item_ops = &iio_hrtimer_item_ops,
 	.ct_owner = THIS_MODULE,
 };
 
@@ -176,7 +188,6 @@ static int iio_trig_hrtimer_remove(struct iio_sw_trigger *swt)
 	/* cancel the timer after unreg to make sure no one rearms it */
 	hrtimer_cancel(&trig_info->timer);
 	iio_trigger_free(swt->trigger);
-	kfree(trig_info);
 
 	return 0;
 }
diff --git a/drivers/iio/trigger/iio-trig-loop.c b/drivers/iio/trigger/iio-trig-loop.c
index 580a465035d1..e6bea96cc565 100644
--- a/drivers/iio/trigger/iio-trig-loop.c
+++ b/drivers/iio/trigger/iio-trig-loop.c
@@ -35,7 +35,19 @@ struct iio_loop_info {
 	struct task_struct *task;
 };
 
+static void iio_loop_release(struct config_item *item)
+{
+	struct iio_sw_trigger *swt = to_iio_sw_trigger(item);
+
+	kfree(container_of(swt, struct iio_loop_info, swt));
+}
+
+static const struct configfs_item_operations iio_loop_item_ops = {
+	.release = iio_loop_release,
+};
+
 static const struct config_item_type iio_loop_type = {
+	.ct_item_ops = &iio_loop_item_ops,
 	.ct_owner = THIS_MODULE,
 };
 
@@ -111,13 +123,8 @@ static struct iio_sw_trigger *iio_trig_loop_probe(const char *name)
 
 static int iio_trig_loop_remove(struct iio_sw_trigger *swt)
 {
-	struct iio_loop_info *trig_info;
-
-	trig_info = iio_trigger_get_drvdata(swt->trigger);
-
 	iio_trigger_unregister(swt->trigger);
 	iio_trigger_free(swt->trigger);
-	kfree(trig_info);
 
 	return 0;
 }

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0