OGAWA Hirofumi wrote:
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx> writes:
Amerigo Wang <amwang@xxxxxxxxxx> 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.
I am not sure if I understand you correctly... You must be referring
notify_change(), it seems to do what you said.
But clearly ATTR_FORCE is the way to bypass the security module on
purpose. I agree that we perhaps should have some wrapper function to do
this (instead of calling notify_change() twice), but currently this is fine.
--
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
