On Tue, Jan 18, 2011 at 6:23 PM, Jarek Poplawski [off-list ref] wrote:
quoted hunk
[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 <redacted>
Cc: Jan Engelhardt <redacted>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jamal Hadi Salim <redacted>
---
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")
On Tue, Jan 18, 2011 at 07:10:35PM +0100, Alessandro Suardi wrote:
On Tue, Jan 18, 2011 at 6:23 PM, Jarek Poplawski [off-list ref] wrote:
quoted
[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 <redacted>
Cc: Jan Engelhardt <redacted>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jamal Hadi Salim <redacted>
---
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.
On Tuesday 2011-01-18 19:10, Alessandro Suardi wrote:
On Tue, Jan 18, 2011 at 6:23 PM, Jarek Poplawski [off-list ref] wrote:
quoted
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.
On Tue, Jan 18, 2011 at 07:28:52PM +0100, Jarek Poplawski wrote:
On Tue, Jan 18, 2011 at 07:24:40PM +0100, Jan Engelhardt wrote:
quoted
On Tuesday 2011-01-18 19:10, Alessandro Suardi wrote:
quoted
On Tue, Jan 18, 2011 at 6:23 PM, Jarek Poplawski [off-list ref] wrote:
quoted
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).
...
quoted
quoted
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.
On the other hand, until there is something better, we might try to
fix it at least for "normal" dumpit cases?
Alessandro, could you try (with the netlink revert)?
Thanks,
Jarek P.
---
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 19:36:25.000000000 +0100
@@ -519,15 +519,14 @@ 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)
- return -EOPNOTSUPP;
-
- genl_unlock();
- err = netlink_dump_start(net->genl_sock, skb, nlh,
- ops->dumpit, ops->done);
- genl_lock();
- return err;
+ 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;
+ }
}
if (ops->doit == NULL)On Tue, Jan 18, 2011 at 08:26:42PM +0100, Alessandro Suardi wrote:
On Tue, Jan 18, 2011 at 7:47 PM, Jarek Poplawski [off-list ref] wrote:
quoted
On Tue, Jan 18, 2011 at 07:28:52PM +0100, Jarek Poplawski wrote:
quoted
On Tue, Jan 18, 2011 at 07:24:40PM +0100, Jan Engelhardt wrote:
quoted
On Tuesday 2011-01-18 19:10, Alessandro Suardi wrote:
quoted
On Tue, Jan 18, 2011 at 6:23 PM, Jarek Poplawski [off-list ref] wrote:
quoted
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).
...
quoted
quoted
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.
On the other hand, until there is something better, we might try to
fix it at least for "normal" dumpit cases?
Alessandro, could you try (with the netlink revert)?
...
2.6.37-git18 + netlink revert + this 2nd attempt
appears to be good for me - both Avahi and acpid start up fine and I
can't see any other program misbehaving.
Alessandro, thanks for testing!
Jan, feel free to NAK if it can't help for your problem.
Jarek P.
---------------->
[PATCH v2] 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. Otherwise, it should work old way. Since, as reported
by Alessandro, apps like acpid use messages with ops->dumpit without
NLM_F_DUMP flags, there is no error reporting for this case.
Original patch by: Jan Engelhardt [off-list ref]
Tested-by: Alessandro Suardi <redacted>
Signed-off-by: Jarek Poplawski <redacted>
Cc: Jan Engelhardt <redacted>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jamal Hadi Salim <redacted>
---
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 19:36:25.000000000 +0100
@@ -519,15 +519,14 @@ 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)
- return -EOPNOTSUPP;
-
- genl_unlock();
- err = netlink_dump_start(net->genl_sock, skb, nlh,
- ops->dumpit, ops->done);
- genl_lock();
- return err;
+ 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;
+ }
}
if (ops->doit == NULL)