iio: trigger: free trigger resource correctly
Subsystems (3)
PATCH v2 1/1 iio: trigger: free trigger resource correctly
These stand-alone trigger drivers were using iio_trigger_put() where they should have been using iio_trigger_free(). The iio_trigger_put() adds a module_put which is bad since they never did a module_get.
In the sysfs driver, module_get/put's are used as triggers are added & removed. This extra module_put() occurs on an error path in the probe routine (probably rare).
In the bfin-timer & interrupt trigger drivers, the module resources are not explicitly managed, so it's doing a put on something that was never get'd. It occurs on the probe error path and on the remove path (not so rare).
Tested with the sysfs trigger driver.
The bfin & interrupt drivers were build tested & inspected only.
---
Changes in v2:
Renamed probe jump label to say free instead of put. Added further explanation to commit log.
Note from v1:
Patches to use devm_* funcs are ready to follow this for the interrupt & bfin-timer triggers.
drivers/iio/trigger/iio-trig-interrupt.c | 8 ++++---- drivers/iio/trigger/iio-trig-sysfs.c | 2 +- drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)
drivers/iio/trigger/iio-trig-interrupt.c +4 -4
@@ -58,7 +58,7 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev) trig_info =kzalloc (sizeof (*trig_info),GFP_KERNEL );if (!trig_info) { ret = -ENOMEM ; - goto error_put_trigger; + goto error_free_trigger; }iio_trigger_set_drvdata (trig, trig_info); trig_info->irq = irq;
@@ -83,8 +83,8 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)free_irq (irq, trig); error_free_trig_info:kfree (trig_info); -error_put_trigger: -iio_trigger_put (trig); +error_free_trigger: +iio_trigger_free (trig); error_ret:return ret; }
@@ -99,7 +99,7 @@ static int iio_interrupt_trigger_remove(struct platform_device *pdev)iio_trigger_unregister (trig);free_irq (trig_info->irq, trig);kfree (trig_info); -iio_trigger_put (trig); +iio_trigger_free (trig);return 0 ; }
drivers/iio/trigger/iio-trig-sysfs.c +1 -1
@@ -174,7 +174,7 @@ static int iio_sysfs_trigger_probe(int id)return 0 ; out2: -iio_trigger_put (t->trig); +iio_trigger_free (t->trig); free_t:kfree (t); out1:
drivers/staging/iio/trigger/iio-trig-bfin-timer.c +2 -2
@@ -260,7 +260,7 @@ static int iio_bfin_tmr_trigger_probe(struct platform_device *pdev) out1:iio_trigger_unregister (st->trig); out: -iio_trigger_put (st->trig); +iio_trigger_free (st->trig);return ret; }
@@ -273,7 +273,7 @@ static int iio_bfin_tmr_trigger_remove(struct platform_device *pdev)peripheral_free (st->t->pin);free_irq (st->irq, st);iio_trigger_unregister (st->trig); -iio_trigger_put (st->trig); +iio_trigger_free (st->trig);return 0 ; }
Signature · 1 line
2.1.4
Jonathan Cameron
maintainerQuoted 14 lines · Alison Schofield
These stand-alone trigger drivers were using
iio_trigger_put()where they should have been usingiio_trigger_free(). Theiio_trigger_put()adds amodule_putwhich is bad since they never did amodule_get.In the sysfs driver, module_get/put's are used as triggers are added & removed. This extra
module_put()occurs on an error path in the probe routine (probably rare).In the bfin-timer & interrupt trigger drivers, the module resources are not explicitly managed, so it's doing a put on something that was never get'd. It occurs on the probe error path and on the remove path (not so rare).
Quoted 4 lines
Tested with the sysfs trigger driver.
The bfin & interrupt drivers were build tested & inspected only.Signed-off-by Alison Schofield <[email protected]>
This is certainly more consistent.
Lars, could you sanity check as well given the bfin timer is in here.
Thanks,
Jonathan
Quoted 84 lines
---
Changes in v2:
Renamed probe jump label to say free instead of put. Added further explanation to commit log.Note from v1:
Patches to use devm_* funcs are ready to follow this for the interrupt & bfin-timer triggers.drivers/iio/trigger/iio-trig-interrupt.c | 8 ++++---- drivers/iio/trigger/iio-trig-sysfs.c | 2 +- drivers/staging/iio/trigger/iio-trig-bfin-timer.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-)drivers/iio/trigger/iio-trig-interrupt.c +4
-4@@ -58,7 +58,7 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev) trig_info =kzalloc (sizeof (*trig_info),GFP_KERNEL );if (!trig_info) { ret = -ENOMEM ; - goto error_put_trigger; + goto error_free_trigger; }iio_trigger_set_drvdata (trig, trig_info); trig_info->irq = irq;@@ -83,8 +83,8 @@ static int iio_interrupt_trigger_probe(struct platform_device *pdev)free_irq (irq, trig); error_free_trig_info:kfree (trig_info); -error_put_trigger: -iio_trigger_put (trig); +error_free_trigger: +iio_trigger_free (trig); error_ret:return ret; }@@ -99,7 +99,7 @@ static int iio_interrupt_trigger_remove(struct platform_device *pdev)iio_trigger_unregister (trig);free_irq (trig_info->irq, trig);kfree (trig_info); -iio_trigger_put (trig); +iio_trigger_free (trig);return 0 ; }drivers/iio/trigger/iio-trig-sysfs.c +1
-1@@ -174,7 +174,7 @@ static int iio_sysfs_trigger_probe(int id)return 0 ; out2: -iio_trigger_put (t->trig); +iio_trigger_free (t->trig); free_t:kfree (t); out1:drivers/staging/iio/trigger/iio-trig-bfin-timer.c +2
-2@@ -260,7 +260,7 @@ static int iio_bfin_tmr_trigger_probe(struct platform_device *pdev) out1:iio_trigger_unregister (st->trig); out: -iio_trigger_put (st->trig); +iio_trigger_free (st->trig);return ret; }@@ -273,7 +273,7 @@ static int iio_bfin_tmr_trigger_remove(struct platform_device *pdev)peripheral_free (st->t->pin);free_irq (st->irq, st);iio_trigger_unregister (st->trig); -iio_trigger_put (st->trig); +iio_trigger_free (st->trig);return 0 ; }
Quoted 15 lines · Jonathan Cameron
Quoted 14 lines · Alison Schofield
These stand-alone trigger drivers were using
iio_trigger_put()where they should have been usingiio_trigger_free(). Theiio_trigger_put()adds amodule_putwhich is bad since they never did amodule_get.In the sysfs driver, module_get/put's are used as triggers are added & removed. This extra
module_put()occurs on an error path in the probe routine (probably rare).In the bfin-timer & interrupt trigger drivers, the module resources are not explicitly managed, so it's doing a put on something that was never get'd. It occurs on the probe error path and on the remove path (not so rare).
Quoted 7 lines
Quoted 4 lines
Tested with the sysfs trigger driver.
The bfin & interrupt drivers were build tested & inspected only.Signed-off-by Alison Schofield <[email protected]>This is certainly more consistent.
Lars, could you sanity check as well given the bfin timer is in here.
looks good.
Quoted 19 lines · Lars-Peter Clausen
Quoted 18 lines · Jonathan Cameron
Quoted 17 lines · Alison Schofield
These stand-alone trigger drivers were using
iio_trigger_put()where they should have been usingiio_trigger_free(). Theiio_trigger_put()adds amodule_putwhich is bad since they never did amodule_get.In the sysfs driver, module_get/put's are used as triggers are added & removed. This extra
module_put()occurs on an error path in the probe routine (probably rare).In the bfin-timer & interrupt trigger drivers, the module resources are not explicitly managed, so it's doing a put on something that was never get'd. It occurs on the probe error path and on the remove path (not so rare).
Tested with the sysfs trigger driver.
The bfin & interrupt drivers were build tested & inspected only.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Signed-off-by Alison Schofield <[email protected]>This is certainly more consistent.
Lars, could you sanity check as well given the bfin timer is in here.looks good.
Applied to the togreg branch of iio.git.
Thought about sending this as a fix, but as it's been there a long time I've taken the view there is no real rush on this.
Thanks,
Jonathan