| Message ID | 20200125051134.11557-1-wenwen@cs.uga.edu |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | firestream: fix memory leaks | expand |
From: Wenwen Wang <wenwen@cs.uga.edu> 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.
On Sat, Jan 25, 2020 at 2:11 AM David Miller <davem@davemloft.net> wrote: > > From: Wenwen Wang <wenwen@cs.uga.edu> > 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.
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; } }
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 <wenwen@cs.uga.edu> --- drivers/atm/firestream.c | 2 ++ 1 file changed, 2 insertions(+)