* [PATCH] staging: comedi: drivers: prevent memory leak
@ 2019-09-17 2:41 Navid Emamdoost
2019-09-17 6:33 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Navid Emamdoost @ 2019-09-17 2:41 UTC (permalink / raw)
Cc: emamd001, smccaman, kjlu, Navid Emamdoost, Ian Abbott,
H Hartley Sweeten, Greg Kroah-Hartman, devel, linux-kernel
In das1800_attach, the buffer allocated via kmalloc_array needs to be
released if an error happens.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
drivers/staging/comedi/drivers/das1800.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/das1800.c b/drivers/staging/comedi/drivers/das1800.c
index f16aa7e9f4f3..5f2d5f7a6229 100644
--- a/drivers/staging/comedi/drivers/das1800.c
+++ b/drivers/staging/comedi/drivers/das1800.c
@@ -1237,12 +1237,16 @@ static int das1800_attach(struct comedi_device *dev,
dev->pacer = comedi_8254_init(dev->iobase + DAS1800_COUNTER,
I8254_OSC_BASE_5MHZ, I8254_IO8, 0);
- if (!dev->pacer)
+ if (!dev->pacer) {
+ kfree(devpriv->fifo_buf);
return -ENOMEM;
+ }
ret = comedi_alloc_subdevices(dev, 4);
- if (ret)
+ if (ret) {
+ kfree(devpriv->fifo_buf);
return ret;
+ }
/*
* Analog Input subdevice
@@ -1290,8 +1294,10 @@ static int das1800_attach(struct comedi_device *dev,
s->insn_write = das1800_ao_insn_write;
ret = comedi_alloc_subdev_readback(s);
- if (ret)
+ if (ret) {
+ kfree(devpriv->fifo_buf);
return ret;
+ }
/* initialize all channels to 0V */
for (i = 0; i < s->n_chan; i++) {
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: comedi: drivers: prevent memory leak
2019-09-17 2:41 [PATCH] staging: comedi: drivers: prevent memory leak Navid Emamdoost
@ 2019-09-17 6:33 ` Dan Carpenter
2019-09-17 10:13 ` Ian Abbott
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2019-09-17 6:33 UTC (permalink / raw)
To: Navid Emamdoost
Cc: devel, Greg Kroah-Hartman, kjlu, linux-kernel, Ian Abbott,
emamd001, smccaman
On Mon, Sep 16, 2019 at 09:41:43PM -0500, Navid Emamdoost wrote:
> In das1800_attach, the buffer allocated via kmalloc_array needs to be
> released if an error happens.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Commedit calls ->detach() if the ->attach() fails so this patch would
lead to a double free. See comedi_device_attach():
drivers/staging/comedi/drivers.c
983 }
984 if (!driv->attach) {
985 /* driver does not support manual configuration */
986 dev_warn(dev->class_dev,
987 "driver '%s' does not support attach using comedi_config\n",
988 driv->driver_name);
989 module_put(driv->module);
990 ret = -EIO;
991 goto out;
992 }
993 dev->driver = driv;
994 dev->board_name = dev->board_ptr ? *(const char **)dev->board_ptr
995 : dev->driver->driver_name;
996 ret = driv->attach(dev, it);
^^^^^^^^^^^^^^^^^^^^^
997 if (ret >= 0)
998 ret = comedi_device_postconfig(dev);
999 if (ret < 0) {
1000 comedi_device_detach(dev);
^^^^^^^^^^^^^^^^^^^^^^^^^
1001 module_put(driv->module);
1002 }
1003 /* On success, the driver module count has been incremented. */
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: comedi: drivers: prevent memory leak
2019-09-17 6:33 ` Dan Carpenter
@ 2019-09-17 10:13 ` Ian Abbott
0 siblings, 0 replies; 3+ messages in thread
From: Ian Abbott @ 2019-09-17 10:13 UTC (permalink / raw)
To: Dan Carpenter, Navid Emamdoost
Cc: devel, Greg Kroah-Hartman, kjlu, linux-kernel, emamd001, smccaman
On 17/09/2019 07:33, Dan Carpenter wrote:
> On Mon, Sep 16, 2019 at 09:41:43PM -0500, Navid Emamdoost wrote:
>> In das1800_attach, the buffer allocated via kmalloc_array needs to be
>> released if an error happens.
>>
>> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
>
> Commedit calls ->detach() if the ->attach() fails so this patch would
> lead to a double free. See comedi_device_attach():
>
> drivers/staging/comedi/drivers.c
> 983 }
> 984 if (!driv->attach) {
> 985 /* driver does not support manual configuration */
> 986 dev_warn(dev->class_dev,
> 987 "driver '%s' does not support attach using comedi_config\n",
> 988 driv->driver_name);
> 989 module_put(driv->module);
> 990 ret = -EIO;
> 991 goto out;
> 992 }
> 993 dev->driver = driv;
> 994 dev->board_name = dev->board_ptr ? *(const char **)dev->board_ptr
> 995 : dev->driver->driver_name;
> 996 ret = driv->attach(dev, it);
> ^^^^^^^^^^^^^^^^^^^^^
> 997 if (ret >= 0)
> 998 ret = comedi_device_postconfig(dev);
> 999 if (ret < 0) {
> 1000 comedi_device_detach(dev);
> ^^^^^^^^^^^^^^^^^^^^^^^^^
>
> 1001 module_put(driv->module);
> 1002 }
> 1003 /* On success, the driver module count has been incremented. */
Yes, everything should be freed properly by comedi_device_detach().
From comedi_device_detach(), some of the stuff is freed by
dev->driver->detach(), and the remainder is freed by
comedi_device_detach_cleanup().
--
-=( Ian Abbott <abbotti@mev.co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address: )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-17 10:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-09-17 2:41 [PATCH] staging: comedi: drivers: prevent memory leak Navid Emamdoost
2019-09-17 6:33 ` Dan Carpenter
2019-09-17 10:13 ` Ian Abbott
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox