linux-kernel - Re: [PATCH v2] NFS: Avoid cross-structure casting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1492711460.82584.0.camel@primarydata.com>
Date:   Thu, 20 Apr 2017 18:04:23 +0000
From:   Trond Myklebust <trondmy@...marydata.com>
To:     "keescook@...omium.org" <keescook@...omium.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "anna.schumaker@...app.com" <anna.schumaker@...app.com>,
        Trond Myklebust <trondmy@...marydata.com>,
        "neilb@...e.com" <neilb@...e.com>,
        "linux-nfs@...r.kernel.org" <linux-nfs@...r.kernel.org>
Subject: Re: [PATCH v2] NFS: Avoid cross-structure casting

On Wed, 2017-04-12 at 11:59 -0700, Kees Cook wrote:
> On Wed, Apr 5, 2017 at 8:29 AM, Kees Cook <keescook@...omium.org>
> wrote:
> > When the call to nfs_devname() fails, the error path attempts to
> > retain
> > the error via the mnt variable, but this requires a cast across
> > very
> > different types (char * to struct vfsmount *), which the upcoming
> > structure layout randomization plugin flags as being potentially
> > dangerous in the face of randomization. This is a false positive,
> > but
> > what this code actually wants to do is retain the error value, so
> > this
> > patch explicitly sets it, instead of using what seems to be an
> > unexpected cast.
> > 
> > Signed-off-by: Kees Cook <keescook@...omium.org>
> 
> If I can get an Acked-by on this, I could push it via the gcc-plugin
> tree.
> 
> Thanks!
> 
> -Kees
> 
> > ---
> > v2: duh, use ERR_CAST. thanks neilb!
> > ---
> >  fs/nfs/namespace.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
> > index 786f17580582..8ca5d147124d 100644
> > --- a/fs/nfs/namespace.c
> > +++ b/fs/nfs/namespace.c
> > @@ -259,9 +259,10 @@ struct vfsmount *nfs_do_submount(struct dentry
> > *dentry, struct nfs_fh *fh,
> >         if (page == NULL)
> >                 goto out;
> >         devname = nfs_devname(dentry, page, PAGE_SIZE);
> > -       mnt = (struct vfsmount *)devname;
> > -       if (IS_ERR(devname))
> > +       if (IS_ERR(devname)) {
> > +               mnt = ERR_CAST(devname);
> >                 goto free_page;
> > +       }
> >         mnt = nfs_do_clone_mount(NFS_SB(dentry->d_sb), devname,
> > &mountdata);
> >  free_page:
> >         free_page((unsigned long)page);
> > --
> > 2.7.4
> > 
> > 
> > --
> > Kees Cook
> > Pixel Security
> 

Acked-by: Trond Myklebust <trond.myklebust@...marydata.com>

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@...marydata.com