[lxc-devel] [PATCH] lxc-monitord.log should not be created with mode 0666

[Dwight Engen]

On Tue, 28 Jan 2014 16:38:46 +0100
Robert Vogelgesang <vogel at users.sourceforge.net> wrote:

> Hi,
> 
> On Tue, Jan 28, 2014 at 10:22:36AM -0500, Dwight Engen wrote:
> > On Tue, 28 Jan 2014 12:01:54 +0100
> > Robert Vogelgesang <vogel at users.sourceforge.net> wrote:
> > 
> > > lxc_monitord_spawn() in src/lxc/monitor.c contained "umask(0);",
> > > and because of this, lxc-monitord created lxc-monitord.log with
> > > mode 0666. World-writeable log files are bad, so remove this
> > > umask(0).
> > 
> > I don't have a problem with this change per-se, but I think the
> > reason the log is 0666 is because that is the mode passed in
> > log.c:log_open(). Did your change actually have the effect you were
> > after?
> 
> yes, removing "umask(0);" fixed the mode for newly-created logs.
> 
> > 
> > If we don't want logs world readable then I'd suggest we should
> > change the mode log_open() so it takes effect for all lxc logs.
> 
> This is not neccessary, because the mode passed to open(2) is modified
> by the current umask.  If you have a umask of 022, you don't get
> files with mode 0666.

Right, but if the original caller for some reason has a umask of 0,
wouldn't the logfile still be 0666, so do we care about that?

> The ONLY reason for a zero umask would be if we want/need mode 0666 on
> the unix socket files and named pipes.  If this should be the case,
> the "umask(0);" should be moved to lxc_monitord_create() in
> lxc_monitord.c, or to main() in lxc_monitord.c, just before calling
> lxc_monitord_create().
>
> My (admittedly few) tests so far indicate that we don't need a zero
> umask.

Yep, I agree, and I think it can just be removed. The only thing
monitord is creating is monitor-fifo and it is 0600. The unix socket is
abstract so I don't think umask applies there, so:

Acked-by: Dwight Engen <dwight.engen at oracle.com>

Thanks!

> 	Robert
> 
> > 
> > > Signed-off-by: Robert Vogelgesang <vogel at users.sourceforge.net>
> > > 
> > > diff -u lxc-lxc-1.0.0.beta3/src/lxc/monitor.c.orig
> > > lxc-lxc-1.0.0.beta3/src/lxc/monitor.c ---
> > > lxc-lxc-1.0.0.beta3/src/lxc/monitor.c.orig	2014-01-27
> > > 14:52:20.000000000 +0100 +++
> > > lxc-lxc-1.0.0.beta3/src/lxc/monitor.c	2014-01-28
> > > 11:57:26.692102355 +0100 @@ -325,7 +325,6 @@ exit(EXIT_SUCCESS); }
> > >  
> > > -	umask(0);
> > >  	if (setsid() < 0) {
> > >  		SYSERROR("failed to setsid");
> > >  		exit(EXIT_FAILURE);
> > > _______________________________________________
> > > lxc-devel mailing list
> > > lxc-devel at lists.linuxcontainers.org
> > > http://lists.linuxcontainers.org/listinfo/lxc-devel