* [PATCH] staging: ft1000: fix skb, netdev, memory leaks
@ 2010-09-26 8:59 Vasiliy Kulikov
0 siblings, 0 replies; 5+ messages in thread
From: Vasiliy Kulikov @ 2010-09-26 8:59 UTC (permalink / raw)
To: kernel-janitors; +Cc: Greg Kroah-Hartman, Marek Belisko, devel, linux-kernel
ft1000_copy_up_pkt() doesn't free skb on errors.
init_ft1000_card() doesn't free netdev with free_netdev() but with kfree().
init_ft1000_card() doesn't check request_region()'s return value
and doesn't free region on error.
---
Compile tested.
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 32 +++++++++++++--------
1 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
index a6ba84c..eed7e94 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
@@ -1686,6 +1686,7 @@ int ft1000_copy_up_pkt(struct net_device *dev)
tempword);
ft1000_flush_fifo(dev, DSP_PKTPHCKSUM_INFO);
info->stats.rx_errors++;
+ kfree_skb(skb);
return FAILURE;
}
//subtract the number of bytes read already
@@ -1711,6 +1712,7 @@ int ft1000_copy_up_pkt(struct net_device *dev)
*pbuffer++ = (u8) (tempword >> 8);
*pbuffer++ = (u8) tempword;
if (ft1000_chkcard(dev) == FALSE) {
+ kfree_skb(skb);
return FAILURE;
}
}
@@ -2236,15 +2238,17 @@ struct net_device *init_ft1000_card(unsigned short irq, int port,
if (request_irq(dev->irq, ft1000_interrupt, IRQF_SHARED, dev->name, dev)) {
printk(KERN_ERR "ft1000: Could not request_irq\n");
- kfree(dev);
- return (NULL);
+ goto err_dev;
}
- request_region(dev->base_addr, 256, dev->name);
+ if (request_region(dev->base_addr, 256, dev->name) == NULL) {
+ printk(KERN_ERR "ft1000: Could not request_region\n");
+ goto err_irq;
+ }
if (register_netdev(dev) != 0) {
DEBUG(0, "ft1000: Could not register netdev");
- return NULL;
+ goto err_reg;
}
info->AsicID = ft1000_read_reg(dev, FT1000_REG_ASIC_ID);
@@ -2252,19 +2256,13 @@ struct net_device *init_ft1000_card(unsigned short irq, int port,
DEBUG(0, "ft1000_hw: ELECTRABUZZ ASIC\n");
if (request_firmware(&fw_entry, "ft1000.img", fdev) != 0) {
printk(KERN_INFO "ft1000: Could not open ft1000.img\n");
- unregister_netdev(dev);
- free_irq(dev->irq, dev);
- kfree(dev);
- return NULL;
+ goto err_unreg;
}
} else {
DEBUG(0, "ft1000_hw: MAGNEMITE ASIC\n");
if (request_firmware(&fw_entry, "ft2000.img", fdev) != 0) {
printk(KERN_INFO "ft1000: Could not open ft2000.img\n");
- unregister_netdev(dev);
- free_irq(dev->irq, dev);
- kfree(dev);
- return NULL;
+ goto err_unreg;
}
}
@@ -2279,6 +2277,16 @@ struct net_device *init_ft1000_card(unsigned short irq, int port,
dev->dev_addr[1], dev->dev_addr[2], dev->dev_addr[3],
dev->dev_addr[4], dev->dev_addr[5]);
return dev;
+
+err_unreg:
+ unregister_netdev(dev);
+err_reg:
+ release_region(dev->base_addr, 256);
+err_irq:
+ free_irq(dev->irq, dev);
+err_dev:
+ free_netdev(dev);
+ return NULL;
}
EXPORT_SYMBOL(init_ft1000_card);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] staging: ft1000: fix skb, netdev, memory leaks
@ 2010-09-28 17:49 Vasiliy Kulikov
2010-09-30 11:13 ` Greg KH
0 siblings, 1 reply; 5+ messages in thread
From: Vasiliy Kulikov @ 2010-09-28 17:49 UTC (permalink / raw)
To: kernel-janitors; +Cc: Greg Kroah-Hartman, Marek Belisko, devel, linux-kernel
ft1000_copy_up_pkt() doesn't free skb on errors.
init_ft1000_card() doesn't free netdev with free_netdev() but with kfree().
init_ft1000_card() doesn't check request_region()'s return value
and doesn't free region on error.
Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
---
Compile tested.
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 32 +++++++++++++--------
1 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
index a6ba84c..eed7e94 100644
--- a/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
+++ b/drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c
@@ -1686,6 +1686,7 @@ int ft1000_copy_up_pkt(struct net_device *dev)
tempword);
ft1000_flush_fifo(dev, DSP_PKTPHCKSUM_INFO);
info->stats.rx_errors++;
+ kfree_skb(skb);
return FAILURE;
}
//subtract the number of bytes read already
@@ -1711,6 +1712,7 @@ int ft1000_copy_up_pkt(struct net_device *dev)
*pbuffer++ = (u8) (tempword >> 8);
*pbuffer++ = (u8) tempword;
if (ft1000_chkcard(dev) == FALSE) {
+ kfree_skb(skb);
return FAILURE;
}
}
@@ -2236,15 +2238,17 @@ struct net_device *init_ft1000_card(unsigned short irq, int port,
if (request_irq(dev->irq, ft1000_interrupt, IRQF_SHARED, dev->name, dev)) {
printk(KERN_ERR "ft1000: Could not request_irq\n");
- kfree(dev);
- return (NULL);
+ goto err_dev;
}
- request_region(dev->base_addr, 256, dev->name);
+ if (request_region(dev->base_addr, 256, dev->name) == NULL) {
+ printk(KERN_ERR "ft1000: Could not request_region\n");
+ goto err_irq;
+ }
if (register_netdev(dev) != 0) {
DEBUG(0, "ft1000: Could not register netdev");
- return NULL;
+ goto err_reg;
}
info->AsicID = ft1000_read_reg(dev, FT1000_REG_ASIC_ID);
@@ -2252,19 +2256,13 @@ struct net_device *init_ft1000_card(unsigned short irq, int port,
DEBUG(0, "ft1000_hw: ELECTRABUZZ ASIC\n");
if (request_firmware(&fw_entry, "ft1000.img", fdev) != 0) {
printk(KERN_INFO "ft1000: Could not open ft1000.img\n");
- unregister_netdev(dev);
- free_irq(dev->irq, dev);
- kfree(dev);
- return NULL;
+ goto err_unreg;
}
} else {
DEBUG(0, "ft1000_hw: MAGNEMITE ASIC\n");
if (request_firmware(&fw_entry, "ft2000.img", fdev) != 0) {
printk(KERN_INFO "ft1000: Could not open ft2000.img\n");
- unregister_netdev(dev);
- free_irq(dev->irq, dev);
- kfree(dev);
- return NULL;
+ goto err_unreg;
}
}
@@ -2279,6 +2277,16 @@ struct net_device *init_ft1000_card(unsigned short irq, int port,
dev->dev_addr[1], dev->dev_addr[2], dev->dev_addr[3],
dev->dev_addr[4], dev->dev_addr[5]);
return dev;
+
+err_unreg:
+ unregister_netdev(dev);
+err_reg:
+ release_region(dev->base_addr, 256);
+err_irq:
+ free_irq(dev->irq, dev);
+err_dev:
+ free_netdev(dev);
+ return NULL;
}
EXPORT_SYMBOL(init_ft1000_card);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: ft1000: fix skb, netdev, memory leaks
2010-09-28 17:49 [PATCH] staging: ft1000: fix skb, netdev, memory leaks Vasiliy Kulikov
@ 2010-09-30 11:13 ` Greg KH
2010-09-30 19:23 ` Vasiliy Kulikov
0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2010-09-30 11:13 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: kernel-janitors, devel, Marek Belisko, Greg Kroah-Hartman,
linux-kernel
On Tue, Sep 28, 2010 at 09:49:35PM +0400, Vasiliy Kulikov wrote:
> ft1000_copy_up_pkt() doesn't free skb on errors.
> init_ft1000_card() doesn't free netdev with free_netdev() but with kfree().
> init_ft1000_card() doesn't check request_region()'s return value
> and doesn't free region on error.
>
> Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
> ---
> Compile tested.
No you didn't
> drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 32 +++++++++++++--------
> 1 files changed, 20 insertions(+), 12 deletions(-)
This file doesn't build at this point in time, how did you build test
this?
I don't mind taking the patch, but please, don't lie.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: ft1000: fix skb, netdev, memory leaks
2010-09-30 11:13 ` Greg KH
@ 2010-09-30 19:23 ` Vasiliy Kulikov
2010-09-30 19:53 ` Greg KH
0 siblings, 1 reply; 5+ messages in thread
From: Vasiliy Kulikov @ 2010-09-30 19:23 UTC (permalink / raw)
To: Greg KH
Cc: kernel-janitors, devel, Marek Belisko, Greg Kroah-Hartman,
linux-kernel
On Thu, Sep 30, 2010 at 04:13 -0700, Greg KH wrote:
> On Tue, Sep 28, 2010 at 09:49:35PM +0400, Vasiliy Kulikov wrote:
> > ft1000_copy_up_pkt() doesn't free skb on errors.
> > init_ft1000_card() doesn't free netdev with free_netdev() but with kfree().
> > init_ft1000_card() doesn't check request_region()'s return value
> > and doesn't free region on error.
> >
> > Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
> > ---
> > Compile tested.
>
> No you didn't
No I did:
make -C /home/vasya/linux drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.o
make: Вход в каталог `/home/vasya/dev/linux-next'
CHK include/linux/version.h
CHK include/generated/utsrelease.h
CALL scripts/checksyscalls.sh
CC drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.o
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c: In function ‘ft1000_reset_card’:
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c:525: warning: passing argument 2 of ‘card_download’ discards qualifiers from pointer target type
drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c:56: note: expected ‘void *’ but argument is of type ‘const u8 * const’
make: Выход из каталога `/home/vasya/dev/linux-next'
However,
cd ../drivers/staging/ft1000/ft1000-pcmcia/
LANG=C make -C ~/linux ft1000_hw.o
make: Entering directory `/home/vasya/dev/linux-next'
make: *** No rule to make target `ft1000_hw.o'. Stop.
make: Leaving directory `/home/vasya/dev/linux-next'
Is "make" from linux root buggy/not supported for staging/everything?
>
> > drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c | 32 +++++++++++++--------
> > 1 files changed, 20 insertions(+), 12 deletions(-)
>
> This file doesn't build at this point in time, how did you build test
> this?
>
> I don't mind taking the patch, but please, don't lie.
>
> thanks,
>
> greg k-h
--
Vasiliy
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: ft1000: fix skb, netdev, memory leaks
2010-09-30 19:23 ` Vasiliy Kulikov
@ 2010-09-30 19:53 ` Greg KH
0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2010-09-30 19:53 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: Greg KH, kernel-janitors, devel, Marek Belisko, linux-kernel
On Thu, Sep 30, 2010 at 11:23:03PM +0400, Vasiliy Kulikov wrote:
> On Thu, Sep 30, 2010 at 04:13 -0700, Greg KH wrote:
> > On Tue, Sep 28, 2010 at 09:49:35PM +0400, Vasiliy Kulikov wrote:
> > > ft1000_copy_up_pkt() doesn't free skb on errors.
> > > init_ft1000_card() doesn't free netdev with free_netdev() but with kfree().
> > > init_ft1000_card() doesn't check request_region()'s return value
> > > and doesn't free region on error.
> > >
> > > Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
> > > ---
> > > Compile tested.
> >
> > No you didn't
>
> No I did:
>
> make -C /home/vasya/linux drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.o
> make: Вход в каталог `/home/vasya/dev/linux-next'
> CHK include/linux/version.h
> CHK include/generated/utsrelease.h
> CALL scripts/checksyscalls.sh
> CC drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.o
> drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c: In function ‘ft1000_reset_card’:
> drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c:525: warning: passing argument 2 of ‘card_download’ discards qualifiers from pointer target type
> drivers/staging/ft1000/ft1000-pcmcia/ft1000_hw.c:56: note: expected ‘void *’ but argument is of type ‘const u8 * const’
> make: Выход из каталога `/home/vasya/dev/linux-next'
Heh, I didn't think to try that, amazed that it worked at all.
> However,
>
> cd ../drivers/staging/ft1000/ft1000-pcmcia/
> LANG=C make -C ~/linux ft1000_hw.o
> make: Entering directory `/home/vasya/dev/linux-next'
> make: *** No rule to make target `ft1000_hw.o'. Stop.
> make: Leaving directory `/home/vasya/dev/linux-next'
>
>
> Is "make" from linux root buggy/not supported for staging/everything?
This driver is marked broken, look at the Kconfig file:
config FT1000_PCMCIA
tristate "Driver for ft1000 pcmcia device."
depends on PCMCIA && BROKEN
It will be fixed up soon hopefully.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-09-30 20:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-28 17:49 [PATCH] staging: ft1000: fix skb, netdev, memory leaks Vasiliy Kulikov
2010-09-30 11:13 ` Greg KH
2010-09-30 19:23 ` Vasiliy Kulikov
2010-09-30 19:53 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2010-09-26 8:59 Vasiliy Kulikov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox