[lkml]   [2018]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: linux-next: manual merge of the selinux tree with the vfs tree
On Thu, Nov 29, 2018 at 04:57:20PM -0800, Casey Schaufler wrote:

> > Question: what *should* happen if we try to cross into a submount and find
> > that the thing on the other side is already mounted elsewhere, with incompatible
> > LSM options? Ditto for referrals, with an extra twist - what if we are given
> > 3 alternatives, the first two already mounted elsewhere with incompatible
> > options, the third one not mounted anywhere yet?
> I fear that the safe answer and the containers answer are likely
> to differ. The safe answer has to be to refuse the mount.
> > Incidentally, should smack have ->sb_clone_mnt_opts()?
> Probably, but I could never figure out what it was for,
> and haven't identified a problem with not using it.

Transferring the Linux S&M options when crossing into a submount.

Frankly, the set of mount-related hooks is atrocious - way too much
duplication between them (sb_kern_mount vs. sb_set_mnt_opts vs.
sb_parse_opts_str vs. sb_clone_mnt_opts) and that, actually, is the
worst part of safely untangling the mount-API series ;-/

And then there's sb_mount, with 3 instances and arseloads of
races in 2 out of 3. Consider e.g. this:
if (need_dev) {
/* Get mount point or device file. */
if (!dev_name || kern_path(dev_name, LOOKUP_FOLLOW, &path)) {
error = -ENOENT;
goto out;
obj.path1 = path;
requested_dev_name = tomoyo_realpath_from_path(&path);
if (!requested_dev_name) {
error = -ENOENT;
goto out;
in tomoyo. OK, so we do a pathname resolution of dev_name (including
the source in mount --bind case). Then we apply checks to it...
and proceed to...
if (obj.path1.dentry)
... discard the result of lookup. Then the caller proceeds to do
the work, including (at various locations, depending upon the
mount(2) flags, fs type, etc.) looking dev_name up. Could you spell TOCTOU?

Or, for example, this:
if (!dev_name || !*dev_name)
return -EINVAL;

flags &= MS_REC | MS_BIND;

error = kern_path(dev_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
if (error)
return error;

get_buffers(buffer, old_buffer);
error = fn_for_each_confined(label, profile,
match_mnt(profile, path, buffer, &old_path, old_buffer,
NULL, flags, NULL, false));
put_buffers(buffer, old_buffer);
Same story, same TOCTOU race, this time in apparmour...

 \ /
  Last update: 2018-11-30 02:28    [W:0.060 / U:0.220 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site