OGAWA Hirofumi wrote:
Amerigo Wang <amwang@xxxxxxxxxx> writes:
OGAWA Hirofumi wrote:
akpm@xxxxxxxxxxxxxxxxxxxx writes:
diff -puN fs/open.c~vfs-allow-file-truncations-when-both-suid-and-write-permissions-set fs/open.c
--- a/fs/open.c~vfs-allow-file-truncations-when-both-suid-and-write-permissions-set
+++ a/fs/open.c
@@ -213,11 +213,15 @@ int do_truncate(struct dentry *dentry, l
newattrs.ia_valid |= ATTR_FILE;
}
+ mutex_lock(&dentry->d_inode->i_mutex);
/* Remove suid/sgid on truncate too */
- newattrs.ia_valid |= should_remove_suid(dentry);
+ err = dentry_remove_suid(dentry);
+ if (err)
+ goto unlock;
Can't we use ATTR_FORCE for this? Because this calls notify_change()
twice, and I guess this removes s[ug]id even if vmtruncate() (or in
future ->truncate() may return error) or something returned error.
I think it would not be good behavior.
Hi, please check:
http://lkml.org/lkml/2009/7/1/459
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.
No, here we should only force ATTR_KILL_SUID and/or ATTR_KILL_SGID.
do_truncate() has ATTR_SIZE and ATTR_FILE.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
