2020-01-25 05:13:48

by Wenwen Wang

Subject: [PATCH] firestream: fix memory leaks

In fs_open(), 'vcc' is allocated through kmalloc() and assigned to
'atm_vcc->dev_data.' In the following execution, if there is no more free
channel, the error code EBUSY will be returned. However, 'vcc' is not
deallocated, leading to memory leaks. Note that, in normal cases where
fs_open() returns 0, 'vcc' will be deallocated in fs_close(). But, if
fs_open() fails, there is no guarantee that fs_close() will be invoked.

To fix this issue, deallocate 'vcc' before EBUSY is returned.

Signed-off-by: Wenwen Wang <[email protected]>
---
drivers/atm/firestream.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/atm/firestream.c b/drivers/atm/firestream.c
index aad00d2b28f5..093712e34de7 100644
--- a/drivers/atm/firestream.c
+++ b/drivers/atm/firestream.c
@@ -912,6 +912,7 @@ static int fs_open(struct atm_vcc *atm_vcc)
}
if (!to) {
printk ("No more free channels for FS50..\n");
+ kfree(vcc);
return -EBUSY;
}
vcc->channo = dev->channo;
@@ -922,6 +923,7 @@ static int fs_open(struct atm_vcc *atm_vcc)
if (((DO_DIRECTION(rxtp) && dev->atm_vccs[vcc->channo])) ||
( DO_DIRECTION(txtp) && test_bit (vcc->channo, dev->tx_inuse))) {
printk ("Channel is in use for FS155.\n");
+ kfree(vcc);
return -EBUSY;
}
}
--
2.17.1

2020-01-25 07:15:42

by David Miller

Subject: Re: [PATCH] firestream: fix memory leaks

From: Wenwen Wang <[email protected]>
Date: Sat, 25 Jan 2020 05:11:34 +0000

> @@ -922,6 +923,7 @@ static int fs_open(struct atm_vcc *atm_vcc)
> if (((DO_DIRECTION(rxtp) && dev->atm_vccs[vcc->channo])) ||
> ( DO_DIRECTION(txtp) && test_bit (vcc->channo, dev->tx_inuse))) {
> printk ("Channel is in use for FS155.\n");
> + kfree(vcc);
> return -EBUSY;
> }
> }
> --

There is another case just below this line:

tc, sizeof (struct fs_transmit_config));
if (!tc) {
fs_dprintk (FS_DEBUG_OPEN, "fs: can't alloc transmit_config.\n");
return -ENOMEM;
}

Please audit the entire function and make sure your patch fixes all of these
missing vcc free cases.

Thank you.

2020-01-25 14:36:31

by Wenwen Wang

Subject: Re: [PATCH] firestream: fix memory leaks

On Sat, Jan 25, 2020 at 2:11 AM David Miller <[email protected]> wrote:
>
> From: Wenwen Wang <[email protected]>
> Date: Sat, 25 Jan 2020 05:11:34 +0000
>
> > @@ -922,6 +923,7 @@ static int fs_open(struct atm_vcc *atm_vcc)
> > if (((DO_DIRECTION(rxtp) && dev->atm_vccs[vcc->channo])) ||
> > ( DO_DIRECTION(txtp) && test_bit (vcc->channo, dev->tx_inuse))) {
> > printk ("Channel is in use for FS155.\n");
> > + kfree(vcc);
> > return -EBUSY;
> > }
> > }
> > --
>
> There is another case just below this line:
>
> tc, sizeof (struct fs_transmit_config));
> if (!tc) {
> fs_dprintk (FS_DEBUG_OPEN, "fs: can't alloc transmit_config.\n");
> return -ENOMEM;
> }
>
> Please audit the entire function and make sure your patch fixes all of these
> missing vcc free cases.

Thanks for your comment! I was planning to submit another patch as
this case has different semantics. But, since you have pointed out, I
will revise this patch.

Wenwen

>
> Thank you.