Stephen Smalley <sds@xxxxxxxxxxxxx> writes:Ok, I know what you are talking about now...
The logic is supposed to map certain attribute changes (mode, owner,SELinux shouldn't apply a permission check for the clearing of the suidOk. Yes. So, to do it efficiently without problem, I'm suggesting the
bit on write or truncate. It should only apply a permission check for
the actual truncate or write operation, and then the clearing of the
suid bit should always be forced if that check passed.
following or something (I don't know whether LSM should do this or not).
selinux_inode_setattr(),
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;
I guess ATTR_FORCE_MASK would be (ATTR_MODE | ATTR_UID | ATTR_GID |
ATTR_ATIME_SET | ATTR_MTIME_SET) or something,
and ATTR_NOT_FORCE_MASK would be ATTR_SIZE or something.
I'm not sure this is the right code what selinux want to do though, but,
I hope it is clear what I want to say. (I'm assuming FILE__WRITE is for
check of ATTR_SIZE)
group, explicit setting of atime or mtime to a specific value rather
than the current time) to the SELinux setattr permission, while mapping
other attribute changes that occur naturally on a write (size, setting
of mtime to current time) to the SELinux write permission. That doesn't
seem clear from using ATTR_FORCE_MASK vs ATTR_NOT_FORCE_MASK above - I'd
use different naming conventions for clarity.
I see. Yes, the naming of this code doesn't matter at all. The code was
just intended to explain what I'm suggesting.
With this change, the caller can pass "(ATTR_SIZE | ATTR_MODE)" orThat was a change in do_truncate(), commit
"(ATTR_SIZE | ATTR_MODE | ATTR_FORCE)" etc. for truncate().
[btw, "(ATTR_SIZE | ATTR_MODE)" is what do_truncate() does currently].
7b82dc0e64e93f430182f36b46b79fcee87d3532.
It makes sense, but no one ever updated selinux_inode_setattr() to match
that change.
I see. Yes, exactly. And for the user of non file owner case, I'm
thinking we would like to pass ATTR_FORCE too.