* [iproute PATCH] ip netns: Make sure netns name is sane
@ 2017-07-10 11:19 Phil Sutter
2017-07-10 15:17 ` Stephen Hemminger
2017-07-12 16:38 ` Stephen Hemminger
0 siblings, 2 replies; 5+ messages in thread
From: Phil Sutter @ 2017-07-10 11:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In order to keep track of created netns, 'ip netns' creates a mount
point inside NETNS_RUN_DIR. By not checking the user-specified name, it
allowed to create that mount point outside of NETNS_RUN_DIR and hence
lose track of it afterwards:
| # ip netns add ../../tmp/foobar
| # ip netns list
| # mount | grep foobar
| nsfs on /tmp/foobar type nsfs (rw)
Prevent this by making sure basename() does not see a need to alter the
given netns name.
Fixes: 0dc34c7713bb7 ("iproute2: Add processless network namespace support")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/ipnetns.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 0b0378ab6560c..4eee85e146b3d 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -595,6 +595,21 @@ static int create_netns_dir(void)
return 0;
}
+static bool is_basename(const char *name)
+{
+ char *name_dup = strdup(name);
+ bool rc = true;
+
+ if (!name_dup)
+ return false;
+
+ if (strcmp(basename(name_dup), name))
+ rc = false;
+
+ free(name_dup);
+ return rc;
+}
+
static int netns_add(int argc, char **argv)
{
/* This function creates a new network namespace and
@@ -616,6 +631,11 @@ static int netns_add(int argc, char **argv)
}
name = argv[0];
+ if (!is_basename(name)) {
+ fprintf(stderr, "Invalid netns name: contains path components\n");
+ return -1;
+ }
+
snprintf(netns_path, sizeof(netns_path), "%s/%s", NETNS_RUN_DIR, name);
if (create_netns_dir())
--
2.13.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [iproute PATCH] ip netns: Make sure netns name is sane
2017-07-10 11:19 [iproute PATCH] ip netns: Make sure netns name is sane Phil Sutter
@ 2017-07-10 15:17 ` Stephen Hemminger
2017-07-12 12:38 ` Phil Sutter
2017-07-12 16:38 ` Stephen Hemminger
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2017-07-10 15:17 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
On Mon, 10 Jul 2017 13:19:12 +0200
Phil Sutter <phil@nwl.cc> wrote:
> +static bool is_basename(const char *name)
> +{
> + char *name_dup = strdup(name);
> + bool rc = true;
> +
> + if (!name_dup)
> + return false;
> +
> + if (strcmp(basename(name_dup), name))
> + rc = false;
> +
> + free(name_dup);
> + return rc;
> +}
Why not just:
static bool is_basename(const char *name)
{
return strchr(name '/') == NULL;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [iproute PATCH] ip netns: Make sure netns name is sane
2017-07-10 15:17 ` Stephen Hemminger
@ 2017-07-12 12:38 ` Phil Sutter
0 siblings, 0 replies; 5+ messages in thread
From: Phil Sutter @ 2017-07-12 12:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
On Mon, Jul 10, 2017 at 08:17:02AM -0700, Stephen Hemminger wrote:
> On Mon, 10 Jul 2017 13:19:12 +0200
> Phil Sutter <phil@nwl.cc> wrote:
>
> > +static bool is_basename(const char *name)
> > +{
> > + char *name_dup = strdup(name);
> > + bool rc = true;
> > +
> > + if (!name_dup)
> > + return false;
> > +
> > + if (strcmp(basename(name_dup), name))
> > + rc = false;
> > +
> > + free(name_dup);
> > + return rc;
> > +}
>
> Why not just:
>
> static bool is_basename(const char *name)
> {
> return strchr(name '/') == NULL;
> }
This is not sufficient since it doesn't cover netns names of '..' and
'.', as Matteo correctly pointed out.
Cheers, Phil
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [iproute PATCH] ip netns: Make sure netns name is sane
2017-07-10 11:19 [iproute PATCH] ip netns: Make sure netns name is sane Phil Sutter
2017-07-10 15:17 ` Stephen Hemminger
@ 2017-07-12 16:38 ` Stephen Hemminger
2017-07-12 18:12 ` Phil Sutter
1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2017-07-12 16:38 UTC (permalink / raw)
To: Phil Sutter; +Cc: netdev
On Mon, 10 Jul 2017 13:19:12 +0200
Phil Sutter <phil@nwl.cc> wrote:
> +static bool is_basename(const char *name)
> +{
> + char *name_dup = strdup(name);
> + bool rc = true;
> +
> + if (!name_dup)
> + return false;
> +
> + if (strcmp(basename(name_dup), name))
> + rc = false;
> +
> + free(name_dup);
> + return rc;
> +}
Looks like natural place to use strdupa.
static bool is_basename(const char *name)
{
return strcmp(basename(strdupa(name), name) == 0;
}
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [iproute PATCH] ip netns: Make sure netns name is sane
2017-07-12 16:38 ` Stephen Hemminger
@ 2017-07-12 18:12 ` Phil Sutter
0 siblings, 0 replies; 5+ messages in thread
From: Phil Sutter @ 2017-07-12 18:12 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
On Wed, Jul 12, 2017 at 09:38:34AM -0700, Stephen Hemminger wrote:
> On Mon, 10 Jul 2017 13:19:12 +0200
> Phil Sutter <phil@nwl.cc> wrote:
>
> > +static bool is_basename(const char *name)
> > +{
> > + char *name_dup = strdup(name);
> > + bool rc = true;
> > +
> > + if (!name_dup)
> > + return false;
> > +
> > + if (strcmp(basename(name_dup), name))
> > + rc = false;
> > +
> > + free(name_dup);
> > + return rc;
> > +}
>
> Looks like natural place to use strdupa.
>
> static bool is_basename(const char *name)
> {
> return strcmp(basename(strdupa(name), name) == 0;
> }
Good point! Anyway, my patch fails to cover 'ip netns del' command
(apart from the '..' issue), so I'd suggest to instead apply Matteo's
version (Message-ID 20170710120831.9355-1-mcroce@redhat.com).
Thanks, Phil
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-07-12 18:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-10 11:19 [iproute PATCH] ip netns: Make sure netns name is sane Phil Sutter
2017-07-10 15:17 ` Stephen Hemminger
2017-07-12 12:38 ` Phil Sutter
2017-07-12 16:38 ` Stephen Hemminger
2017-07-12 18:12 ` Phil Sutter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.