| Message ID | 20110118172340.GB1843@del.dom.local |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Jan 18, 2011 at 6:23 PM, Jarek Poplawski <jarkao2@gmail.com> wrote: > [PATCH] netlink: Fix possible NLM_F_DUMP misuse in genetlink > > NLM_F_DUMP flags should be applied to GET requests only, eg. rtnetlink > tests message type to verify this. Since genetlink can't do the same > use "practical" test for ops->dumpit (assuming NEW request won't be > mixed with GET). > > Signed-off-by: Jarek Poplawski <jarkao2@gmail.com> > Cc: Jan Engelhardt <jengelh@medozas.de> > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > Cc: Jamal Hadi Salim <hadi@cyberus.ca> > --- > Not for stable before testing! > > diff -Nurp a/net/netlink/genetlink.c b/net/netlink/genetlink.c > --- a/net/netlink/genetlink.c 2011-01-18 16:58:16.000000000 +0100 > +++ b/net/netlink/genetlink.c 2011-01-18 17:08:43.000000000 +0100 > @@ -519,15 +519,16 @@ static int genl_rcv_msg(struct sk_buff * > security_netlink_recv(skb, CAP_NET_ADMIN)) > return -EPERM; > > - if (nlh->nlmsg_flags & NLM_F_DUMP) { > - if (ops->dumpit == NULL) > + if (ops->dumpit) { > + if (nlh->nlmsg_flags & NLM_F_DUMP) { > + genl_unlock(); > + err = netlink_dump_start(net->genl_sock, skb, nlh, > + ops->dumpit, ops->done); > + genl_lock(); > + return err; > + } else { > return -EOPNOTSUPP; > - > - genl_unlock(); > - err = netlink_dump_start(net->genl_sock, skb, nlh, > - ops->dumpit, ops->done); > - genl_lock(); > - return err; > + } > } > > if (ops->doit == NULL) > 2.6.37-git18 + netlink revert + this patch - fixes Avahi - breaks acpid Upon startup I have: Starting acpi daemon: RTNETLINK1 answers: Operation not supported acpid: error talking to the kernel via netlink From strace output: open("/dev/input/event6", O_RDONLY|O_NONBLOCK) = 8 fcntl(8, F_SETFD, FD_CLOEXEC) = 0 ioctl(8, 0x80604520, 0x7fffb3418550) = 8 ioctl(8, 0x80604521, 0x7fffb34185b0) = 96 open("/dev/input/event8", O_RDONLY|O_NONBLOCK) = 9 fcntl(9, F_SETFD, FD_CLOEXEC) = 0 ioctl(9, 0x80604520, 0x7fffb3418550) = 8 ioctl(9, 0x80604532, 0x7fffb3418c10) = 8 close(9) = 0 inotify_init() = 9 inotify_add_watch(9, "/dev/input", IN_CREATE) = 1 socket(PF_NETLINK, SOCK_RAW, 16) = 10 setsockopt(10, SOL_SOCKET, SO_SNDBUF, [32768], 4) = 0 setsockopt(10, SOL_SOCKET, SO_RCVBUF, [32768], 4) = 0 bind(10, {sa_family=AF_NETLINK, pid=0, groups=00000000}, 12) = 0 getsockname(10, {sa_family=AF_NETLINK, pid=4298, groups=00000000}, [12]) = 0 sendmsg(10, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=00000000}, msg_iov(1)=[{"$\0\0\0\20\0\5\0\346\3265M\0\0\0\0\3\0\0\0\17\0\2\0acpi_eve"..., 36}], msg_controllen=0, msg_flags=0}, 0) = 36 recvmsg(10, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=00000000}, msg_iov(1)=[{"8\0\0\0\2\0\0\0\346\3265M\312\20\0\0\241\377\377\377$\0\0\0\20\0\5\0\346\3265M"..., 16384}], msg_controllen=0, msg_flags=0}, 0) = 56 dup(2) = 11 fcntl(11, F_GETFL) = 0x1 (flags O_WRONLY) close(11) = 0 write(2, "RTNETLINK1 answers: Operation no"..., 44RTNETLINK1 answers: Operation not supported ) = 44 write(2, "acpid: error talking to the kern"..., 47acpid: error talking to the kernel via netlink ) = 47 close(10) = 0 socket(PF_NETLINK, SOCK_RAW, 16) = 10 setsockopt(10, SOL_SOCKET, SO_SNDBUF, [32768], 4) = 0 setsockopt(10, SOL_SOCKET, SO_RCVBUF, [32768], 4) = 0 bind(10, {sa_family=AF_NETLINK, pid=0, groups=00000000}, 12) = 0 getsockname(10, {sa_family=AF_NETLINK, pid=4298, groups=00000000}, [12]) = 0 unlink("/var/run/acpid.socket") = 0 socket(PF_FILE, SOCK_STREAM, 0) = 11 bind(11, {sa_family=AF_FILE, path="/var/run/acpid.socket"}, 110) = 0 listen(11, 10) = 0 If more debugging/testing is needed, do ping me. Thanks, --alessandro "There's always a siren singing you to shipwreck" (Radiohead, "There There") -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 18, 2011 at 07:10:35PM +0100, Alessandro Suardi wrote: > On Tue, Jan 18, 2011 at 6:23 PM, Jarek Poplawski <jarkao2@gmail.com> wrote: > > [PATCH] netlink: Fix possible NLM_F_DUMP misuse in genetlink > > > > NLM_F_DUMP flags should be applied to GET requests only, eg. rtnetlink > > tests message type to verify this. Since genetlink can't do the same > > use "practical" test for ops->dumpit (assuming NEW request won't be > > mixed with GET). > > > > Signed-off-by: Jarek Poplawski <jarkao2@gmail.com> > > Cc: Jan Engelhardt <jengelh@medozas.de> > > Cc: Pablo Neira Ayuso <pablo@netfilter.org> > > Cc: Jamal Hadi Salim <hadi@cyberus.ca> > > --- > > Not for stable before testing! > > > > diff -Nurp a/net/netlink/genetlink.c b/net/netlink/genetlink.c > > --- a/net/netlink/genetlink.c 2011-01-18 16:58:16.000000000 +0100 > > +++ b/net/netlink/genetlink.c 2011-01-18 17:08:43.000000000 +0100 > > @@ -519,15 +519,16 @@ static int genl_rcv_msg(struct sk_buff * ... > 2.6.37-git18 + netlink revert + this patch > - fixes Avahi > - breaks acpid Well, then fixing genetlink needs more than this and I withdraw this patch. Anyway, netlink revert is IMHO needed to prevent the regression. ... > If more debugging/testing is needed, do ping me. Thanks, Many thanks for such a fast response! Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 2011-01-18 19:10, Alessandro Suardi wrote: >On Tue, Jan 18, 2011 at 6:23 PM, Jarek Poplawski <jarkao2@gmail.com> wrote: >> >> NLM_F_DUMP flags should be applied to GET requests only, eg. rtnetlink >> tests message type to verify this. Since genetlink can't do the same >> use "practical" test for ops->dumpit (assuming NEW request won't be >> mixed with GET). >> >> - if (nlh->nlmsg_flags & NLM_F_DUMP) { >> - if (ops->dumpit == NULL) >> + if (ops->dumpit) { >> + if (nlh->nlmsg_flags & NLM_F_DUMP) { >> + genl_unlock(); >> + err = netlink_dump_start(net->genl_sock, skb, nlh, >> + ops->dumpit, ops->done); >> + genl_lock(); >> + return err; >> + } else { >> return -EOPNOTSUPP; >> - >> - genl_unlock(); >> - err = netlink_dump_start(net->genl_sock, skb, nlh, >> - ops->dumpit, ops->done); >> - genl_lock(); >> - return err; >> + } > >2.6.37-git18 + netlink revert + this patch > - fixes Avahi > - breaks acpid >Starting acpi daemon: RTNETLINK1 answers: Operation not supported >acpid: error talking to the kernel via netlink Deducing from that, it is a GET-like request that was sent by acpid, and the message type is one that has both a dumpit and a doit function. So if EOPNOTSUPP now occurs on all message types that have both dumpit and doit, you should have broken a lot more than just acpid. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 18, 2011 at 07:24:40PM +0100, Jan Engelhardt wrote: > > On Tuesday 2011-01-18 19:10, Alessandro Suardi wrote: > >On Tue, Jan 18, 2011 at 6:23 PM, Jarek Poplawski <jarkao2@gmail.com> wrote: > >> > >> NLM_F_DUMP flags should be applied to GET requests only, eg. rtnetlink > >> tests message type to verify this. Since genetlink can't do the same > >> use "practical" test for ops->dumpit (assuming NEW request won't be > >> mixed with GET). ... > >2.6.37-git18 + netlink revert + this patch > > - fixes Avahi > > - breaks acpid > >Starting acpi daemon: RTNETLINK1 answers: Operation not supported > >acpid: error talking to the kernel via netlink > > Deducing from that, it is a GET-like request that was sent by acpid, > and the message type is one that has both a dumpit and a doit function. > So if EOPNOTSUPP now occurs on all message types that have both dumpit > and doit, you should have broken a lot more than just acpid. Right, we need something better here. Jarek P. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff -Nurp a/net/netlink/genetlink.c b/net/netlink/genetlink.c --- a/net/netlink/genetlink.c 2011-01-18 16:58:16.000000000 +0100 +++ b/net/netlink/genetlink.c 2011-01-18 17:08:43.000000000 +0100 @@ -519,15 +519,16 @@ static int genl_rcv_msg(struct sk_buff * security_netlink_recv(skb, CAP_NET_ADMIN)) return -EPERM; - if (nlh->nlmsg_flags & NLM_F_DUMP) { - if (ops->dumpit == NULL) + if (ops->dumpit) { + if (nlh->nlmsg_flags & NLM_F_DUMP) { + genl_unlock(); + err = netlink_dump_start(net->genl_sock, skb, nlh, + ops->dumpit, ops->done); + genl_lock(); + return err; + } else { return -EOPNOTSUPP; - - genl_unlock(); - err = netlink_dump_start(net->genl_sock, skb, nlh, - ops->dumpit, ops->done); - genl_lock(); - return err; + } } if (ops->doit == NULL)
[PATCH] netlink: Fix possible NLM_F_DUMP misuse in genetlink NLM_F_DUMP flags should be applied to GET requests only, eg. rtnetlink tests message type to verify this. Since genetlink can't do the same use "practical" test for ops->dumpit (assuming NEW request won't be mixed with GET). Signed-off-by: Jarek Poplawski <jarkao2@gmail.com> Cc: Jan Engelhardt <jengelh@medozas.de> Cc: Pablo Neira Ayuso <pablo@netfilter.org> Cc: Jamal Hadi Salim <hadi@cyberus.ca> --- Not for stable before testing! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html