Re: [PATCH 17/19] mount.cifs: guard against signals by unprivileged users

On Thu, 2010-04-01 at 15:32 -0400, Jeff Layton wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On Sun, 28 Mar 2010 09:34:10 -0400
> Jeff Layton <[email protected]> wrote:
> 
> > On Fri, 26 Mar 2010 10:25:40 -0400
> > Jeff Layton <[email protected]> wrote:
> > 
> > > From: Jeff Layton <[email protected]>
> > > 
> > > If mount.cifs is setuid root, then the unprivileged user who runs the
> > > program can send the mount.cifs process a signal and kill it. This is
> > > not a huge problem unless we happen to be updating the mtab at the
> > > time, in which case the mtab lockfiles might not get cleaned up.
> > > 
> > > To remedy this, have the privileged mount.cifs process set its real
> > > uid to the effective uid (usually, root). This prevents unprivileged
> > > users from being able to signal the process.
> > > 
> > > While we're at it, also mask off signals while we're updating the
> > > mtab. This leaves a SIGKILL by root as the only way to interrupt the
> > > mtab update, but there's really nothing we can do about that.
> > > 
> > > Signed-off-by: Jeff Layton <[email protected]>
> >
> > A little self-review on this patch...
> > 
> > It's probably better not to change the real uid until the mtab is set
> > to be updated, so I'm moving that piece into add_mtab. Doing so very
> > early on like this means that the kernel loses the ability to get the
> > real uid of the user running the mount command.
> > 
> > Replacement patch attached...
> 
> Simo pointed out another problem with this patch. It's possible for
> getpwuid to block for quite a while if using something like NIS, SSSD,
> etc...
> 
> Thus, it's better to move the getusername() call out of the section
> where we hold the mtab lock. New patch attached.

With this change the whole patch set looks good to me.
I haven't had a chance to actually test it though.

Simo.

-- 
Simo Sorce
Samba Team GPL Compliance Officer <[email protected]>
Principal Software Engineer at Red Hat, Inc. <[email protected]>