From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-owner@vger.kernel.org>
Received: from wtarreau.pck.nerim.net ([62.212.114.60]:48962 "EHLO 1wt.eu"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751235AbbJHFLg (ORCPT <rfc822;stable@vger.kernel.org>);
	Thu, 8 Oct 2015 01:11:36 -0400
Date: Thu, 8 Oct 2015 07:11:18 +0200
From: Willy Tarreau <w@1wt.eu>
To: Ben Hutchings <ben@decadent.org.uk>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>,
	stable@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Sasha Levin <sasha.levin@oracle.com>,
	Jiri Slaby <jirislaby@gmail.com>,
	Willy Tarreau <willy@meta-x.org>, Li Zefan <lizefan@huawei.com>
Subject: Re: [PATCHES] Bind mount escape fixes (CVE-2015-2925)
Message-ID: <20151008051118.GA4263@1wt.eu>
References: <87a8s2a7kc.fsf@x220.int.ebiederm.org> <1444266508.2956.228.camel@decadent.org.uk>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1444266508.2956.228.camel@decadent.org.uk>
Sender: stable-owner@vger.kernel.org
List-ID: <stable.vger.kernel.org>

Hi Ben,

On Thu, Oct 08, 2015 at 02:08:28AM +0100, Ben Hutchings wrote:
> For 2.6.32, the first backport looks wrong:
> 
> > --- a/fs/dcache.c
> > +++ b/fs/dcache.c
> > @@ -1910,7 +1910,7 @@ char *__d_path(const struct path *path, struct path *root,
> >         struct dentry *dentry = path->dentry;
> >         struct vfsmount *vfsmnt = path->mnt;
> >         char *end = buffer + buflen;
> > -       char *retval;
> > +       char *retval, *tail;
> >  
> >         spin_lock(&vfsmount_lock);
> >         prepend(&end, &buflen, "\0", 1);
> > @@ -1923,6 +1923,7 @@ char *__d_path(const struct path *path, struct path *root,
> >         /* Get '/' right */
> >         retval = end-1;
> >         *retval = '/';
> > +       tail = end;
> 
> So tail points to the null terminator.
> 
> >         for (;;) {
> >                 struct dentry * parent;
> > @@ -1930,6 +1931,12 @@ char *__d_path(const struct path *path, struct path *root,
> >                 if (dentry == root->dentry && vfsmnt == root->mnt)
> >                         break;
> >                 if (dentry == vfsmnt->mnt_root || IS_ROOT(dentry)) {
> > +                       /* Escaped? */
> > +                       if (dentry != vfsmnt->mnt_root) {
> > +                               retval = tail;
> > +                               *retval = '/';
> 
> Now we overwrite the null terminator.

Good catch!

> > +                               goto out;
> > +                       }
> >                         /* Global root? */
> >                         if (vfsmnt->mnt_parent == vfsmnt) {
> >                                 goto global_root;
> 
> Also, nothing inserts the "(unreachable)" string.  I've attached my
> version, which deals with both of these.

Thanks, I've queued this one instead!

Willy