Linux-Security-Module Archive on lore.kernel.org
 help / Atom feed
* Re: linux-next: manual merge of the selinux tree with the vfs tree
       [not found]           ` <20181129235130.GI2217@ZenIV.linux.org.uk>
@ 2018-11-30  0:57             ` Casey Schaufler
  2018-11-30  1:27               ` Al Viro
  0 siblings, 1 reply; 4+ messages in thread
From: Casey Schaufler @ 2018-11-30  0:57 UTC (permalink / raw)
  To: Al Viro, Paul Moore
  Cc: omosnace, sfr, linux-next, linux-kernel, dhowells, selinux,
	linux-fsdevel, LSM

On 11/29/2018 3:51 PM, Al Viro wrote:

I've added linux-security-module to the CC list.

> On Thu, Nov 29, 2018 at 05:23:24PM -0500, Paul Moore wrote:
>
>>> OK, I will verify that the SELinux submount fix rebased on top of
>>> vfs/work.mount in the way I suggested above passes the same testing
>>> (seliinux-testsuite + NFS crossmnt reproducer). I am now building two
>>> kernels (vfs/work.mount with and without the fix) to test. Let me know
>>> if there is anything more to do.
>> Thanks.
>>
>> The big thing is just making sure that we don't regress on the fix in
>> selinux/next if/when David's mount rework hits Linus' tree.
> FWIW, the whole thing is getting massaged/reordered/etc. and I would
> like some input from you guys at some point - assuming that I recover
> the ability to talk about LSM without obscenities...
>
> 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.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: linux-next: manual merge of the selinux tree with the vfs tree
  2018-11-30  0:57             ` linux-next: manual merge of the selinux tree with the vfs tree Casey Schaufler
@ 2018-11-30  1:27               ` Al Viro
  2018-11-30  1:36                 ` Al Viro
  0 siblings, 1 reply; 4+ messages in thread
From: Al Viro @ 2018-11-30  1:27 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Paul Moore, omosnace, sfr, linux-next, linux-kernel, dhowells,
	selinux, linux-fsdevel, LSM

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)
                path_put(&obj.path1);
... 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);
        path_put(&old_path);
Same story, same TOCTOU race, this time in apparmour...

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: linux-next: manual merge of the selinux tree with the vfs tree
  2018-11-30  1:27               ` Al Viro
@ 2018-11-30  1:36                 ` Al Viro
  0 siblings, 0 replies; 4+ messages in thread
From: Al Viro @ 2018-11-30  1:36 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Paul Moore, omosnace, sfr, linux-next, linux-kernel, dhowells,
	selinux, linux-fsdevel, LSM

On Fri, Nov 30, 2018 at 01:27:07AM +0000, Al Viro wrote:

> And then there's sb_mount, with 3 instances and arseloads of
> races in 2 out of 3.

PS: the 3rd one (in selinux) is, AFAICS, TOCTOU-free, because
it ignores everything except the mountpoint, which is already
looked up by the caller.  No idea what any out-of-tree ones do,
of course, but judging by the in-tree sample...

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: linux-next: manual merge of the selinux tree with the vfs tree
       [not found]                   ` <20181205161601.GW2217@ZenIV.linux.org.uk>
@ 2018-12-05 21:58                     ` Casey Schaufler
  0 siblings, 0 replies; 4+ messages in thread
From: Casey Schaufler @ 2018-12-05 21:58 UTC (permalink / raw)
  To: Al Viro, Ondrej Mosnacek
  Cc: Paul Moore, Stephen Rothwell, linux-next,
	Linux kernel mailing list, David Howells, selinux, linux-fsdevel,
	LSM

On 12/5/2018 8:16 AM, Al Viro wrote:
> On Wed, Dec 05, 2018 at 10:37:56AM +0100, Ondrej Mosnacek wrote:
>
>> I just tested the Q28 branch rebased onto a recent Fedora rawhide
>> kernel (4.20.0-0.rc5.git0.1) and that code seems to be working fine.

Not so good with Smack.

	# mount -t tmpfs -o size=512m,smackfsroot=Pop tmpfs /mnt
	# attr -S -g SMACK64 /mnt
	Attribute "SMACK64" had a 1 byte value for /mnt:
	_
	#

attr should have reported a 3 byte value "Pop".


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20181127115246.00967523@canb.auug.org.au>
     [not found] ` <CAFqZXNs6XTOx2-EQ5P9bK3vPsPby7rzryHPqzqsVD9XsbsG20w@mail.gmail.com>
     [not found]   ` <20181127225013.133adc7d@canb.auug.org.au>
     [not found]     ` <CAHC9VhSFSxBc+-O=GQMkiyiQz3S+_ZreicLhsoAuvR-oq2mi6g@mail.gmail.com>
     [not found]       ` <CAFqZXNuZCof=7CtyMq8JDyAgRmONYRZhhqr6bFVy9-F70-Uwrw@mail.gmail.com>
     [not found]         ` <CAHC9VhRthWKfgYenGq6Az+jVQ+76wCeBhBcMyo9zZwv+nBhy3A@mail.gmail.com>
     [not found]           ` <20181129235130.GI2217@ZenIV.linux.org.uk>
2018-11-30  0:57             ` linux-next: manual merge of the selinux tree with the vfs tree Casey Schaufler
2018-11-30  1:27               ` Al Viro
2018-11-30  1:36                 ` Al Viro
     [not found]         ` <CAFqZXNu_=MxcX5jvX4y=5nrCqi7_btOGVccbH1NZ=xm3TGvaYQ@mail.gmail.com>
     [not found]           ` <CAFqZXNv642dfT0PpB0M7sO5J90cAM7Luv0A8nX7JtEXqw=yu1w@mail.gmail.com>
     [not found]             ` <CAFqZXNuv_mF8Yt50J3giZjxzZX_WwCOePZeEtmR-G_m+vRyPrQ@mail.gmail.com>
     [not found]               ` <20181203215639.GV2217@ZenIV.linux.org.uk>
     [not found]                 ` <CAFqZXNvSOzyhnTUxF2mP4BTFMVQ8H1uh-Yyn2fH2K=4yRDX-tQ@mail.gmail.com>
     [not found]                   ` <20181205161601.GW2217@ZenIV.linux.org.uk>
2018-12-05 21:58                     ` Casey Schaufler

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable: git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org linux-security-module@archiver.kernel.org
	public-inbox-index linux-security-module


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/ public-inbox