[v2,2/2] alfred: Mount debugfs before reducing capabilities

  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(-)
  

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