From: Flavio Leitner <fbl@sysclose•org>
To: Nicolas Dichtel <nicolas.dichtel@6wind•com>
Cc: davem@davemloft•net, netdev@vger•kernel.org
Subject: Re: [PATCH net] netlink: don't send unknown nsid
Date: Thu, 1 Jun 2017 19:44:54 -0300 [thread overview]
Message-ID: <20170601224454.GG2673@x240.lan> (raw)
In-Reply-To: <75fccaea-0ebe-1a2a-27d7-ad30fd99f41d@6wind.com>
On Thu, Jun 01, 2017 at 10:42:13PM +0200, Nicolas Dichtel wrote:
> Le 01/06/2017 à 19:02, Flavio Leitner a écrit :
> > On Thu, Jun 01, 2017 at 10:00:07AM +0200, Nicolas Dichtel wrote:
> >> 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 <fbl@sysclose•org>
> >> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind•com>
> >> ---
> >> 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.
I say it's incorrect because of the explanation below.
> > 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.
"only if an nsid is assigned" that's the issue.
Let me ask this instead: How do you think userspace should behave when
netnsid allocation fails?
--
Flavio
next prev parent reply other threads:[~2017-06-01 22:45 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-30 21:33 [PATCH net-next] netlink: include netnsid only when netns differs Flavio Leitner
2017-05-31 8:38 ` Nicolas Dichtel
2017-05-31 12:28 ` Flavio Leitner
2017-05-31 13:48 ` Nicolas Dichtel
2017-05-31 18:34 ` Flavio Leitner
2017-06-01 7:57 ` Nicolas Dichtel
2017-06-01 8:00 ` [PATCH net] netlink: don't send unknown nsid Nicolas Dichtel
2017-06-01 15:50 ` David Miller
2017-06-01 17:02 ` Flavio Leitner
2017-06-01 20:42 ` Nicolas Dichtel
2017-06-01 22:44 ` Flavio Leitner [this message]
2017-06-05 8:40 ` Nicolas Dichtel
2017-06-07 18:40 ` Flavio Leitner
2017-06-08 8:31 ` Nicolas Dichtel
2017-06-09 17:33 ` Flavio Leitner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170601224454.GG2673@x240.lan \
--to=fbl@sysclose$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=netdev@vger$(echo .)kernel.org \
--cc=nicolas.dichtel@6wind$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox