Skip to contentSkip to conversation

linux-2.6.9: add errorcheck for netlink

KJ Domen Puncer 2 participants 3 messages Dec 15, 2004 Kernel janitors

lore
Share & tools
lore

Domen Puncer

author
#0 lore raw
Quoted 8 lines · walter harms

Hi list,
i moved the netlink patches from 2.6.7 to 2.6.9.
it adds 2 error checks.
1. register_netdevice_notifier()
2. sock_register()

To propagate the error i changed the type from void to int.

Quoted 8 lines

re,
walter

Signed-off-by walter harms <[email protected]>

--- linux-2.6.9/net/core/rtnetlink.c.bak 2004-11-26
22:11:15.000000000 +0100

Fix mailer not to wrap lines.

Quoted 26 lines

+++ linux-2.6.9/net/core/rtnetlink.c 2004-11-26 23:22:00.000000000 +0100

(quoted diff) +6 -4
@@ -653,7 +653,7 @@
 	.notifier_call	= rtnetlink_event,
 };
 
-void __init rtnetlink_init(void)
+int __init rtnetlink_init(void)
 {
 	int i;
 

@@ -661,17 +661,34 @@
 	for (i = 0; i < ARRAY_SIZE(rta_max); i++)
 		if (rta_max[i] > rtattr_max)
 			rtattr_max = rta_max[i];
+
 	rta_buf = kmalloc(rtattr_max * sizeof(struct rtattr *), GFP_KERNEL);
-	if (!rta_buf)
-		panic("rtnetlink_init: cannot allocate rta_buf\n");
+	if (!rta_buf) {
+		printk(KERN_CRIT "%s: cannot allocate 
 rta_buf\n",__FUNCTION__);
+		goto no_mem;
+	}
 
 	rtnl = netlink_kernel_create(NETLINK_ROUTE, rtnetlink_rcv);
-	if (rtnl == NULL)

Review line 680

Quoted 10 lines
(quoted diff) +6 -2
-		panic("rtnetlink_init: cannot initialize rtnetlink\n");
+	if (rtnl == NULL) {
+		printk(KERN_CRIT "%s: cannot initialize 
 rtnetlink\n",__FUNCTION__);
+		goto no_netlink;
+	}
 	netlink_set_nonroot(NETLINK_ROUTE, NL_NONROOT_RECV);
-	register_netdevice_notifier(&rtnetlink_dev_notifier);
+
+        if ( register_netdevice_notifier(&rtnetlink_dev_notifier) < 0 ) {

Whitespace damage.

Quoted 4 lines
(quoted diff) +3 -0
+
+                printk(KERN_ERR "%s: cannot initialize rtnetlink 
 notifier\n",__FUNCTION__);
+		goto no_netlink;

Review line 694

Quoted 10 lines
(quoted diff) +8 -0
+        }
+
 	rtnetlink_links[PF_UNSPEC] = link_rtnetlink_table;
 	rtnetlink_links[PF_PACKET] = link_rtnetlink_table;
+	return 0;
+
+no_netlink:
+	kfree(rtnl);
+no_mem:
+	return -1;

How about -ENOMEM or similar?
Also: nl_table isn't freed.

Quoted 23 lines

}

EXPORT_SYMBOL(__rta_fill);
--- linux-2.6.9/include/linux/rtnetlink.h.bak 2004-11-26
22:59:02.000000000 +0100
+++ linux-2.6.9/include/linux/rtnetlink.h 2004-11-26
23:00:09.986720520 +0100

(quoted diff) +1 -2
@@ -805,7 +805,7 @@
 
 extern void rtnl_lock(void);
 extern void rtnl_unlock(void);
-extern void rtnetlink_init(void);
+extern int rtnetlink_init(void);
 
 #define ASSERT_RTNL() do { \
 	if (unlikely(down_trylock(&rtnl_sem) == 0)) { \
--- linux-2.6.9/net/netlink/af_netlink.c.bak	2004-11-26

22:24:29.000000000 +0100
+++ linux-2.6.9/net/netlink/af_netlink.c 2004-11-26
22:44:22.000000000 +0100

(quoted diff) +0 -0
@@ -1209,29 +1209,51 @@
 	.owner	= THIS_MODULE,	/* for consistency 8) */
 };

Review line 1220

Quoted 10 lines
(quoted diff) +0 -8
 
-static int __init netlink_proto_init(void)
-{
-	struct sk_buff *dummy_skb;
 
-	if (sizeof(struct netlink_skb_parms) > sizeof(dummy_skb->cb)) {
-		printk(KERN_CRIT "netlink_init: panic\n");
-		return -1;
-	}
-	sock_register(&netlink_family_ops);

Moving functions instead of predeclaring? It makes reviewing harder.

Quoted 32 lines
(quoted diff) +20 -7
+static void __exit netlink_proto_exit(void)
+{
+       sock_unregister(PF_NETLINK);
 #ifdef CONFIG_PROC_FS
-	proc_net_fops_create("netlink", 0, &netlink_seq_fops);
+       proc_net_remove("netlink");
 #endif
-	/* The netlink device handler may be needed early. */
-	rtnetlink_init();
-	return 0;
 }
 
-static void __exit netlink_proto_exit(void)
+static int __init netlink_proto_init(void)
 {
-       sock_unregister(PF_NETLINK);
-       proc_net_remove("netlink");
+        struct sk_buff *dummy_skb;
+#ifdef CONFIG_PROC_FS
+        struct proc_dir_entry *proc;
+#endif
+        int err;
+
+        if (sizeof(struct netlink_skb_parms) > sizeof(dummy_skb->cb)) {
+                printk(KERN_CRIT "netlink_init: panic\n");
+                return -1;
+        }
+        err = sock_register(&netlink_family_ops);
+        if (err < 0)
+                return err;
+
+#ifdef CONFIG_PROC_FS

Review line 1247

Quoted 10 lines
(quoted diff) +10 -0
+        proc = proc_net_fops_create("netlink", 0, &netlink_seq_fops);
+        if (!proc) {
+                sock_unregister(PF_NETLINK);
+                return -ENOMEM;
+        }
+#endif
+
+        /* The netlink device handler may be needed early. */
+        if ( rtnetlink_init() < 0) {
+                netlink_proto_exit();

You can't call __exit function from non-__exit. (If this is linked in kernel __exit stuff gets discarded)

Review line 1256

Quoted 9 lines
(quoted diff) +7 -0
+                return -1;
+        } ;
+
+        return 0;
 }
 
+
+
+

?

Quoted 8 lines
(quoted diff) +0 -0
 core_initcall(netlink_proto_init);
 module_exit(netlink_proto_exit);

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

author
#1 lore raw
Quoted 26 lines · walter harms
Quoted 4 lines

Fix mailer not to wrap lines.

I am unsing now mozilla-mail hope that help,
if i have more time i will switch to something more easy.

Quoted 4 lines
Quoted 4 lines
	rtnetlink_links[PF_UNSPEC] = link_rtnetlink_table;
	rtnetlink_links[PF_PACKET] = link_rtnetlink_table;
(quoted diff) +2 -0
+	return 0;
+
Quoted 9 lines

Review

Quoted 4 lines
(quoted diff) +4 -0
+no_netlink:
+	kfree(rtnl);
+no_mem:
+	return -1;

How about -ENOMEM or similar?
Also: nl_table isn't freed.

Quoted 4 lines

Whitespace fixed
-ENOM instead of -1
freeing rta_buf
what is nl_table ? typo ? i missed something ?

Uh, mixed that up; should have been rta_buf

Quoted 98 lines
Quoted 81 lines
Quoted 27 lines

}

EXPORT_SYMBOL(__rta_fill);
--- linux-2.6.9/include/linux/rtnetlink.h.bak 2004-11-26
22:59:02.000000000 +0100
+++ linux-2.6.9/include/linux/rtnetlink.h 2004-11-26
23:00:09.986720520 +0100

(quoted diff) +1 -2
@@ -805,7 +805,7 @@
 
 extern void rtnl_lock(void);
 extern void rtnl_unlock(void);
-extern void rtnetlink_init(void);
+extern int rtnetlink_init(void);
 
 #define ASSERT_RTNL() do { \
 	if (unlikely(down_trylock(&rtnl_sem) == 0)) { \
--- linux-2.6.9/net/netlink/af_netlink.c.bak	2004-11-26

22:24:29.000000000 +0100
+++ linux-2.6.9/net/netlink/af_netlink.c 2004-11-26
22:44:22.000000000 +0100

(quoted diff) +0 -0
@@ -1209,29 +1209,51 @@
 	.owner	= THIS_MODULE,	/* for consistency 8) */
 };

Review line 1220

Quoted 10 lines
(quoted diff) +0 -8
 
-static int __init netlink_proto_init(void)
-{
-	struct sk_buff *dummy_skb;
 
-	if (sizeof(struct netlink_skb_parms) > sizeof(dummy_skb->cb)) {
-		printk(KERN_CRIT "netlink_init: panic\n");
-		return -1;
-	}
-	sock_register(&netlink_family_ops);

Moving functions instead of predeclaring? It makes reviewing harder.

Quoted 38 lines
(quoted diff) +26 -7
+static void __exit netlink_proto_exit(void)
+{
+       sock_unregister(PF_NETLINK);
 #ifdef CONFIG_PROC_FS
-	proc_net_fops_create("netlink", 0, &netlink_seq_fops);
+       proc_net_remove("netlink");
 #endif
-	/* The netlink device handler may be needed early. */
-	rtnetlink_init();
-	return 0;
 }
 
-static void __exit netlink_proto_exit(void)
+static int __init netlink_proto_init(void)
 {
-       sock_unregister(PF_NETLINK);
-       proc_net_remove("netlink");
+        struct sk_buff *dummy_skb;
+#ifdef CONFIG_PROC_FS
+        struct proc_dir_entry *proc;
+#endif
+        int err;
+
+        if (sizeof(struct netlink_skb_parms) > sizeof(dummy_skb->cb)) {
+                printk(KERN_CRIT "netlink_init: panic\n");
+                return -1;
+        }
+        err = sock_register(&netlink_family_ops);
+        if (err < 0)
+                return err;
+
+#ifdef CONFIG_PROC_FS
+        proc = proc_net_fops_create("netlink", 0, &netlink_seq_fops);
+        if (!proc) {
+                sock_unregister(PF_NETLINK);
+                return -ENOMEM;
+        }
+#endif
Quoted 16 lines

Review line 1247

Quoted 4 lines
(quoted diff) +4 -0
+
+        /* The netlink device handler may be needed early. */
+        if ( rtnetlink_init() < 0) {
+                netlink_proto_exit();

You can't call __exit function from non-__exit. (If this is linked in kernel __exit stuff gets discarded)

Quoted 6 lines
(quoted diff) +4 -0
+                return -1;
+        } ;
+
+        return 0;
 }
Quoted 4 lines

+ whitespace fixed 'if ( rtnetlink_init'
+ netlink_proto_exit is prototyped
Will you move the function in front of _init again ? A prototype for a 2 liner is noise for me :)

Ah, false alarm then; looked like moving big functions, but is in fact indentiation (in original tabs, in you version spaces).

Quoted 3 lines

+ i removed CONFIG_PROC_FS from the exit code since i have learned that proc_net_remove() will be empty in that case

Right.

Quoted 4 lines

re,
walter

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors

#2 lore raw
In reply to Domen Puncer
Quoted 9 lines · Domen Puncer
Quoted 7 lines · walter harms
Quoted 2 lines

Fix mailer not to wrap lines.

I am unsing now mozilla-mail hope that help,
if i have more time i will switch to something more easy.

Drop mozilla-mail for submitting patches. We hashed this out on LKML a couple of months ago - see:

http://lkml.org/lkml/2004/10/22/488

I use the sendpatchset script written by Paul Jackson at:

http://www.speakeasy.net/~pj99/sgi/sendpatchset

It makes life much easier.

Jim Nelson

_______________________________________________
Kernel-janitors mailing list
[email protected]
http://lists.osdl.org/mailman/listinfo/kernel-janitors