Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1541211
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: CVE-2016-7097 causes acl leak |
| Date | 2016-12-13 17:10 +0100 |
| Message-ID | <sNXOp-3i2-3@gated-at.bofh.it> (permalink) |
| References | <sL55L-7o6-23@gated-at.bofh.it> <sNmOS-5RV-7@gated-at.bofh.it> <sNJ8K-2DO-15@gated-at.bofh.it> <sNOL7-6aF-7@gated-at.bofh.it> <sNXEJ-2Zs-25@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
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
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
Re: CVE-2016-7097 causes acl leak Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-12 01:40 +0100
Re: CVE-2016-7097 causes acl leak Jan Kara <jack@suse.cz> - 2016-12-12 11:50 +0100
Re: CVE-2016-7097 causes acl leak Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-12 22:20 +0100
Re: CVE-2016-7097 causes acl leak Mark Salyzyn <salyzyn@android.com> - 2016-12-13 01:30 +0100
Re: CVE-2016-7097 causes acl leak Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-13 07:30 +0100
Re: CVE-2016-7097 causes acl leak Jan Kara <jack@suse.cz> - 2016-12-13 12:30 +0100
Re: CVE-2016-7097 causes acl leak Cong Wang <xiyou.wangcong@gmail.com> - 2016-12-14 01:00 +0100
Re: CVE-2016-7097 causes acl leak Mark Salyzyn <salyzyn@android.com> - 2016-12-13 17:00 +0100
Re: CVE-2016-7097 causes acl leak Jan Kara <jack@suse.cz> - 2016-12-13 17:10 +0100
Re: CVE-2016-7097 causes acl leak Mark Salyzyn <salyzyn@android.com> - 2016-12-14 00:50 +0100
Re: CVE-2016-7097 causes acl leak Greg KH <gregkh@linuxfoundation.org> - 2016-12-14 01:10 +0100
Re: CVE-2016-7097 causes acl leak Mark Salyzyn <salyzyn@android.com> - 2016-12-14 21:30 +0100
Re: CVE-2016-7097 causes acl leak Greg KH <gregkh@linuxfoundation.org> - 2016-12-15 00:40 +0100
Re: CVE-2016-7097 causes acl leak Mark Salyzyn <salyzyn@android.com> - 2016-12-15 16:30 +0100
Re: CVE-2016-7097 causes acl leak Jan Kara <jack@suse.cz> - 2016-12-15 17:40 +0100
Re: CVE-2016-7097 causes acl leak Jan Kara <jack@suse.cz> - 2016-12-13 12:20 +0100
csiph-web