* Re: Fw: [Kernel-janitors] net/atm/clip.c: check kmem_cache_create() #1
[not found] <20040128091803.1da4cf1c.rddunlap@osdl.org>
@ 2004-02-04 19:04 ` chas williams
2004-02-04 21:47 ` Francois Romieu
2004-02-05 7:24 ` David S. Miller
0 siblings, 2 replies; 6+ messages in thread
From: chas williams @ 2004-02-04 19:04 UTC (permalink / raw)
To: davem, Randy.Dunlap; +Cc: netdev
randy, i think it should probably return -ENOMEM instead of -1.
dave, please apply the following to both the 2.6 and 2.4 trees.
thanks!
# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.1548 -> 1.1549
# net/atm/clip.c 1.29 -> 1.30
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/02/04 chas@relax.cmf.nrl.navy.mil 1.1549
# [ATM]: [clip] check return code from kmem_cache_create (by "Randy.Dunlap" <rddunlap@osdl.org>)
# --------------------------------------------
#
diff -Nru a/net/atm/clip.c b/net/atm/clip.c
--- a/net/atm/clip.c Wed Feb 4 13:24:23 2004
+++ b/net/atm/clip.c Wed Feb 4 13:24:23 2004
@@ -1021,6 +1021,9 @@
clip_tbl.kmem_cachep = kmem_cache_create(clip_tbl.id,
clip_tbl.entry_size, 0, SLAB_HWCACHE_ALIGN, NULL, NULL);
+ if (!clip_tbl.kmem_cachep)
+ return -ENOMEM;
+
/* so neigh_ifdown() doesn't complain */
clip_tbl.proxy_timer.data = 0;
clip_tbl.proxy_timer.function = 0;
In message <20040128091803.1da4cf1c.rddunlap@osdl.org>,"Randy.Dunlap" writes:
>
>Hi Chas-
>
>What do you think of this patch?
>
>ISTM that the result of kmem_cache_create() should be checked,
>and this patch looks OK to me for a loadable module (it will
>fail loading). But when this code is in the kernel image,
>will the rest of the code that depends on this kmem_cache_create()
>be OK, or will it try to use the kmem_cachep even though it
>is NULL?
>
>Thanks,
>~Randy
>~~~~~~~~~~~~~~~~~~~~~~~
>
>
>Begin forwarded message:
>
>Date: Sun, 25 Jan 2004 15:07:41 +0100
>From: <WHarms@bfs.de>(Walter Harms)
>To: <kernel-janitors@lists.osdl.org>
>Subject: [Kernel-janitors] net/atm/clip.c: check kmem_cache_create() #1
>
>
>
>
>--- net/atm/clip.c.org 2004-01-24 12:22:02.691771888 +0100
>+++ net/atm/clip.c 2004-01-24 12:27:57.074897464 +0100
>@@ -1026,6 +1026,9 @@
> clip_tbl.kmem_cachep = kmem_cache_create(clip_tbl.id,
> clip_tbl.entry_size, 0, SLAB_HWCACHE_ALIGN, NULL, NULL);
>
>+ if (!clip_tbl.kmem_cachep)
>+ return -1;
>+
> /* so neigh_ifdown() doesn't complain */
> clip_tbl.proxy_timer.data = 0;
> clip_tbl.proxy_timer.function = 0;
>_______________________________________________
>Kernel-janitors mailing list
>Kernel-janitors@lists.osdl.org
>http://lists.osdl.org/mailman/listinfo/kernel-janitors
>
>
>--
>~Randy
>kernel-janitors project: http://janitor.kernelnewbies.org/
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fw: [Kernel-janitors] net/atm/clip.c: check kmem_cache_create() #1
2004-02-04 19:04 ` Fw: [Kernel-janitors] net/atm/clip.c: check kmem_cache_create() #1 chas williams
@ 2004-02-04 21:47 ` Francois Romieu
2004-02-05 12:22 ` chas williams
2004-02-05 7:24 ` David S. Miller
1 sibling, 1 reply; 6+ messages in thread
From: Francois Romieu @ 2004-02-04 21:47 UTC (permalink / raw)
To: chas williams; +Cc: davem, Randy.Dunlap, netdev
chas williams <chas@cmf.nrl.navy.mil> :
> randy, i think it should probably return -ENOMEM instead of -1.
One should probably apply the following patch on top of it btw.
Unbalanced call to create_proc_entry() on error path.
net/atm/clip.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletion(-)
diff -puN net/atm/clip.c~clip-procfs-leak net/atm/clip.c
--- linux-2.6.2-rc3/net/atm/clip.c~clip-procfs-leak 2004-02-04 22:41:42.000000000 +0100
+++ linux-2.6.2-rc3-fr/net/atm/clip.c 2004-02-04 22:43:33.000000000 +0100
@@ -1021,8 +1021,10 @@ static int __init atm_clip_init(void)
clip_tbl.kmem_cachep = kmem_cache_create(clip_tbl.id,
clip_tbl.entry_size, 0, SLAB_HWCACHE_ALIGN, NULL, NULL);
- if (!clip_tbl.kmem_cachep)
+ if (!clip_tbl.kmem_cachep) {
+ remove_proc_entry("arp", atm_proc_root);
return -ENOMEM;
+ }
/* so neigh_ifdown() doesn't complain */
clip_tbl.proxy_timer.data = 0;
_
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fw: [Kernel-janitors] net/atm/clip.c: check kmem_cache_create() #1
2004-02-04 19:04 ` Fw: [Kernel-janitors] net/atm/clip.c: check kmem_cache_create() #1 chas williams
2004-02-04 21:47 ` Francois Romieu
@ 2004-02-05 7:24 ` David S. Miller
1 sibling, 0 replies; 6+ messages in thread
From: David S. Miller @ 2004-02-05 7:24 UTC (permalink / raw)
To: chas williams; +Cc: rddunlap, netdev
On Wed, 04 Feb 2004 14:04:17 -0500
chas williams (contractor) <chas@cmf.nrl.navy.mil> wrote:
> randy, i think it should probably return -ENOMEM instead of -1.
>
> dave, please apply the following to both the 2.6 and 2.4 trees.
Applied, thanks guys.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fw: [Kernel-janitors] net/atm/clip.c: check kmem_cache_create() #1
2004-02-04 21:47 ` Francois Romieu
@ 2004-02-05 12:22 ` chas williams
2004-02-05 18:38 ` Francois Romieu
0 siblings, 1 reply; 6+ messages in thread
From: chas williams @ 2004-02-05 12:22 UTC (permalink / raw)
To: Francois Romieu; +Cc: davem, Randy.Dunlap, netdev
In message <20040204224740.B19321@electric-eye.fr.zoreil.com>,Francois Romieu w
rites:
>One should probably apply the following patch on top of it btw.
>Unbalanced call to create_proc_entry() on error path.
how about the following instead? we probably shouldnt register the
proc entry until clip_tbl is going to be ready for use anyway (the
arp table iterators should probably also use clip_tbl instead of
clip_tbl_hook).
===== net/atm/clip.c 1.30 vs edited =====
--- 1.30/net/atm/clip.c Wed Feb 4 13:07:57 2004
+++ edited/net/atm/clip.c Thu Feb 5 07:18:03 2004
@@ -1008,14 +1008,6 @@
static int __init atm_clip_init(void)
{
-#ifdef CONFIG_PROC_FS
- struct proc_dir_entry *p;
-
- p = create_proc_entry("arp", S_IRUGO, atm_proc_root);
- if (p)
- p->proc_fops = &arp_seq_fops;
-#endif
-
/* we should use neigh_table_init() */
clip_tbl.lock = RW_LOCK_UNLOCKED;
clip_tbl.kmem_cachep = kmem_cache_create(clip_tbl.id,
@@ -1032,6 +1024,16 @@
clip_tbl_hook = &clip_tbl;
register_atm_ioctl(&clip_ioctl_ops);
+
+#ifdef CONFIG_PROC_FS
+{
+ struct proc_dir_entry *p;
+
+ p = create_proc_entry("arp", S_IRUGO, atm_proc_root);
+ if (p)
+ p->proc_fops = &arp_seq_fops;
+}
+#endif
return 0;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fw: [Kernel-janitors] net/atm/clip.c: check kmem_cache_create() #1
2004-02-05 12:22 ` chas williams
@ 2004-02-05 18:38 ` Francois Romieu
2004-02-05 18:48 ` chas williams
0 siblings, 1 reply; 6+ messages in thread
From: Francois Romieu @ 2004-02-05 18:38 UTC (permalink / raw)
To: chas williams; +Cc: Randy.Dunlap, netdev
chas williams <chas@cmf.nrl.navy.mil> :
[...]
> how about the following instead? we probably shouldnt register the
As long as the ugly-ifdef patrol does not bite, it's fine with me. :o)
> proc entry until clip_tbl is going to be ready for use anyway (the
> arp table iterators should probably also use clip_tbl instead of
> clip_tbl_hook).
It would not hurt. Do you have a specific race in mind before I send
an update on top your patch for the clip_tbl_hook -> clip_tbl change ?
--
Ueimor
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Fw: [Kernel-janitors] net/atm/clip.c: check kmem_cache_create() #1
2004-02-05 18:38 ` Francois Romieu
@ 2004-02-05 18:48 ` chas williams
0 siblings, 0 replies; 6+ messages in thread
From: chas williams @ 2004-02-05 18:48 UTC (permalink / raw)
To: Francois Romieu; +Cc: netdev
In message <20040205193813.A4230@electric-eye.fr.zoreil.com>,Francois Romieu wr
ites:
>As long as the ugly-ifdef patrol does not bite, it's fine with me. :o)
it would less ugly if people could accept a possibly unused stack
variable for the !CONFIG_PROC_FS case.
>It would not hurt. Do you have a specific race in mind before I send
>an update on top your patch for the clip_tbl_hook -> clip_tbl change ?
no particular race. just that the arp /proc entry should not be available
until clip gets initialized. clip_tbl_hook should only be used by
net/ipv4/arp.c really.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-02-05 18:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20040128091803.1da4cf1c.rddunlap@osdl.org>
2004-02-04 19:04 ` Fw: [Kernel-janitors] net/atm/clip.c: check kmem_cache_create() #1 chas williams
2004-02-04 21:47 ` Francois Romieu
2004-02-05 12:22 ` chas williams
2004-02-05 18:38 ` Francois Romieu
2004-02-05 18:48 ` chas williams
2004-02-05 7:24 ` David S. Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).