| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1354039239.2701.8.camel@bwh-desktop.uk.solarflarecom.com>
Date: Tue, 27 Nov 2012 18:00:39 +0000
From: Ben Hutchings <bhutchings@...arflare.com>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
CC: Stephen Hemminger <shemminger@...tta.com>,
<netdev@...r.kernel.org>, "Serge E. Hallyn" <serge@...lyn.com>
Subject: Re: [PATCH for 3.8] iproute2: Add "ip netns pids" and "ip netns
identify"
On Mon, 2012-11-26 at 17:16 -0600, Eric W. Biederman wrote:
> Add command that go between network namespace names and process
> identifiers. The code builds and runs agains older kernels but
> only works on Linux 3.8+ kernels where I have fixed stat to work
> properly.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
> ---
>
> I don't know if this is too soon to send this patch to iproute as the
> kernel code that fixes stat is currently sitting in my for-next branch
> of:
> git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git
>
> and has not hit Linus's tree yet. Still the code runs and is harmless
> on older kernels so it should be harmless whatever happens with it.
>
> ip/ipnetns.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++
> man/man8/ip-netns.8 | 5 ++-
> 2 files changed, 145 insertions(+), 1 deletions(-)
>
> diff --git a/ip/ipnetns.c b/ip/ipnetns.c
> index e41a598..c55fe3a 100644
> --- a/ip/ipnetns.c
> +++ b/ip/ipnetns.c
> @@ -13,6 +13,7 @@
> #include <dirent.h>
> #include <errno.h>
> #include <unistd.h>
> +#include <ctype.h>
>
> #include "utils.h"
> #include "ip_common.h"
> @@ -48,6 +49,8 @@ static void usage(void)
> fprintf(stderr, "Usage: ip netns list\n");
> fprintf(stderr, " ip netns add NAME\n");
> fprintf(stderr, " ip netns delete NAME\n");
> + fprintf(stderr, " ip netns identify PID\n");
> + fprintf(stderr, " ip netns pids NAME\n");
> fprintf(stderr, " ip netns exec NAME cmd ...\n");
> fprintf(stderr, " ip netns monitor\n");
> exit(-1);
> @@ -171,6 +174,138 @@ static int netns_exec(int argc, char **argv)
> exit(-1);
> }
>
> +static int is_pid(const char *str)
> +{
> + int ch;
> + for (; (ch = *str); str++) {
> + if (!isdigit(ch))
ch must be cast to unsigned char before passing to isdigit().
> + return 0;
> + }
> + return 1;
> +}
> +
> +static int netns_pids(int argc, char **argv)
> +{
> + const char *name;
> + char net_path[MAXPATHLEN];
> + int netns;
> + struct stat netst;
> + DIR *dir;
> + struct dirent *entry;
> +
> + if (argc < 1) {
> + fprintf(stderr, "No netns name specified\n");
> + return -1;
> + }
> + if (argc > 1) {
> + fprintf(stderr, "extra arguments specified\n");
> + return -1;
> + }
These, and many other return statements in this file which set the
process exit code, should return 1 (general failure) or 2 (user error)
rather than -1 (likely to be interpreted as command not found).
> + name = argv[0];
> + snprintf(net_path, sizeof(net_path), "%s/%s", NETNS_RUN_DIR, name);
No check for truncation?
> + netns = open(net_path, O_RDONLY);
This file descriptor is leaked, though that probably doesn't really
matter.
[...]
> +static int netns_identify(int argc, char **argv)
> +{
> + const char *pidstr;
> + char net_path[MAXPATHLEN];
> + int netns;
> + struct stat netst;
> + DIR *dir;
> + struct dirent *entry;
> +
> + if (argc < 1) {
> + fprintf(stderr, "No pid specified\n");
> + return -1;
> + }
> + if (argc > 1) {
> + fprintf(stderr, "extra arguments specified\n");
> + return -1;
> + }
> + pidstr = argv[0];
> +
> + if (!is_pid(pidstr)) {
> + fprintf(stderr, "Specified string '%s' is not a pid\n",
> + pidstr);
> + return -1;
> + }
> +
> + snprintf(net_path, sizeof(net_path), "/proc/%s/ns/net", pidstr);
> + netns = open(net_path, O_RDONLY);
> + if (netns < 0) {
> + fprintf(stderr, "Cannot open network namespace: %s\n",
> + strerror(errno));
> + return -1;
> + }
> + if (fstat(netns, &netst) < 0) {
> + fprintf(stderr, "Stat of netns failed: %s\n",
> + strerror(errno));
> + return -1;
> + }
> + dir = opendir(NETNS_RUN_DIR);
> + if (!dir)
> + return 0;
Shouldn't this be treated as an error? Or, if you want it to succeed
when the kernel does not have netns functionality, then treat it as an
error if !dir && errno != ENOENT.
> + while((entry = readdir(dir))) {
> + char name_path[MAXPATHLEN];
> + struct stat st;
> +
> + if (strcmp(entry->d_name, ".") == 0)
> + continue;
> + if (strcmp(entry->d_name, "..") == 0)
> + continue;
> +
> + snprintf(name_path, sizeof(name_path), "%s/%s", NETNS_RUN_DIR,
> + entry->d_name);
> +
> + if (stat(name_path, &st) != 0)
> + continue;
> +
> + if ((st.st_dev == netst.st_dev) &&
> + (st.st_ino == netst.st_ino)) {
> + printf("%s\n", entry->d_name);
> + }
> + }
> + closedir(dir);
> + return 0;
> +
> +}
[...]
It's a shame there isn't a more efficient way to do these lookups.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists