diff mbox

[3/3] alfred: Mount debugfs before reducing capabilities

Message ID 1453953196-29943-3-git-send-email-andrew@lunn.ch (mailing list archive)
State Superseded, archived
Delegated to: Simon Wunderlich
Headers

Commit Message

Andrew Lunn Jan. 28, 2016, 3:53 a.m. UTC 
  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.

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>
---
 main.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Simon Wunderlich Jan. 29, 2016, 12:14 p.m. UTC | #1 
On Thursday 28 January 2016 04:53:15 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.
> 
> 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>
> ---
>  main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/main.c b/main.c
> index 452d9ae..b1c5ec5 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;

Can't we remove the other calls to debugfs_mount() ? I see 3 more calls in 
alfred ...

Cheers,
     Simon
Andrew Lunn Feb. 2, 2016, 2:25 a.m. UTC | #2 
On Fri, Jan 29, 2016 at 01:14:53PM +0100, Simon Wunderlich wrote:
> On Thursday 28 January 2016 04:53:15 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.
> > 
> > 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>
> > ---
> >  main.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/main.c b/main.c
> > index 452d9ae..b1c5ec5 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;
> 
> Can't we remove the other calls to debugfs_mount() ? I see 3 more calls in 
> alfred ...

Yes, the other calls within this binary are probably failing, due to
reduced capabilities. I can remove them in a v3 patch.

	Andrew
Simon Wunderlich March 10, 2016, 3:09 p.m. UTC | #3 
Hi Andrew,

On Tuesday 02 February 2016 03:25:12 Andrew Lunn wrote:
> On Fri, Jan 29, 2016 at 01:14:53PM +0100, Simon Wunderlich wrote:
> > On Thursday 28 January 2016 04:53:15 Andrew Lunn wrote:
> [...]
> > > @@ -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;
> > 
> > Can't we remove the other calls to debugfs_mount() ? I see 3 more calls in
> > alfred ...
> 
> Yes, the other calls within this binary are probably failing, due to
> reduced capabilities. I can remove them in a v3 patch.
> 
> 	Andrew

Are you still planning on sending a fixed patchset? :)

Thanks!
    Simon
Andrew Lunn March 10, 2016, 3:49 p.m. UTC | #4 
On Thu, Mar 10, 2016 at 04:09:18PM +0100, Simon Wunderlich wrote:
> Hi Andrew,
> 
> On Tuesday 02 February 2016 03:25:12 Andrew Lunn wrote:
> > On Fri, Jan 29, 2016 at 01:14:53PM +0100, Simon Wunderlich wrote:
> > > On Thursday 28 January 2016 04:53:15 Andrew Lunn wrote:
> > [...]
> > > > @@ -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;
> > > 
> > > Can't we remove the other calls to debugfs_mount() ? I see 3 more calls in
> > > alfred ...
> > 
> > Yes, the other calls within this binary are probably failing, due to
> > reduced capabilities. I can remove them in a v3 patch.
> > 
> > 	Andrew
> 
> Are you still planning on sending a fixed patchset? :)

Hi Simon

I have a fixed up version. But at the moment, it is not clear what is
happening with the kernel patches. Maybe we need to change the debugfs
API in order to make it easier to build with older kernels?

    Andrew
diff mbox

Patch

diff --git a/main.c b/main.c
index 452d9ae..b1c5ec5 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;