Thread (15 messages) 15 messages, 3 authors, 2017-06-09

Re: [PATCH net] netlink: don't send unknown nsid

From: Nicolas Dichtel <hidden>
Date: 2017-06-01 20:42:18

Le 01/06/2017 à 19:02, Flavio Leitner a écrit :
On Thu, Jun 01, 2017 at 10:00:07AM +0200, Nicolas Dichtel wrote:
quoted
The NETLINK_F_LISTEN_ALL_NSID otion enables to listen all netns that have a
nsid assigned into the netns where the netlink socket is opened.
The nsid is sent as metadata to userland, but the existence of this nsid is
checked only for netns that are different from the socket netns. Thus, if
no nsid is assigned to the socket netns, NETNSA_NSID_NOT_ASSIGNED is
reported to the userland. This value is confusing and useless.
After this patch, only valid nsid are sent to userland.

Reported-by: Flavio Leitner <redacted>
Signed-off-by: Nicolas Dichtel <redacted>
---
 net/netlink/af_netlink.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index ee841f00a6ec..7586d446d7dc 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -62,6 +62,7 @@
 #include <asm/cacheflush.h>
 #include <linux/hash.h>
 #include <linux/genetlink.h>
+#include <linux/net_namespace.h>
 
 #include <net/net_namespace.h>
 #include <net/sock.h>
@@ -1415,7 +1416,8 @@ static void do_one_broadcast(struct sock *sk,
 		goto out;
 	}
 	NETLINK_CB(p->skb2).nsid = peernet2id(sock_net(sk), p->net);
-	NETLINK_CB(p->skb2).nsid_is_set = true;
+	if (NETLINK_CB(p->skb2).nsid != NETNSA_NSID_NOT_ASSIGNED)
+		NETLINK_CB(p->skb2).nsid_is_set = true;
 	val = netlink_broadcast_deliver(sk, p->skb2);
 	if (val < 0) {
 		netlink_overrun(sk);
If the assumption is that nsid allocation can never fail or that if it
does, we can't report to userspace, then the patch is good, but it
doesn't sound like a good long term solution.

Let's consider that the allocation of an id fails for whatever reason.
I think that should be reported to userspace to allow it to retry, or
do something else to handle this situation properly.  Not sending
anything means that it's in the same netns as the old kernels did,
which is incorrect.
This is correct, because if nsid allocation fails, no netlink messages from this
netns are sent to userspace (the check is done at the beginning of
do_one_broadcast). The only netns allowed to send netlink messages to userspace
without nsid is the netns of the socket.
On the other hand, with the original patch, if the socket and the
device are in the same netns, we don't need to report any ID.  Previous
kernels did that, so we are not breaking anything.  When the netns
differs, then we either should report the real ID or an error.
I don't understand. With or without my last patch, the kernel sends netlink
messages of other netns than the netns where the socket is opened, only if an
nsid is assigned.


Nicolas

ps: I won't be able to read my emails before monday ;-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help