From mboxrd@z Thu Jan 1 00:00:00 1970 From: OGAWA Hirofumi Subject: Re: [patch 12/12] vfs: allow file truncations when both suid and write permissions set Date: Fri, 07 Aug 2009 20:25:04 +0900 Message-ID: <87ab2bsw6n.fsf@devron.myhome.or.jp> References: <200908062310.n76NAIEo013014@imap1.linux-foundation.org> <87ocqs2w17.fsf@devron.myhome.or.jp> <4A7B9DC5.7080700@redhat.com> <871vno1cn7.fsf@devron.myhome.or.jp> <4A7BF394.8010302@redhat.com> <87prb7sx8p.fsf@devron.myhome.or.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: akpm@linux-foundation.org, viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org, eparis@redhat.com, esandeen@redhat.com, eteo@redhat.com To: Amerigo Wang Return-path: Received: from mail.parknet.ad.jp ([210.171.162.6]:39383 "EHLO mail.officemail.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754082AbZHGLZG (ORCPT ); Fri, 7 Aug 2009 07:25:06 -0400 In-Reply-To: <87prb7sx8p.fsf@devron.myhome.or.jp> (OGAWA Hirofumi's message of "Fri, 07 Aug 2009 20:02:14 +0900") Sender: linux-fsdevel-owner@vger.kernel.org List-ID: OGAWA Hirofumi writes: > Amerigo Wang writes: > >>> static int selinux_inode_setattr(struct dentry *dentry, struct iattr *iattr) >>> { >>> const struct cred *cred = current_cred(); >>> >>> if (iattr->ia_valid & ATTR_FORCE) >>> return 0; >>> >>> if (iattr->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID | >>> ATTR_ATIME_SET | ATTR_MTIME_SET)) >>> return dentry_has_perm(cred, NULL, dentry, FILE__SETATTR); >>> >>> return dentry_has_perm(cred, NULL, dentry, FILE__WRITE); >>> } >>> >>> I guess it's assuming the ia_valid doesn't have (ATTR_MODE | ATTR_SIZE), >>> but truncate() already does it, I don't know whether it's ok. >> >> No, here we should only force ATTR_KILL_SUID and/or ATTR_KILL_SGID. >> do_truncate() has ATTR_SIZE and ATTR_FILE. > > I guess security module should do, > > ia_valid = iattr->ia_valid; > if (!(ia_valid & ATTR_FORCE) && (ia_valid & ATTR_FORCE_MASK)) { > err = dentry_has_perm(cred, NULL, dentry, FILE__SETATTR); > if (err) > return err; > ia_valid &= ~ATTR_FORCE_MASK; > } > if (ia_valid & ATTR_NOT_FORCE_MASK) > err = dentry_has_perm(cred, NULL, dentry, FILE__WRITE); > return err; > > or something. Because do_truncate() already do (ATTR_MODE | ATTR_SIZE) > without ATTR_FORCE. BTW, it seems original code doesn't check ATTR_SIZE if (ATTR_MODE | ATTR_SIZE), right? So, ATTR_FORCE is just forcing ATTR_MODE, but I guess that's problem itself. Thanks. -- OGAWA Hirofumi