On Thu, Feb 18, 2021 at 5:07 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote: > > On 2/18/2021 11:50 AM, Olga Kornievskaia wrote: > > From: Olga Kornievskaia <kolga@xxxxxxxxxx> > > > > Keep track of whether or not there was an selinux context mount > > options during the mount. > > This may be the intention, but it's not what the change you're > introducing here does. Can you explain why, as I must be doing something wrong? I'm familiar with NFS but not with Selinux and I thought I was doing the correct changes to the Selinux. If that's not the case would you share what has been done incorrectly. > > While deciding if the superblock can be > > shared for the new mount, check for if we had selinux context on > > the existing mount and call into selinux to tell if new passed > > in selinux context is compatible with the existing mount's options. > > You're describing how you expect the change to be used, not > what it does. If I am the author of a security module other > than SELinux (which, it turns out, I am) what would I use this > for? How might this change interact with my security module? > Is this something I might exploit? If I am the author of a > filesystem other than NFS (which I am not) should I be doing > the same thing? I'm not sure I'm understanding your questions. I'm solving a bug that exists when NFS interacts with selinux. This is really not a feature that I'm introducing. I thought it was somewhat descriptive with an example that if you want to mount with different security contexts but whatever you are mounting has a possibility of sharing the same superblock, we need to be careful and take security contexts that are being used by those mount into account to decide whether or not to share the superblock. Again I thought that's exactly what the commit states. A different security module, if it acts on security context, would have to have the same ability to compare if one context options are compatible with anothers. Another filesystem can decide if it wants to share superblocks based on compatibility of existing security context and new one. Is that what you are asking? > > > > > Previously, NFS wasn't able to do the following 2mounts: > > mount -o vers=4.2,sec=sys,context=system_u:object_r:root_t:s0 > > <serverip>:/ /mnt > > mount -o vers=4.2,sec=sys,context=system_u:object_r:swapfile_t:s0 > > <serverip>:/scratch /scratch > > > > 2nd mount would fail with "mount.nfs: an incorrect mount option was > > specified" and var log messages would have: > > "SElinux: mount invalid. Same superblock, different security > > settings for.." > > > > Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx> > > --- > > fs/nfs/fs_context.c | 3 +++ > > fs/nfs/internal.h | 1 + > > fs/nfs/super.c | 4 ++++ > > include/linux/nfs_fs_sb.h | 1 + > > 4 files changed, 9 insertions(+) > > > > diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c > > index 06894bcdea2d..8067f055d842 100644 > > --- a/fs/nfs/fs_context.c > > +++ b/fs/nfs/fs_context.c > > @@ -448,6 +448,9 @@ static int nfs_fs_context_parse_param(struct fs_context *fc, > > if (opt < 0) > > return ctx->sloppy ? 1 : opt; > > > > + if (fc->security) > > + ctx->has_sec_mnt_opts = 1; > > + > > switch (opt) { > > case Opt_source: > > if (fc->source) > > diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h > > index 62d3189745cd..08f4f34e8cf5 100644 > > --- a/fs/nfs/internal.h > > +++ b/fs/nfs/internal.h > > @@ -96,6 +96,7 @@ struct nfs_fs_context { > > char *fscache_uniq; > > unsigned short protofamily; > > unsigned short mountfamily; > > + bool has_sec_mnt_opts; > > > > struct { > > union { > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c > > index 4034102010f0..0a2d252cf90f 100644 > > --- a/fs/nfs/super.c > > +++ b/fs/nfs/super.c > > @@ -1058,6 +1058,7 @@ static void nfs_fill_super(struct super_block *sb, struct nfs_fs_context *ctx) > > &sb->s_blocksize_bits); > > > > nfs_super_set_maxbytes(sb, server->maxfilesize); > > + server->has_sec_mnt_opts = ctx->has_sec_mnt_opts; > > } > > > > static int nfs_compare_mount_options(const struct super_block *s, const struct nfs_server *b, > > @@ -1174,6 +1175,9 @@ static int nfs_compare_super(struct super_block *sb, struct fs_context *fc) > > return 0; > > if (!nfs_compare_userns(old, server)) > > return 0; > > + if ((old->has_sec_mnt_opts || fc->security) && > > + security_sb_mnt_opts_compat(sb, fc->security)) > > + return 0; > > return nfs_compare_mount_options(sb, server, fc); > > } > > > > diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h > > index 38e60ec742df..3f0acada5794 100644 > > --- a/include/linux/nfs_fs_sb.h > > +++ b/include/linux/nfs_fs_sb.h > > @@ -254,6 +254,7 @@ struct nfs_server { > > > > /* User namespace info */ > > const struct cred *cred; > > + bool has_sec_mnt_opts; > > }; > > > > /* Server capabilities */ >