Messages in this thread | | | Date | Mon, 12 Dec 2016 11:46:43 +0100 | From | Jan Kara <> | Subject | Re: CVE-2016-7097 causes acl leak |
| |
On Sun 11-12-16 16:34:31, Cong Wang wrote: > On Mon, Dec 5, 2016 at 9:16 AM, Mark Salyzyn <salyzyn@android.com> wrote: > > Commit 073931017b49d9458aa351605b43a7e34598caef has several occurrences of > > an acl leak. > > > > posix_acl_update_mode(inose, &mode, &acl); > > > > . . . > > > > posix_acl_release(acl); > > > > > > acl is NULLed in posix_acl_update_mode to signal caller to not update the > > acl; but because it is nulled, it is never released. > > I think you blame the wrong commit, this leak exists before that commit. > Looks like we should just release it before NULL'ing.
So I agree with you the mentioned commit didn't change anything. I took care to keep the previous behavior wrt NULLing the acl pointer (obviously I could have made mistake somewhere but I don't see where). However your patch is definitely wrong. See e.g. fs/ext2/acl.c: ext2_set_acl() - there we really want to just clear the pointer. We release the ACL in the caller of ext2_set_acl().
Mark why do you think we are leaking ACL references and can you be more specific where exactly it happens?
Honza
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c > index 5955220..edd862a 100644 > --- a/fs/posix_acl.c > +++ b/fs/posix_acl.c > @@ -648,8 +648,10 @@ int posix_acl_update_mode(struct inode *inode, > umode_t *mode_p, > error = posix_acl_equiv_mode(*acl, &mode); > if (error < 0) > return error; > - if (error == 0) > + if (error == 0) { > + posix_acl_release(*acl); > *acl = NULL; > + } > if (!in_group_p(inode->i_gid) && > !capable_wrt_inode_uidgid(inode, CAP_FSETID)) > mode &= ~S_ISGID; -- Jan Kara <jack@suse.com> SUSE Labs, CR
| |