* Question regarding shared subtree mount flags.
@ 2012-02-28 13:33 Tetsuo Handa
2012-02-29 5:59 ` Ram Pai
0 siblings, 1 reply; 2+ messages in thread
From: Tetsuo Handa @ 2012-02-28 13:33 UTC (permalink / raw)
To: linuxram; +Cc: linux-fsdevel
I noticed that TOMOYO's mount flag checking priority
if (flags & MS_REMOUNT) {
type = tomoyo_mounts[TOMOYO_MOUNT_REMOUNT];
flags &= ~MS_REMOUNT;
}
if (flags & MS_MOVE) {
type = tomoyo_mounts[TOMOYO_MOUNT_MOVE];
flags &= ~MS_MOVE;
}
if (flags & MS_BIND) {
type = tomoyo_mounts[TOMOYO_MOUNT_BIND];
flags &= ~MS_BIND;
}
if (flags & MS_UNBINDABLE) {
type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_UNBINDABLE];
flags &= ~MS_UNBINDABLE;
}
if (flags & MS_PRIVATE) {
type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_PRIVATE];
flags &= ~MS_PRIVATE;
}
if (flags & MS_SLAVE) {
type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_SLAVE];
flags &= ~MS_SLAVE;
}
if (flags & MS_SHARED) {
type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_SHARED];
flags &= ~MS_SHARED;
}
if (!type)
type = "<NULL>";
idx = tomoyo_read_lock();
error = tomoyo_mount_acl(&r, dev_name, path, type, flags);
tomoyo_read_unlock(idx);
is wrong and it need to follow priority used by do_mount().
if (flags & MS_REMOUNT)
retval = do_remount(&path, flags & ~MS_REMOUNT, mnt_flags,
data_page);
else if (flags & MS_BIND)
retval = do_loopback(&path, dev_name, flags & MS_REC);
else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
retval = do_change_type(&path, flags);
else if (flags & MS_MOVE)
retval = do_move_mount(&path, dev_name);
I got a question regarding priority among MS_SHARED, MS_PRIVATE, MS_SLAVE and
MS_UNBINDABLE. I'm wondering which flag TOMOYO should use for permission check
if more than one of MS_SHARED, MS_PRIVATE, MS_SLAVE or MS_UNBINDABLE is passed.
commit 07b20889 "[PATCH] beginning of the shared-subtree proper"
commit 03e06e68 "[PATCH] introduce shared mounts"
commit a58b0eb8 "[PATCH] introduce slave mounts"
commit 9676f0c6 "[PATCH] unbindable mounts"
introduced these flags and
commit 7a2e8a8f "VFS: Sanity check mount flags passed to change_mnt_propagation()"
clarified that these flags must be exclusively passed.
Commit 7a2e8a8f went to 2.6.36 but has not gone to 2.6.27 nor 2.6.32.
Is this simply because commit 7a2e8a8f was not proposed for backporting?
If so, is security_sb_mount() allowed to return -EINVAL if more than one of
these flags are passed (like shown below) so that TOMOYO will not generate
inaccurate audit logs?
if (flags & MS_REMOUNT) {
type = tomoyo_mounts[TOMOYO_MOUNT_REMOUNT];
flags &= ~MS_REMOUNT;
} else if (flags & MS_BIND) {
type = tomoyo_mounts[TOMOYO_MOUNT_BIND];
flags &= ~MS_BIND;
} else if (flags & MS_SHARED) {
type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_SHARED];
flags &= ~MS_SHARED;
if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
return -EINVAL;
} else if (flags & MS_PRIVATE) {
type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_PRIVATE];
flags &= ~MS_PRIVATE;
if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
return -EINVAL;
} else if (flags & MS_SLAVE) {
type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_SLAVE];
flags &= ~MS_SLAVE;
if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
return -EINVAL;
} else if (flags & MS_UNBINDABLE) {
type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_UNBINDABLE];
flags &= ~MS_UNBINDABLE;
if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
return -EINVAL;
} else if (flags & MS_MOVE) {
type = tomoyo_mounts[TOMOYO_MOUNT_MOVE];
flags &= ~MS_MOVE;
}
if (!type)
type = "<NULL>";
idx = tomoyo_read_lock();
error = tomoyo_mount_acl(&r, dev_name, path, type, flags);
tomoyo_read_unlock(idx);
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: Question regarding shared subtree mount flags.
2012-02-28 13:33 Question regarding shared subtree mount flags Tetsuo Handa
@ 2012-02-29 5:59 ` Ram Pai
0 siblings, 0 replies; 2+ messages in thread
From: Ram Pai @ 2012-02-29 5:59 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: linux-fsdevel
On Tue, Feb 28, 2012 at 10:33:15PM +0900, Tetsuo Handa wrote:
> I noticed that TOMOYO's mount flag checking priority
>
> if (flags & MS_REMOUNT) {
> type = tomoyo_mounts[TOMOYO_MOUNT_REMOUNT];
> flags &= ~MS_REMOUNT;
> }
> if (flags & MS_MOVE) {
> type = tomoyo_mounts[TOMOYO_MOUNT_MOVE];
> flags &= ~MS_MOVE;
> }
> if (flags & MS_BIND) {
> type = tomoyo_mounts[TOMOYO_MOUNT_BIND];
> flags &= ~MS_BIND;
> }
> if (flags & MS_UNBINDABLE) {
> type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_UNBINDABLE];
> flags &= ~MS_UNBINDABLE;
> }
> if (flags & MS_PRIVATE) {
> type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_PRIVATE];
> flags &= ~MS_PRIVATE;
> }
> if (flags & MS_SLAVE) {
> type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_SLAVE];
> flags &= ~MS_SLAVE;
> }
> if (flags & MS_SHARED) {
> type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_SHARED];
> flags &= ~MS_SHARED;
> }
> if (!type)
> type = "<NULL>";
> idx = tomoyo_read_lock();
> error = tomoyo_mount_acl(&r, dev_name, path, type, flags);
> tomoyo_read_unlock(idx);
>
> is wrong and it need to follow priority used by do_mount().
>
> if (flags & MS_REMOUNT)
> retval = do_remount(&path, flags & ~MS_REMOUNT, mnt_flags,
> data_page);
> else if (flags & MS_BIND)
> retval = do_loopback(&path, dev_name, flags & MS_REC);
> else if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
> retval = do_change_type(&path, flags);
> else if (flags & MS_MOVE)
> retval = do_move_mount(&path, dev_name);
>
> I got a question regarding priority among MS_SHARED, MS_PRIVATE, MS_SLAVE and
> MS_UNBINDABLE. I'm wondering which flag TOMOYO should use for permission check
> if more than one of MS_SHARED, MS_PRIVATE, MS_SLAVE or MS_UNBINDABLE is passed.
>
> commit 07b20889 "[PATCH] beginning of the shared-subtree proper"
> commit 03e06e68 "[PATCH] introduce shared mounts"
> commit a58b0eb8 "[PATCH] introduce slave mounts"
> commit 9676f0c6 "[PATCH] unbindable mounts"
>
> introduced these flags and
>
> commit 7a2e8a8f "VFS: Sanity check mount flags passed to change_mnt_propagation()"
>
> clarified that these flags must be exclusively passed.
right. It makes more sense for a user to change a vfsmount to one type at a given
time.
>
> Commit 7a2e8a8f went to 2.6.36 but has not gone to 2.6.27 nor 2.6.32.
> Is this simply because commit 7a2e8a8f was not proposed for backporting?
looks like it. No one saw the need to backport that patch.
>
> If so, is security_sb_mount() allowed to return -EINVAL if more than one of
> these flags are passed (like shown below) so that TOMOYO will not generate
> inaccurate audit logs?
>
> if (flags & MS_REMOUNT) {
> type = tomoyo_mounts[TOMOYO_MOUNT_REMOUNT];
> flags &= ~MS_REMOUNT;
> } else if (flags & MS_BIND) {
> type = tomoyo_mounts[TOMOYO_MOUNT_BIND];
> flags &= ~MS_BIND;
> } else if (flags & MS_SHARED) {
> type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_SHARED];
> flags &= ~MS_SHARED;
> if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
> return -EINVAL;
> } else if (flags & MS_PRIVATE) {
> type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_PRIVATE];
> flags &= ~MS_PRIVATE;
> if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
> return -EINVAL;
> } else if (flags & MS_SLAVE) {
> type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_SLAVE];
> flags &= ~MS_SLAVE;
> if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
> return -EINVAL;
> } else if (flags & MS_UNBINDABLE) {
> type = tomoyo_mounts[TOMOYO_MOUNT_MAKE_UNBINDABLE];
> flags &= ~MS_UNBINDABLE;
> if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
> return -EINVAL;
> } else if (flags & MS_MOVE) {
> type = tomoyo_mounts[TOMOYO_MOUNT_MOVE];
> flags &= ~MS_MOVE;
> }
I think that is right. However the code can be further reorganized to check for
(MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE) only ones, instead of checking
under every else if.
RP
> if (!type)
> type = "<NULL>";
> idx = tomoyo_read_lock();
> error = tomoyo_mount_acl(&r, dev_name, path, type, flags);
> tomoyo_read_unlock(idx);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-02-29 6:00 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-28 13:33 Question regarding shared subtree mount flags Tetsuo Handa
2012-02-29 5:59 ` Ram Pai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).