From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932602AbcLLKqt (ORCPT ); Mon, 12 Dec 2016 05:46:49 -0500 Received: from mx2.suse.de ([195.135.220.15]:59460 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932196AbcLLKqq (ORCPT ); Mon, 12 Dec 2016 05:46:46 -0500 Date: Mon, 12 Dec 2016 11:46:43 +0100 From: Jan Kara To: Cong Wang Cc: Mark Salyzyn , LKML , aneesh.kumar@linux.vnet.ibm.com, Jan Kara Subject: Re: CVE-2016-7097 causes acl leak Message-ID: <20161212104643.GB1213@quack2.suse.cz> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun 11-12-16 16:34:31, Cong Wang wrote: > On Mon, Dec 5, 2016 at 9:16 AM, Mark Salyzyn 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 SUSE Labs, CR