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

[Jeff Layton <jlayton@kernel.org>]

On Thu, 2023-08-03 at 22:48 -0400, Paul Moore wrote:
> On Thu, Aug 3, 2023 at 12:27 PM Jeff Layton <jlayton@kernel.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@kernel.org> wrote:
> > > > On Wed, 2023-08-02 at 14:16 -0400, Paul Moore wrote:
> > > > > On Aug  2, 2023 Jeff Layton <jlayton@kernel.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@kernel.org>