Dropped patch #1, since it has been merged Fixed the socket leak Removed redundant calls to mount debugfs
Andrew Lunn (2): alfred: Add support for network namespaces alfred: Mount debugfs before reducing capabilities
batadv_query.c | 19 +------------------ debugfs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- main.c | 4 ++++ vis/vis.h | 2 +- 4 files changed, 54 insertions(+), 20 deletions(-)
When running within a network namespace, access to files within debugfs have to take into account the network name space. Each namespace has its own directory under /sys/kernel/debug/batman_adv/netns.
Signed-off-by: Andrew Lunn andrew@lunn.ch --- v2: Fix file descriptor leak --- batadv_query.c | 2 +- debugfs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++- vis/vis.h | 2 +- 3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/batadv_query.c b/batadv_query.c index 2604503..1b9e631 100644 --- a/batadv_query.c +++ b/batadv_query.c @@ -31,7 +31,7 @@ #include <sys/types.h> #include "debugfs.h"
-#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s" +#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s%s" #define DEBUG_TRANSTABLE_GLOBAL "transtable_global" #define DEBUG_ORIGINATORS "originators"
diff --git a/debugfs.c b/debugfs.c index fc39322..3ee2cbd 100644 --- a/debugfs.c +++ b/debugfs.c @@ -20,11 +20,15 @@
#include "debugfs.h" #include <errno.h> +#include <fcntl.h> +#include <limits.h> #include <stdio.h> #include <string.h> #include <sys/mount.h> #include <sys/stat.h> #include <sys/statfs.h> +#include <sys/types.h> +#include <unistd.h>
#ifndef DEBUGFS_MAGIC #define DEBUGFS_MAGIC 0x64626720 @@ -42,16 +46,59 @@ static const char *debugfs_known_mountpoints[] = { NULL, };
+/* Return the current net namespace number. 0 is never a valid + * namespace, so use it to return that there is no name space + * support. + */ + +static unsigned int debugfs_get_netns_inum(void) +{ + char net_path[] = "/proc/self/ns/net"; + struct stat netst; + int netns; + + netns = open(net_path, O_RDONLY); + if (netns < 0) { + if (errno == ENOENT) + /* Probably means no netns support in the kernel */ + return 0; + + fprintf(stderr, + "Error - can't open /proc/self/ns/net for read: %s\n", + strerror(errno)); + return 0; + } + + if (fstat(netns, &netst) < 0) { + fprintf(stderr, "Stat of netns failed: %s\n", + strerror(errno)); + netst.st_ino = 0; + } + close (netns); + + return netst.st_ino; +} + /* construct a full path to a debugfs element */ int debugfs_make_path(const char *fmt, const char *mesh_iface, char *buffer, int size) { + unsigned int ns = debugfs_get_netns_inum(); + char ns_dir[PATH_MAX]; + if (strlen(debugfs_mountpoint) == 0) { buffer[0] = '\0'; return -1; }
- return snprintf(buffer, size, fmt, debugfs_mountpoint, mesh_iface); + if (ns) { + snprintf(ns_dir, PATH_MAX, "netns/%u/", ns); + return snprintf(buffer, size, fmt, debugfs_mountpoint, ns_dir, + mesh_iface); + } else { + return snprintf(buffer, size, fmt, debugfs_mountpoint, "", + mesh_iface); + } }
static int debugfs_found; diff --git a/vis/vis.h b/vis/vis.h index 37e3164..2ccc6cb 100644 --- a/vis/vis.h +++ b/vis/vis.h @@ -37,7 +37,7 @@ #define UPDATE_INTERVAL 10
#define SYS_IFACE_PATH "/sys/class/net" -#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s" +#define DEBUG_BATIF_PATH_FMT "%s/batman_adv/%s%s" #define SYS_MESH_IFACE_FMT SYS_IFACE_PATH"/%s/batman_adv/mesh_iface" #define SYS_IFACE_STATUS_FMT SYS_IFACE_PATH"/%s/batman_adv/iface_status"
The debugfs helper code has the ability to mount the debugfs file system if it is not already mounted. However, it cannot do this after the capabilities have been dropped. So perform the mount early, and remove all the other attempts to do the mount.
This is especially important when using network name spaces. Each namespace has its own /sys, so the mount of debugfs in the global namespace is not visible in other namespaces.
Signed-off-by: Andrew Lunn andrew@lunn.ch --- v2: Remove the other calls to mount --- batadv_query.c | 17 ----------------- main.c | 4 ++++ 2 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/batadv_query.c b/batadv_query.c index 1b9e631..1a385c9 100644 --- a/batadv_query.c +++ b/batadv_query.c @@ -90,16 +90,9 @@ int ipv6_to_mac(const struct in6_addr *addr, struct ether_addr *mac)
int batadv_interface_check(const char *mesh_iface) { - char *debugfs_mnt; char full_path[MAX_PATH + 1]; FILE *f;
- debugfs_mnt = debugfs_mount(NULL); - if (!debugfs_mnt) { - fprintf(stderr, "Could not find debugfs path\n"); - return -1; - } - debugfs_make_path(DEBUG_BATIF_PATH_FMT "/" DEBUG_TRANSTABLE_GLOBAL, mesh_iface, full_path, sizeof(full_path)); f = fopen(full_path, "r"); @@ -134,7 +127,6 @@ struct ether_addr *translate_mac(const char *mesh_iface, struct ether_addr *mac) tg_originator, } pos; char full_path[MAX_PATH + 1]; - char *debugfs_mnt; static struct ether_addr in_mac; struct ether_addr *mac_result, *mac_tmp; FILE *f = NULL; @@ -146,10 +138,6 @@ struct ether_addr *translate_mac(const char *mesh_iface, struct ether_addr *mac) memcpy(&in_mac, mac, sizeof(in_mac)); mac_result = &in_mac;
- debugfs_mnt = debugfs_mount(NULL); - if (!debugfs_mnt) - goto out; - debugfs_make_path(DEBUG_BATIF_PATH_FMT "/" DEBUG_TRANSTABLE_GLOBAL, mesh_iface, full_path, sizeof(full_path));
@@ -216,7 +204,6 @@ uint8_t get_tq(const char *mesh_iface, struct ether_addr *mac) orig_tqvalue, } pos; char full_path[MAX_PATH + 1]; - char *debugfs_mnt; static struct ether_addr in_mac; struct ether_addr *mac_tmp; FILE *f = NULL; @@ -228,10 +215,6 @@ uint8_t get_tq(const char *mesh_iface, struct ether_addr *mac)
memcpy(&in_mac, mac, sizeof(in_mac));
- debugfs_mnt = debugfs_mount(NULL); - if (!debugfs_mnt) - goto out; - debugfs_make_path(DEBUG_BATIF_PATH_FMT "/" DEBUG_ORIGINATORS, mesh_iface, full_path, sizeof(full_path));
diff --git a/main.c b/main.c index 9610398..52dca97 100644 --- a/main.c +++ b/main.c @@ -30,6 +30,7 @@ #include <unistd.h> #endif #include "alfred.h" +#include "debugfs.h" #include "packet.h" #include "list.h"
@@ -160,6 +161,9 @@ static struct globals *alfred_init(int argc, char *argv[]) {NULL, 0, NULL, 0}, };
+ /* We need full capabilities to mount debugfs, so do that now */ + debugfs_mount(NULL); + ret = reduce_capabilities(); if (ret < 0) return NULL;
On Wednesday 16 March 2016 20:51:12 Andrew Lunn wrote:
--- a/batadv_query.c +++ b/batadv_query.c @@ -90,16 +90,9 @@ int ipv6_to_mac(const struct in6_addr *addr, struct ether_addr *mac)
int batadv_interface_check(const char *mesh_iface) {
char *debugfs_mnt; char full_path[MAX_PATH + 1]; FILE *f;
debugfs_mnt = debugfs_mount(NULL);
if (!debugfs_mnt) {
fprintf(stderr, "Could not find debugfs path\n");
return -1;
}
[...]
diff --git a/main.c b/main.c index 9610398..52dca97 100644 --- a/main.c +++ b/main.c
[...]
@@ -160,6 +161,9 @@ static struct globals *alfred_init(int argc, char *argv[]) {NULL, 0, NULL, 0}, };
- /* We need full capabilities to mount debugfs, so do that now */
- debugfs_mount(NULL);
- ret = reduce_capabilities(); if (ret < 0) return NULL;
There were some return value checks for the debugfs_mount calls which you've removed. Why isn't here some kind of check with error handling/warning, ...?
Kind regards, Sven
On Fri, Mar 18, 2016 at 12:27:38PM +0100, Sven Eckelmann wrote:
On Wednesday 16 March 2016 20:51:12 Andrew Lunn wrote:
--- a/batadv_query.c +++ b/batadv_query.c @@ -90,16 +90,9 @@ int ipv6_to_mac(const struct in6_addr *addr, struct ether_addr *mac)
int batadv_interface_check(const char *mesh_iface) {
char *debugfs_mnt; char full_path[MAX_PATH + 1]; FILE *f;
debugfs_mnt = debugfs_mount(NULL);
if (!debugfs_mnt) {
fprintf(stderr, "Could not find debugfs path\n");
return -1;
}
[...]
diff --git a/main.c b/main.c index 9610398..52dca97 100644 --- a/main.c +++ b/main.c
[...]
@@ -160,6 +161,9 @@ static struct globals *alfred_init(int argc, char *argv[]) {NULL, 0, NULL, 0}, };
- /* We need full capabilities to mount debugfs, so do that now */
- debugfs_mount(NULL);
- ret = reduce_capabilities(); if (ret < 0) return NULL;
There were some return value checks for the debugfs_mount calls which you've removed. Why isn't here some kind of check with error handling/warning, ...?
Hi Sven
I think the other times debugfs mount was called was just before it was used. So a mount failure means the code that follows is going to fail.
Putting the mount very early in the program means it could be about to do something which does not require debugfs. e.g. run in client mode. So the most we can do a print a warning, something like: "No access to debugfs. Some functionality may not work".
The other option is a bigger change. Move the calls to reduce_capabilities() and debugfs_mount() to later, after the option parsing. We then have a better idea if debugfs is needed or not, and can then print a fatal error.
Andrew
On Wednesday 16 March 2016 20:51:12 Andrew Lunn wrote:
The debugfs helper code has the ability to mount the debugfs file system if it is not already mounted. However, it cannot do this after the capabilities have been dropped. So perform the mount early, and remove all the other attempts to do the mount.
This is especially important when using network name spaces. Each namespace has its own /sys, so the mount of debugfs in the global namespace is not visible in other namespaces.
Signed-off-by: Andrew Lunn andrew@lunn.ch
v2: Remove the other calls to mount
Applied in revision d08b0e0.
Thanks! Simon
b.a.t.m.a.n@lists.open-mesh.org