[PATCH 1/2] iio: iio_alloc_device(): Free device correctly on error

* [PATCH 1/2] iio: iio_alloc_device(): Free device correctly on error
@ 2021-10-31  7:32 Lars-Peter Clausen
  2021-10-31  7:32 ` [PATCH 2/2] iio: viio_trigger_alloc(): Correctly free trigger " Lars-Peter Clausen
  0 siblings, 1 reply; 5+ messages in thread
From: Lars-Peter Clausen @ 2021-10-31  7:32 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: linux-iio, Lars-Peter Clausen

Once device_initialize() has been called on a struct device the device must
be freed by decreasing the reference count rather than directly freeing the
underlying memory.

This is so that any additional state and resources associated with the
device get properly freed.

In this particular case there are no additional resources associated with
the device and no additional reference count. So there is no resource leak
or use-after-free by freeing the struct device directly

But in order to follow best practices and avoid accidental future breakage
use put_device() instead of kfree() to free the device when an error
occurs.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
No fixes tag since, while the code is wrong, it works. No leaks and no
use-after-free.
 drivers/iio/industrialio-core.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 463a63d5bf56..669218365277 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1615,7 +1615,8 @@ static void iio_dev_release(struct device *device)
 
 	iio_device_detach_buffers(indio_dev);
 
-	ida_simple_remove(&iio_ida, iio_dev_opaque->id);
+	if (iio_dev_opaque->id >= 0)
+		ida_simple_remove(&iio_ida, iio_dev_opaque->id);
 	kfree(iio_dev_opaque);
 }
 
@@ -1662,20 +1663,20 @@ struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv)
 	if (iio_dev_opaque->id < 0) {
 		/* cannot use a dev_err as the name isn't available */
 		pr_err("failed to get device id\n");
-		kfree(iio_dev_opaque);
-		return NULL;
+		goto err_put_device;
 	}
 
-	if (dev_set_name(&indio_dev->dev, "iio:device%d", iio_dev_opaque->id)) {
-		ida_simple_remove(&iio_ida, iio_dev_opaque->id);
-		kfree(iio_dev_opaque);
-		return NULL;
-	}
+	if (dev_set_name(&indio_dev->dev, "iio:device%d", iio_dev_opaque->id))
+		goto err_put_device;
 
 	INIT_LIST_HEAD(&iio_dev_opaque->buffer_list);
 	INIT_LIST_HEAD(&iio_dev_opaque->ioctl_handlers);
 
 	return indio_dev;
+
+err_put_device:
+	put_device(&indio_dev->dev);
+	return NULL;
 }
 EXPORT_SYMBOL(iio_device_alloc);
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread