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