* 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 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
* 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
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).