On Tue, 2020-03-03 at 17:58 -0500, Scott Mayhew wrote:
> When using NFSv4.2, the security label for the root inode should be
> set
> via a call to nfs_setsecurity() during the mount process, otherwise
> the
> inode will appear as unlabeled for up to acdirmin seconds. Currently
> the label for the root inode is allocated, retrieved, and freed
> entirely
> witin nfs4_proc_get_root().
>
> Add a field for the label to the nfs_fattr struct, and allocate &
> free
> the label in nfs_get_root(), where we also add a call to
> nfs_setsecurity(). Note that for the call to nfs_setsecurity() to
> succeed, it's necessary to also move the logic calling
> security_sb_{set,clone}_security() from nfs_get_tree_common() down
> into
> nfs_get_root()... otherwise the SBLABEL_MNT flag will not be set in
> the
> super_block's security flags and nfs_setsecurity() will silently
> fail.
I built and tested this patch on selinux-next (note that the NFS module
is a few patches behind).
The unlabeled problem is solved, however using:
mount -t nfs -o
vers=4.2,rootcontext=system_u:object_r:test_filesystem_file_t:s0
localhost:$TESTDIR /mnt/selinux-testsuite
I get the message:
mount.nfs: an incorrect mount option was specified
with a log entry:
SELinux: mount invalid. Same superblock, different security
settings for (dev 0:42, type nfs)
If I use "fscontext=" instead then works okay. Using no context option
also works. I guess the rootcontext= option should still work ???
>
> Reported-by: Richard Haines <richard_c_haines@xxxxxxxxxxxxxx>
> Signed-off-by: Scott Mayhew <smayhew@xxxxxxxxxx>
> ---
> fs/nfs/getroot.c | 39 +++++++++++++++++++++++++++++++++++----
> fs/nfs/nfs4proc.c | 12 +++---------
> fs/nfs/super.c | 25 -------------------------
> include/linux/nfs_xdr.h | 1 +
> 4 files changed, 39 insertions(+), 38 deletions(-)
>
> diff --git a/fs/nfs/getroot.c b/fs/nfs/getroot.c
> index b012c2668a1f..6d0628149390 100644
> --- a/fs/nfs/getroot.c
> +++ b/fs/nfs/getroot.c
> @@ -73,6 +73,7 @@ int nfs_get_root(struct super_block *s, struct
> fs_context *fc)
> struct inode *inode;
> char *name;
> int error = -ENOMEM;
> + unsigned long kflags = 0, kflags_out = 0;
>
> name = kstrdup(fc->source, GFP_KERNEL);
> if (!name)
> @@ -83,11 +84,14 @@ int nfs_get_root(struct super_block *s, struct
> fs_context *fc)
> if (fsinfo.fattr == NULL)
> goto out_name;
>
> + fsinfo.fattr->label = nfs4_label_alloc(server, GFP_KERNEL);
> + if (IS_ERR(fsinfo.fattr->label))
> + goto out_fattr;
> error = server->nfs_client->rpc_ops->getroot(server, ctx-
> >mntfh, &fsinfo);
> if (error < 0) {
> dprintk("nfs_get_root: getattr error = %d\n", -error);
> nfs_errorf(fc, "NFS: Couldn't getattr on root");
> - goto out_fattr;
> + goto out_label;
> }
>
> inode = nfs_fhget(s, ctx->mntfh, fsinfo.fattr, NULL);
> @@ -95,12 +99,12 @@ int nfs_get_root(struct super_block *s, struct
> fs_context *fc)
> dprintk("nfs_get_root: get root inode failed\n");
> error = PTR_ERR(inode);
> nfs_errorf(fc, "NFS: Couldn't get root inode");
> - goto out_fattr;
> + goto out_label;
> }
>
> error = nfs_superblock_set_dummy_root(s, inode);
> if (error != 0)
> - goto out_fattr;
> + goto out_label;
>
> /* root dentries normally start off anonymous and get spliced
> in later
> * if the dentry tree reaches them; however if the dentry
> already
> @@ -111,7 +115,7 @@ int nfs_get_root(struct super_block *s, struct
> fs_context *fc)
> dprintk("nfs_get_root: get root dentry failed\n");
> error = PTR_ERR(root);
> nfs_errorf(fc, "NFS: Couldn't get root dentry");
> - goto out_fattr;
> + goto out_label;
> }
>
> security_d_instantiate(root, inode);
> @@ -123,12 +127,39 @@ int nfs_get_root(struct super_block *s, struct
> fs_context *fc)
> }
> spin_unlock(&root->d_lock);
> fc->root = root;
> + if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> + kflags |= SECURITY_LSM_NATIVE_LABELS;
> + if (ctx->clone_data.sb) {
> + if (d_inode(fc->root)->i_fop != &nfs_dir_operations) {
> + error = -ESTALE;
> + goto error_splat_root;
> + }
> + /* clone any lsm security options from the parent to
> the new sb */
> + error = security_sb_clone_mnt_opts(ctx->clone_data.sb,
> s, kflags,
> + &kflags_out);
> + } else {
> + error = security_sb_set_mnt_opts(s, fc->security,
> + kflags,
> &kflags_out);
> + }
> + if (error)
> + goto error_splat_root;
> + if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> + !(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> + NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> +
> + nfs_setsecurity(inode, fsinfo.fattr, fsinfo.fattr->label);
> error = 0;
>
> +out_label:
> + nfs4_label_free(fsinfo.fattr->label);
> out_fattr:
> nfs_free_fattr(fsinfo.fattr);
> out_name:
> kfree(name);
> out:
> return error;
> +error_splat_root:
> + dput(fc->root);
> + fc->root = NULL;
> + goto out_label;
> }
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 69b7ab7a5815..cb34e840e4fb 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4002,7 +4002,7 @@ static int nfs4_proc_get_root(struct nfs_server
> *server, struct nfs_fh *mntfh,
> {
> int error;
> struct nfs_fattr *fattr = info->fattr;
> - struct nfs4_label *label = NULL;
> + struct nfs4_label *label = fattr->label;
>
> error = nfs4_server_capabilities(server, mntfh);
> if (error < 0) {
> @@ -4010,23 +4010,17 @@ static int nfs4_proc_get_root(struct
> nfs_server *server, struct nfs_fh *mntfh,
> return error;
> }
>
> - label = nfs4_label_alloc(server, GFP_KERNEL);
> - if (IS_ERR(label))
> - return PTR_ERR(label);
> -
> error = nfs4_proc_getattr(server, mntfh, fattr, label, NULL);
> if (error < 0) {
> dprintk("nfs4_get_root: getattr error = %d\n", -error);
> - goto err_free_label;
> + goto out;
> }
>
> if (fattr->valid & NFS_ATTR_FATTR_FSID &&
> !nfs_fsid_equal(&server->fsid, &fattr->fsid))
> memcpy(&server->fsid, &fattr->fsid, sizeof(server-
> >fsid));
>
> -err_free_label:
> - nfs4_label_free(label);
> -
> +out:
> return error;
> }
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index dada09b391c6..bb14bede6da5 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1179,7 +1179,6 @@ int nfs_get_tree_common(struct fs_context *fc)
> struct super_block *s;
> int (*compare_super)(struct super_block *, struct fs_context *)
> = nfs_compare_super;
> struct nfs_server *server = ctx->server;
> - unsigned long kflags = 0, kflags_out = 0;
> int error;
>
> ctx->server = NULL;
> @@ -1239,26 +1238,6 @@ int nfs_get_tree_common(struct fs_context *fc)
> goto error_splat_super;
> }
>
> - if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL)
> - kflags |= SECURITY_LSM_NATIVE_LABELS;
> - if (ctx->clone_data.sb) {
> - if (d_inode(fc->root)->i_fop != &nfs_dir_operations) {
> - error = -ESTALE;
> - goto error_splat_root;
> - }
> - /* clone any lsm security options from the parent to
> the new sb */
> - error = security_sb_clone_mnt_opts(ctx->clone_data.sb,
> s, kflags,
> - &kflags_out);
> - } else {
> - error = security_sb_set_mnt_opts(s, fc->security,
> - kflags,
> &kflags_out);
> - }
> - if (error)
> - goto error_splat_root;
> - if (NFS_SB(s)->caps & NFS_CAP_SECURITY_LABEL &&
> - !(kflags_out & SECURITY_LSM_NATIVE_LABELS))
> - NFS_SB(s)->caps &= ~NFS_CAP_SECURITY_LABEL;
> -
> s->s_flags |= SB_ACTIVE;
> error = 0;
>
> @@ -1268,10 +1247,6 @@ int nfs_get_tree_common(struct fs_context *fc)
> out_err_nosb:
> nfs_free_server(server);
> goto out;
> -
> -error_splat_root:
> - dput(fc->root);
> - fc->root = NULL;
> error_splat_super:
> deactivate_locked_super(s);
> goto out;
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 94c77ed55ce1..6838c149f335 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -75,6 +75,7 @@ struct nfs_fattr {
> struct nfs4_string *owner_name;
> struct nfs4_string *group_name;
> struct nfs4_threshold *mdsthreshold; /* pNFS threshold
> hints */
> + struct nfs4_label *label;
> };
>
> #define NFS_ATTR_FATTR_TYPE (1U << 0)
