lkml.org 
[lkml]   [2016]   [Dec]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: CVE-2016-7097 causes acl leak
On Tue 13-12-16 07:55:23, Mark Salyzyn wrote:
> On 12/12/2016 10:26 PM, Cong Wang wrote:
> >On Mon, Dec 12, 2016 at 4:26 PM, Mark Salyzyn <salyzyn@android.com> wrote:
> >>The leaks were introduced in 9p, gfs2, jfs and xfs drivers only.
> >
> >Only the 9p case is obvious to me:
> >
> >diff --git a/fs/9p/acl.c b/fs/9p/acl.c
> >index b3c2cc7..082d227 100644
> >--- a/fs/9p/acl.c
> >+++ b/fs/9p/acl.c
> >@@ -277,6 +277,7 @@ static int v9fs_xattr_set_acl(const struct
> >xattr_handler *handler,
> > case ACL_TYPE_ACCESS:
> > if (acl) {
> > struct iattr iattr;
> >+ struct posix_acl *old_acl = acl;
> >
> > retval = posix_acl_update_mode(inode,
> >&iattr.ia_mode, &acl);
> > if (retval)
> >@@ -287,6 +288,7 @@ static int v9fs_xattr_set_acl(const struct
> >xattr_handler *handler,
> > * by the mode bits. So don't
> > * update ACL.
> > */
> >+ posix_acl_release(old_acl);
> > value = NULL;
> > size = 0;
> > }
> >
> >
> >The rest are anti-pattern (modifying parameters on stack via address)
> >but look correct
> I chose to modify posix_acl_update_mode as follows and set release_acl in
> the specific drivers, it clears the *acl reference to nul preventing
> posix_acl_release from functioning in these other driver paths:
>
> */
> int posix_acl_update_mode(struct inode *inode, umode_t *mode_p,
> struct posix_acl **acl, ++++bool release_acl)
> {
> umode_t mode = inode->i_mode;
> int error;
>
> error = posix_acl_equiv_mode(*acl, &mode);
> if (error < 0)
> return error;
> if (error == 0) +{
> + if (release_acl)
> + posix_acl_release(*acl);
> *acl = NULL;
> + }
> if (!in_group_p(inode->i_gid) &&
> !capable_wrt_inode_uidgid(inode, CAP_FSETID))
> mode &= ~S_ISGID;
> *mode_p = mode;
> return 0;
> }
> EXPORT_SYMBOL(posix_acl_update_mode);

I much prefer what Cong Wang did. These bool parameters which subtly change
the function behavior are too easy to screw up and it is not really
visible by just looking at the function call...

Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR

\
 
 \ /
  Last update: 2016-12-13 17:08    [W:0.084 / U:1.220 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site