linux-kernel - Re: [PATCH v6] vfs, security: Fix automount superblock LSM init problem, preventing NFS sb sharing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 04 Aug 2023 08:58:37 -0400
From:   Jeff Layton <jlayton@...nel.org>
To:     Paul Moore <paul@...l-moore.com>,
        David Howells <dhowells@...hat.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        Trond Myklebust <trond.myklebust@...merspace.com>,
        Anna Schumaker <anna@...nel.org>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Stephen Smalley <stephen.smalley.work@...il.com>,
        Eric Paris <eparis@...isplace.org>,
        Casey Schaufler <casey@...aufler-ca.com>,
        David Howells <dhowells@...hat.com>,
        Scott Mayhew <smayhew@...hat.com>,
        Stephen Smalley <sds@...ho.nsa.gov>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-nfs@...r.kernel.org, linux-security-module@...r.kernel.org,
        selinux@...r.kernel.org
Subject: Re: [PATCH v6] vfs, security: Fix automount superblock LSM init
 problem, preventing NFS sb sharing

On Thu, 2023-08-03 at 22:48 -0400, Paul Moore wrote:
> On Thu, Aug 3, 2023 at 12:27 PM Jeff Layton <jlayton@...nel.org> wrote:
> > On Wed, 2023-08-02 at 22:46 -0400, Paul Moore wrote:
> > > On Wed, Aug 2, 2023 at 3:34 PM Jeff Layton <jlayton@...nel.org> wrote:
> > > > On Wed, 2023-08-02 at 14:16 -0400, Paul Moore wrote:
> > > > > On Aug  2, 2023 Jeff Layton <jlayton@...nel.org> wrote:
> 
> ...
> 
> > > My only concern now is the fs_context::lsm_set flag.
> > 
> > Yeah, that bit is ugly. David studied this problem a lot more than I
> > have, but basically, we only want to set the context info once, and
> > we're not always going to have a nice string to parse to set up the
> > options. This obviously works, but I'm fine with a more elegant method
> > if you can spot one.
> 
> Like I said before, sometimes making a LSM hook conditional on some
> flag is the only practical solution, but I always worry that there is
> a chance that a future patch might end up toggling that flag by
> accident and we lose an important call into the LSM.  Even if all we
> end up doing is moving the flag down into the LSMs I would be happier;
> there is still a risk, but at least if something breaks it is our (the
> LSM folks) own damn fault ;)
> 
> > > You didn't mention exactly why the security_sb_set_mnt_opts() was
> > > failing, and requires the fs_context::lsm_set check, but my guess is
> > > that something is tripping over the fact that the superblock is
> > > already properly setup.  I'm working under the assumption that this
> > > problem - attempting to reconfigure a properly configured superblock -
> > > should only be happening in the submount/non-NULL-reference case.  If
> > > it is happening elsewhere I think I'm going to need some help
> > > understanding that ...
> > 
> > Correct. When you pass in the mount options, fc->security seems to be
> > properly set. NFS mounting is complex though, so the final superblock
> > you care about may end up being a descendant of the one that was
> > originally configured.
> 
> Ooof, okay, there goes that idea.
> 
> At this point I guess it comes back to that question of why is calling
> into security_sb_set_mnt_opts() a second (or third, etc.) time failing
> for you?  Is there some conflict with the superblock
> config/labeling/etc.?  Is there a permissions problem?  Better
> understanding why that is failing might help us come up with a better
> solution.
> 

I removed the lsm_set parameter from this patch, and my testcase still
works fine. I can post a v7 if we want to go forward with that. I'm
guessing the complaint he saw was the "out_double_mount" pr_warn.

It looks like as long as the context options match, there shouldn't be
an issue, and I don't see how you'd get mismatched ones if you're
inheriting them.

David, do you remember what prompted you to add the lsm_set parameter?
-- 
Jeff Layton <jlayton@...nel.org>