On 10/31/21 9:54 AM, Andy Shevchenko wrote:
On Sunday, October 31, 2021, Lars-Peter Clausen <lars@xxxxxxxxxx
<mailto:lars@xxxxxxxxxx>> wrote:
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@xxxxxxxxxx
<mailto:lars@xxxxxxxxxx>>
---
drivers/iio/industrialio-trigger.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/industrialio-trigger.c
b/drivers/iio/industrialio-trigger.c
index 93990ff1dfe3..d566e8d4a14b 100644
--- a/drivers/iio/industrialio-trigger.c
+++ b/drivers/iio/industrialio-trigger.c
@@ -480,7 +480,7 @@ static void iio_trig_release(struct device
*device)
struct iio_trigger *trig = to_iio_trigger(device);
int i;
- if (trig->subirq_base) {
+ if (trig->subirq_base > 0) {
>= ?
I don't know. 0 is not supposed to be a valid irq number. And we
kzalloc() the struct, so if it hasn't been explicitly initialized we'd
get 0.
The way the code is at the moment we'd never end up here without calling
irq_alloc_descs(), so it is either a valid irq or a negative error code
and I can see why you might want to use >= for consistency and symmetry.
for (i = 0; i < CONFIG_IIO_CONSUMERS_PER_TRIGGER;
i++) {
irq_modify_status(trig->subirq_base + i,
IRQ_NOAUTOEN,
@@ -541,11 +541,11 @@ struct iio_trigger
*viio_trigger_alloc(struct device *parent,
CONFIG_IIO_CONSUMERS_PER_TRIGGER,
0);
if (trig->subirq_base < 0)
- goto free_trig;
+ goto err_put_trig;
trig->name = kvasprintf(GFP_KERNEL, fmt, vargs);
if (trig->name == NULL)
- goto free_descs;
+ goto err_put_trig;
trig->subirq_chip.name <http://subirq_chip.name> = trig->name;
trig->subirq_chip.irq_mask = &iio_trig_subirqmask;
@@ -559,10 +559,8 @@ struct iio_trigger *viio_trigger_alloc(struct
device *parent,
return trig;
-free_descs:
- irq_free_descs(trig->subirq_base,
CONFIG_IIO_CONSUMERS_PER_TRIGGER);
-free_trig:
- kfree(trig);
+err_put_trig:
+ put_device(&trig->dev);
return NULL;
}
--
2.20.1
--
With Best Regards,
Andy Shevchenko
