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 14:49:59 +0900 Message-ID: <878whwuq9k.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> 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]:56933 "EHLO mail.officemail.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752695AbZHGFuB (ORCPT ); Fri, 7 Aug 2009 01:50:01 -0400 In-Reply-To: <871vno1cn7.fsf@devron.myhome.or.jp> (OGAWA Hirofumi's message of "Fri, 07 Aug 2009 13:17:00 +0900") Sender: linux-fsdevel-owner@vger.kernel.org List-ID: OGAWA Hirofumi writes: > Sorry for same argument. I see. However, um... > > I found this piece in security/selinux/hooks.c > > 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. The > definition of ATTR_FORCE is unclear at all, it would be problem. But, > I'm not sure though, I suspect the above code also has problem... I found the SuSv3 says for ftruncate(): Upon successful completion, if fildes refers to a regular file, ftruncate() shall mark for update the last data modification and last file status change timestamps of the file and the S_ISUID and S_ISGID bits of the file mode may be cleared. If the ftruncate() function is unsuccessful, the file is unaffected. And vmtruncate() can return error easily with RLIMIT_FSIZE or ->s_maxbytes. So, I think clearing s[ug]id first may be bad behavior without good reason. Thanks. -- OGAWA Hirofumi