Ack: [Lucid][CVE-2015-3339] fs: take i_mutex during prepare_binprm for set[ug]id executables

Seth Forshee seth.forshee at canonical.com
Mon Apr 27 17:06:46 UTC 2015


On Mon, Apr 27, 2015 at 03:10:37PM +0100, Luis Henriques wrote:
> On Mon, Apr 27, 2015 at 08:46:03AM -0500, Seth Forshee wrote:
> > On Mon, Apr 27, 2015 at 01:43:58PM +0100, Luis Henriques wrote:
> > > From: Jann Horn <jann at thejh.net>
> > > 
> > > This prevents a race between chown() and execve(), where chowning a
> > > setuid-user binary to root would momentarily make the binary setuid
> > > root.
> > > 
> > > This patch was mostly written by Linus Torvalds.
> > > 
> > > Signed-off-by: Jann Horn <jann at thejh.net>
> > > Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
> > > (backported from commit 8b01fc86b9f425899f8a3a8fc1c47d73c2c20543)
> > > [ luis: backport to Lucid:
> > >   - replaced kuid_t/kgid_t by uid_t/gid_t
> > >   - replaced READ_ONCE() by ACCESS_ONCE()
> > >   - replaced task_no_new_privs() by current->no_new_privs
> > 
> > <snip>
> > 
> 
> Doh!  This is a copy&paste problem -- I should have removed this line
> from the Lucid backport.  I'll do that when applying it to Lucid (let
> me know if you rather have me resubmitting with this fixed).
> 
> > > +static void bprm_fill_uid(struct linux_binprm *bprm)
> > > +{
> > > +	struct inode *inode;
> > > +	unsigned int mode;
> > > +	uid_t uid;
> > > +	gid_t gid;
> > > +
> > > +	/* clear any previous set[ug]id data from a previous binary */
> > > +	bprm->cred->euid = current_euid();
> > > +	bprm->cred->egid = current_egid();
> > > +
> > > +	if (bprm->file->f_path.mnt->mnt_flags & MNT_NOSUID)
> > > +		return;
> > 
> > The current->no_new_privs bit is missing here, not sure if it's missing
> > or if the commit log is wrong and that part isn't relevant for lucid.
> > 
> 
> My understanding is that it is not relevant.  If you look at the
> backport for Precise, the "current->no_new_privs" was dropped from the
> prepare_binprm() and moved into bprm_fill_uid().  Since Lucid doesn't
> have this check in prepare_binprm(), I haven't added it into the new
> function.  Actually, struct task_struct in Lucid doesn't even have
> that field.

Well it will be awfully hard to check then :-)

I'm cool with just fixing up the commit message when the patch is
applied.

Acked-by: Seth Forshee <seth.forshee at canonical.com>




More information about the kernel-team mailing list