On Tue, 4 Feb 2003, Andy Chou wrote:
> [BUG] Maybe.
> /u1/acc/linux/2.5.48/drivers/scsi/scsi_scan.c:1730:scsi_report_lun_scan:
> ERROR:LEAK:1706:1730:Memory leak [Allocated from:
> /u1/acc/linux/2.5.48/drivers/scsi/scsi_scan.c:1706:scsi_allocate_request]
This one seems fixed in 2.5 current.
> [BUG]
> u1/acc/linux/2.5.48/drivers/scsi/sr_ioctl.c:188:sr_do_ioctl:
> ERROR:LEAK:85:188:Memory leak [Allocated from:
> /u1/acc/linux/2.5.48/drivers/scsi/sr_ioctl.c:85:scsi_allocate_request]
Bug indeed, I've created a patch to fix the possible leak of
a scsi request, but can't figure out the bounce buffer logic...
> [BUG] Leaking tmp_ba
> /u1/acc/linux/2.5.48/drivers/scsi/st.c:3686:st_attach:
> ERROR:LEAK:3704:3686:Memory leak [Allocated from:
> /u1/acc/linux/2.5.48/drivers/scsi/st.c:3704:kmalloc]
Doesn't seem to be present in 2.5 current.
> [BUG] [GEM] The case where __dev_get_by_index() returns 0,
> followed by goto out.
> /u1/acc/linux/2.5.48/net/ipv4/route.c:2229:inet_rtm_getroute:
> ERROR:LEAK:2166:2229:Memory leak [Allocated from:
> /u1/acc/linux/2.5.48/net/ipv4/route.c:2166:alloc_skb]
Confirmed. I'll take a stab at fixing this one.
Rik
--
Bravely reimplemented by the knights who say "NIH".
http://www.surriel.com/ http://guru.conectiva.com/
Current spamtrap: <a href=mailto:"[email protected]">[email protected]</a>
On Wed, 5 Feb 2003, Rik van Riel wrote:
> On Tue, 4 Feb 2003, Andy Chou wrote:
Thanks for the checker output. First patch below...
> > [BUG]
> > u1/acc/linux/2.5.48/drivers/scsi/sr_ioctl.c:188:sr_do_ioctl:
> > ERROR:LEAK:85:188:Memory leak [Allocated from:
> > /u1/acc/linux/2.5.48/drivers/scsi/sr_ioctl.c:85:scsi_allocate_request]
>
> Bug indeed, I've created a patch to fix the possible leak of
> a scsi request, but can't figure out the bounce buffer logic...
The patch below fixes the scsi request leak. I'm not sure
how the bounce buffer thing is supposed to work (Christoph?
James?) so I'm not touching that at the moment.
Linus, could you please apply this patch (against today's
bk tree) ?
thank you,
Rik
--
Bravely reimplemented by the knights who say "NIH".
http://www.surriel.com/ http://guru.conectiva.com/
Current spamtrap: <a href=mailto:"[email protected]">[email protected]</a>
===== drivers/scsi/sr_ioctl.c 1.27 vs edited =====
--- 1.27/drivers/scsi/sr_ioctl.c Sat Jan 4 14:21:59 2003
+++ edited/drivers/scsi/sr_ioctl.c Tue Feb 4 23:37:02 2003
@@ -99,7 +99,7 @@
if (bounce_buffer == NULL) {
printk("SCSI DMA pool exhausted.");
err = -ENOMEM;
- goto out;
+ goto out_free;
}
memcpy(bounce_buffer, cgc->buffer, cgc->buflen);
cgc->buffer = bounce_buffer;
@@ -107,7 +107,7 @@
retry:
if (!scsi_block_when_processing_errors(SDev)) {
err = -ENODEV;
- goto out;
+ goto out_free;
}
scsi_wait_req(SRpnt, cgc->cmd, cgc->buffer, cgc->buflen,
@@ -179,6 +179,7 @@
memcpy(cgc->sense, SRpnt->sr_sense_buffer, sizeof(*cgc->sense));
/* Wake up a process waiting for device */
+ out_free:
scsi_release_request(SRpnt);
SRpnt = NULL;
out:
Hi,
> /u1/acc/linux/2.5.48/drivers/net/tokenring/madgemc.c:356:madgemc_probe:
> ERROR:LEAK:194:356:Memory leak [Allocated from:
> /u1/acc/linux/2.5.48/drivers/net/tokenring/madgemc.c:194:kmalloc]
This should fix the leaks (and updates the driver to use propper
reference counting).
--jochen
===== madgemc.c 1.9 vs edited =====
--- 1.9/drivers/net/tokenring/madgemc.c Thu Nov 21 18:59:53 2002
+++ edited/madgemc.c Wed Feb 5 08:16:32 2003
@@ -180,8 +180,11 @@
if ((dev = init_trdev(NULL, 0))==NULL) {
printk("madgemc: unable to allocate dev space\n");
+ if (madgemc_card_list)
+ return 0;
return -1;
}
+ SET_MODULE_OWNER(dev);
dev->dma = 0;
/*
@@ -193,6 +196,9 @@
card = kmalloc(sizeof(struct madgemc_card), GFP_KERNEL);
if (card==NULL) {
printk("madgemc: unable to allocate card struct\n");
+ kfree(dev); /* release_trdev? */
+ if (madgemc_card_list)
+ return 0;
return -1;
}
card->dev = dev;
@@ -223,14 +229,14 @@
if (dev->irq == 0) {
printk("%s: invalid IRQ\n", dev->name);
- goto getout;
+ goto getout1;
}
if (!request_region(dev->base_addr, MADGEMC_IO_EXTENT,
"madgemc")) {
printk(KERN_INFO "madgemc: unable to setup Smart MC in slot %d because of I/O base conflict at 0x%04lx\n", slot, dev->base_addr);
dev->base_addr += MADGEMC_SIF_OFFSET;
- goto getout;
+ goto getout1;
}
dev->base_addr += MADGEMC_SIF_OFFSET;
@@ -348,6 +354,14 @@
if (tmsdev_init(dev, ISA_MAX_ADDRESS, NULL)) {
printk("%s: unable to get memory for dev->priv.\n",
dev->name);
+ release_region(dev->base_addr-MADGEMC_SIF_OFFSET,
+ MADGEMC_IO_EXTENT);
+
+ kfree(card);
+ tmsdev_term(dev);
+ kfree(dev);
+ if (madgemc_card_list)
+ return 0;
return -1;
}
tp = (struct net_local *)dev->priv;
@@ -376,10 +390,14 @@
madgemc_card_list = card;
} else {
printk("madgemc: register_trdev() returned non-zero.\n");
+ release_region(dev->base_addr-MADGEMC_SIF_OFFSET,
+ MADGEMC_IO_EXTENT);
kfree(card);
tmsdev_term(dev);
kfree(dev);
+ if (madgemc_card_list)
+ return 0;
return -1;
}
@@ -389,6 +407,7 @@
getout:
release_region(dev->base_addr-MADGEMC_SIF_OFFSET,
MADGEMC_IO_EXTENT);
+ getout1:
kfree(card);
kfree(dev); /* release_trdev? */
slot++;
@@ -696,7 +715,6 @@
*/
madgemc_chipset_init(dev);
tms380tr_open(dev);
- MOD_INC_USE_COUNT;
return 0;
}
@@ -704,7 +722,6 @@
{
tms380tr_close(dev);
madgemc_chipset_close(dev);
- MOD_DEC_USE_COUNT;
return 0;
}
On Tue, 4 Feb 2003, Andy Chou wrote:
> [BUG] /u1/acc/linux/2.5.48/net/ipv6/route.c:1583:inet6_rtm_getroute:
> ERROR:LEAK:1556:1583:Memory leak
Here's the ipv6 fix.
- James
--
James Morris
<[email protected]>
diff -urN -X dontdiff linux-2.5.59.orig/net/ipv6/route.c linux-2.5.59.w1/net/ipv6/route.c
--- linux-2.5.59.orig/net/ipv6/route.c Tue Nov 12 00:12:07 2002
+++ linux-2.5.59.w1/net/ipv6/route.c Wed Feb 5 23:00:17 2003
@@ -1548,14 +1548,14 @@
{
struct rtattr **rta = arg;
int iif = 0;
- int err;
+ int err = -ENOBUFS;
struct sk_buff *skb;
struct flowi fl;
struct rt6_info *rt;
skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
if (skb == NULL)
- return -ENOBUFS;
+ goto out;
/* Reserve room for dummy headers, this skb can pass
through good chunk of routing engine.
@@ -1579,8 +1579,10 @@
if (iif) {
struct net_device *dev;
dev = __dev_get_by_index(iif);
- if (!dev)
- return -ENODEV;
+ if (!dev) {
+ err = -ENODEV;
+ goto out_free;
+ }
}
fl.oif = 0;
@@ -1597,13 +1599,19 @@
fl.nl_u.ip6_u.saddr,
iif,
RTM_NEWROUTE, NETLINK_CB(in_skb).pid, nlh->nlmsg_seq);
- if (err < 0)
- return -EMSGSIZE;
+ if (err < 0) {
+ err = -EMSGSIZE;
+ goto out_free;
+ }
err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
- if (err < 0)
- return err;
- return 0;
+ if (err > 0)
+ err = 0;
+out:
+ return err;
+out_free:
+ kfree_skb(skb);
+ goto out;
}
void inet6_rt_notify(int event, struct rt6_info *rt)
On Tue, 4 Feb 2003, Andy Chou wrote:
> [BUG] [GEM] The case where __dev_get_by_index() returns 0, followed by goto out.
> /u1/acc/linux/2.5.48/net/ipv4/route.c:2229:inet_rtm_getroute:
> ERROR:LEAK:2166:2229:Memory leak
Here's a fix for 2.5.59.
- James
--
James Morris
<[email protected]>
diff -urN -X dontdiff linux-2.5.59.orig/net/ipv4/route.c linux-2.5.59.w1/net/ipv4/route.c
--- linux-2.5.59.orig/net/ipv4/route.c Thu Jan 16 22:51:35 2003
+++ linux-2.5.59.w1/net/ipv4/route.c Wed Feb 5 22:51:48 2003
@@ -2288,7 +2288,7 @@
struct net_device *dev = __dev_get_by_index(iif);
err = -ENODEV;
if (!dev)
- goto out;
+ goto out_free;
skb->protocol = htons(ETH_P_IP);
skb->dev = dev;
local_bh_disable();
@@ -2307,10 +2307,8 @@
fl.oif = oif;
err = ip_route_output_key(&rt, &fl);
}
- if (err) {
- kfree_skb(skb);
- goto out;
- }
+ if (err)
+ goto out_free;
skb->dst = &rt->u.dst;
if (rtm->rtm_flags & RTM_F_NOTIFY)
@@ -2321,16 +2319,20 @@
err = rt_fill_info(skb, NETLINK_CB(in_skb).pid, nlh->nlmsg_seq,
RTM_NEWROUTE, 0);
if (!err)
- goto out;
+ goto out_free;
if (err < 0) {
err = -EMSGSIZE;
- goto out;
+ goto out_free;
}
err = netlink_unicast(rtnl, skb, NETLINK_CB(in_skb).pid, MSG_DONTWAIT);
if (err > 0)
err = 0;
out: return err;
+
+out_free:
+ kfree_skb(skb);
+ goto out;
}
int ip_rt_dump(struct sk_buff *skb, struct netlink_callback *cb)
On Tue, 2003-02-04 at 20:42, Rik van Riel wrote:
> The patch below fixes the scsi request leak. I'm not sure
> how the bounce buffer thing is supposed to work (Christoph?
> James?) so I'm not touching that at the moment.
The patch below should fix all the bounce buffer problems (including the
stack allocation of a DMA region, the missing kfree and use of
virt_to_phys).
> Linus, could you please apply this patch (against today's
> bk tree) ?
I've captured both patches in the scsi-misc-2.5 BK tree.
James
===== ./sr_ioctl.c 1.28 vs edited =====
--- 1.28/drivers/scsi/sr_ioctl.c Tue Feb 4 17:37:02 2003
+++ edited/./sr_ioctl.c Wed Feb 5 10:18:19 2003
@@ -80,30 +80,16 @@
struct scsi_device *SDev;
struct request *req;
int result, err = 0, retries = 0;
- char *bounce_buffer;
SDev = cd->device;
SRpnt = scsi_allocate_request(SDev);
if (!SRpnt) {
- printk("Unable to allocate SCSI request in sr_do_ioctl");
+ printk(KERN_ERR "Unable to allocate SCSI request in sr_do_ioctl");
err = -ENOMEM;
goto out;
}
SRpnt->sr_data_direction = cgc->data_direction;
- /* use ISA DMA buffer if necessary */
- SRpnt->sr_request->buffer = cgc->buffer;
- if (cgc->buffer && SRpnt->sr_host->unchecked_isa_dma &&
- (virt_to_phys(cgc->buffer) + cgc->buflen - 1 > ISA_DMA_THRESHOLD)) {
- bounce_buffer = (char *) kmalloc(cgc->buflen, GFP_DMA);
- if (bounce_buffer == NULL) {
- printk("SCSI DMA pool exhausted.");
- err = -ENOMEM;
- goto out_free;
- }
- memcpy(bounce_buffer, cgc->buffer, cgc->buflen);
- cgc->buffer = bounce_buffer;
- }
retry:
if (!scsi_block_when_processing_errors(SDev)) {
err = -ENODEV;
@@ -276,11 +262,15 @@
return 0;
}
+/* primitive to determine whether we need to have GFP_DMA set based on
+ * the status of the unchecked_isa_dma flag in the host structure */
+#define SR_GFP_DMA(cd) (((cd)->device->host->unchecked_isa_dma) ? GFP_DMA : 0)
+
int sr_get_mcn(struct cdrom_device_info *cdi, struct cdrom_mcn *mcn)
{
Scsi_CD *cd = cdi->handle;
struct cdrom_generic_command cgc;
- char buffer[32];
+ char *buffer = kmalloc(32, GFP_KERNEL | SR_GFP_DMA(cd));
int result;
memset(&cgc, 0, sizeof(struct cdrom_generic_command));
@@ -297,6 +287,7 @@
memcpy(mcn->medium_catalog_number, buffer + 9, 13);
mcn->medium_catalog_number[13] = 0;
+ kfree(buffer);
return result;
}
@@ -338,7 +329,7 @@
Scsi_CD *cd = cdi->handle;
struct cdrom_generic_command cgc;
int result;
- unsigned char buffer[32];
+ unsigned char *buffer = kmalloc(32, GFP_KERNEL | SR_GFP_DMA(cd));
memset(&cgc, 0, sizeof(struct cdrom_generic_command));
cgc.timeout = IOCTL_TIMEOUT;
@@ -409,7 +400,7 @@
}
default:
- return -EINVAL;
+ result = -EINVAL;
}
#if 0
@@ -417,6 +408,7 @@
printk("DEBUG: sr_audio: result for ioctl %x: %x\n", cmd, result);
#endif
+ kfree(buffer);
return result;
}
@@ -528,7 +520,7 @@
if (!xa_test)
return 0;
- raw_sector = (unsigned char *) kmalloc(2048, GFP_DMA | GFP_KERNEL);
+ raw_sector = (unsigned char *) kmalloc(2048, GFP_KERNEL | SR_GFP_DMA(cd));
if (!raw_sector)
return -ENOMEM;
if (0 == sr_read_sector(cd, cd->ms_offset + 16,
[BUG] Only possible if the argument len == 0.
/u1/acc/linux/2.5.48/fs/xfs/pagebuf/page_buf.c:966:pagebuf_get_no_daddr: ERROR:LEAK:966:966:Memory leak [Allocated from: /u1/acc/linux/2.5.48/fs/xfs/pagebuf/page_buf.c:966:kmalloc]
This function is never called with len == 0
[BUG] Not sure. Code path is complicated.
/u1/acc/linux/2.5.48/fs/xfs/xfs_da_btree.c:2221:xfs_da_do_buf: ERROR:LEAK:2179:2221:Memory leak [Allocated from: /u1/acc/linux/2.5.48/fs/xfs/xfs_da_btree.c:2179:xfs_da_buf_make]
The code makes me shudder, but there is no leak in here, I chased it
around and all the callers use the right set of arguments to not leak.
[BUG]
/u1/acc/linux/2.5.48/fs/xfs/xfs_dir_leaf.c:651:xfs_dir_leaf_to_shortform: ERROR:LEAK:645:651:Memory leak [Allocated from: /u1/acc/linux/2.5.48/fs/xfs/xfs_dir_leaf.c:645:kmem_alloc]
yep, that's a bug, only happens in forced shutdown of the filesystem.
[BUG]
/u1/acc/linux/2.5.48/fs/xfs/xfs_log_recover.c:1304:xlog_recover_add_to_trans: ERROR:LEAK:1294:1304:Memory leak [Allocated from: /u1/acc/linux/2.5.48/fs/xfs/xfs_log_recover.c:1294:kmem_zalloc]
Also a bug, and we don't need to zero the memory either.
Fixes for the last two will be in the pipeline with other xfs fixes to
Linus.
Thanks
Steve
--
Steve Lord voice: +1-651-683-3511
Principal Engineer, Filesystem Software email: [email protected]
On Tue, Feb 04, 2003 at 05:13:53PM -0800, Andy Chou wrote:
> 7 | drivers/hotplug/cpqphp_nvram.c
These were all real problems (and you missed one at the end, there was
two places bus_node could be leaked like the other ones) and I've fixed
them in my trees.
thanks,
greg k-h
Thanks for the confirmation. The checker may skip reporting some errors
if there are already other errors reported for a function. But if only
one error is fixed, the other one will be "unmasked" and the checker will
warn about it.
-Andy
On Wed, Feb 05, 2003 at 03:48:29PM -0800, Greg KH wrote:
> On Tue, Feb 04, 2003 at 05:13:53PM -0800, Andy Chou wrote:
> > 7 | drivers/hotplug/cpqphp_nvram.c
>
> These were all real problems (and you missed one at the end, there was
> two places bus_node could be leaked like the other ones) and I've fixed
> them in my trees.
>
> thanks,
>
> greg k-h
On Tue, Feb 04, 2003 at 05:13:53PM -0800, Andy Chou wrote:
> 1 | drivers/hotplug/ibmphp_pci.c
Real problem, fixed in my trees now.
thanks,
greg k-h
On Tue, Feb 04, 2003 at 05:13:53PM -0800, Andy Chou wrote:
> 14 | drivers/hotplug/ibmphp_ebda.c
Look, I'm number 1! :(
{sigh} Yes all of these are bugs, I've reworked a lot of this code to
fix all of these problems. A bunch of these were also found by the
smatch program, but I hadn't sent a patch for them yet.
Thanks for the report, I appreciate it.
greg k-h
From: James Morris <[email protected]>
Date: Wed, 5 Feb 2003 23:20:08 +1100 (EST)
Here's the ipv6 fix.
Applied, and like the ipv4 fix I also backported it to 2.4.x
From: James Morris <[email protected]>
Date: Wed, 5 Feb 2003 23:19:14 +1100 (EST)
Here's a fix for 2.5.59.
Applied, and backported to 2.4.x as the bug is there too.