Re: Fw: genetlink or connector interface for delay accounting patches ?

[jamal]

On Fri, 2006-03-02 at 16:34 -0800, David S. Miller wrote:
> Can someone look into the holes found in the genetlink code
> Andrew mentions below?


> "how come the return value from genlmsg_multicast() gets ignored" 

Sometimes the users dont care about the return code; i.e if the message
doesnt make it fine. You should look at if you care about reliability of
delivery and perhaps retransmit when it makes sense.

> "how come i takes the address of an inline function" 
Didnt quiet follow

> "how come genl_register_family() leaves genl_sem held if kmalloc
>failed" and "how come there's a return statement after panic() which
>is marked NORET_TYPE".

certainly bugs. patch attached.

PS:- I have a doc i started working on - will look it up and post

cheers,
jamal

Fix genetlink bugs found by Sir Andrew Morton

Signed-off-by: Jamal Hadi Salim <[EMAIL PROTECTED]>
---

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 4ae1538..8cbad1d 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -238,7 +238,7 @@ int genl_register_family(struct genl_fam
sizeof(struct nlattr *), GFP_KERNEL);
if (family->attrbuf == NULL) {
err = -ENOMEM;
-   goto errout;
+   goto errout_locked;
}
} else
family->attrbuf = NULL;
@@ -549,10 +549,8 @@ static int __init genl_init(void)
netlink_set_nonroot(NETLINK_GENERIC, NL_NONROOT_RECV);
genl_sock = netlink_kernel_create(NETLINK_GENERIC, GENL_MAX_ID,
  genl_rcv, THIS_MODULE);
-   if (genl_sock == NULL) {
+   if (genl_sock == NULL) 
panic("GENL: Cannot initialize generic netlink\n");
-   return -ENOMEM;
-   }
 
return 0;
 
@@ -560,7 +558,6 @@ errout_register:
genl_unregister_family(&genl_ctrl);
 errout:
panic("GENL: Cannot register controller: %d\n", err);
-   return err;
 }
 
 subsys_initcall(genl_init);

Fw: genetlink or connector interface for delay accounting patches ?

[David S. Miller]

Can someone look into the holes found in the genetlink code
Andrew mentions below?

Thanks.
--- Begin Message ---
"David S. Miller" <[EMAIL PROTECTED]> wrote:
>
> Or do you think people working on networking should have to shift
> through 300 postings a day about the copyrights of dead authors and
> the bi-yearly flame fests about cdrecord?

No, I think that people who are proposing a piece of code which other
kernel developers are expected to use should keep those developers informed
on it.

I at no point suggested that Jamal & co should read linux-kernel.  I meant
that they (or you) should have cc'ed linux-kernel on the discussions
regarding this feature.  And I think you know that's what I meant.


And if the patches _had_ been cc'ed to linux-kernel, not only would the
knowledge been better spread, people might have asked stuff like "how come
the return value from genlmsg_multicast() gets ignored" and "how come it
takes the address of an inline function" and "how come
genl_register_family() leaves genl_sem held if kmalloc failed" and "how
come there's a return statement after panic() which is marked NORET_TYPE".

Again: bottom line: the target audience for this feature don't know how to
use it, don't know who wrote it, don't know why they wrote it and don't
even know it exists.  That's just a boring old fact and we should have
found a way to prevent this from coming about.
--- End Message ---