NFS and SELinux worked together previously because SELinux had NFS
specific knowledge built in. This design was approved by both groups
back in 2004 but the recent NFS changes to use nfs_parsed_mount_data and
the usage of nfs_clone_mount_data showed this to be a poor fragile
solution. This patch fixes the NFS functionality regression by making
use of the new LSM interfaces to allow an FS to explicitly set its own
mount options.
The explicit setting of mount options is done in the nfs get_sb
functions which are called before the generic vfs hooks try to set mount
options for filesystems which use text mount data.
This does not currently support NFSv4 as that functionality did not
exist in previous kernels and thus there is no regression. I will be
adding the needed code, which I believe to be the exact same as the v3
code, in nfs4_get_sb for 2.6.26.
Signed-off-by: Eric Paris <[email protected]>
---
fs/nfs/internal.h | 3 ++
fs/nfs/super.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 0f56196..9319927 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -3,6 +3,7 @@
*/
#include <linux/mount.h>
+#include <linux/security.h>
struct nfs_string;
@@ -57,6 +58,8 @@ struct nfs_parsed_mount_data {
char *export_path;
int protocol;
} nfs_server;
+
+ struct security_mnt_opts lsm_opts;
};
/* client.c */
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 1fb3818..a097adf 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -684,8 +684,9 @@ static void nfs_parse_server_address(char *value,
static int nfs_parse_mount_options(char *raw,
struct nfs_parsed_mount_data *mnt)
{
- char *p, *string;
+ char *p, *string, *secdata;
unsigned short port = 0;
+ int rc;
if (!raw) {
dfprintk(MOUNT, "NFS: mount options string was NULL.\n");
@@ -693,6 +694,20 @@ static int nfs_parse_mount_options(char *raw,
}
dfprintk(MOUNT, "NFS: nfs mount opts='%s'\n", raw);
+ secdata = alloc_secdata();
+ if (!secdata)
+ goto out_nomem;
+
+ rc = security_sb_copy_data(raw, secdata);
+ if (rc)
+ goto out_security_failure;
+
+ rc = security_sb_parse_opts_str(secdata, &mnt->lsm_opts);
+ if (rc)
+ goto out_security_failure;
+
+ free_secdata(secdata);
+
while ((p = strsep(&raw, ",")) != NULL) {
substring_t args[MAX_OPT_ARGS];
int option, token;
@@ -1042,7 +1057,10 @@ static int nfs_parse_mount_options(char *raw,
out_nomem:
printk(KERN_INFO "NFS: not enough memory to parse option\n");
return 0;
-
+out_security_failure:
+ free_secdata(secdata);
+ printk(KERN_INFO "NFS: security options invalid: %d\n", rc);
+ return 0;
out_unrec_vers:
printk(KERN_INFO "NFS: unrecognized NFS version number\n");
return 0;
@@ -1214,6 +1232,32 @@ static int nfs_validate_mount_data(void *options,
args->namlen = data->namlen;
args->bsize = data->bsize;
args->auth_flavors[0] = data->pseudoflavor;
+
+ /*
+ * The legacy version 6 binary mount data from userspace has a
+ * field used only to transport selinux information into the
+ * the kernel. To continue to support that functionality we
+ * have a touch of selinux knowledge here in the NFS code. The
+ * userspace code converted context=blah to just blah so we are
+ * converting back to the full string selinux understands.
+ */
+ if (data->context[0]){
+#ifdef CONFIG_SECURITY_SELINUX
+ int rc;
+ char *opts_str = kmalloc(sizeof(data->context) + 8, GFP_KERNEL);
+ if (!opts_str)
+ return -ENOMEM;
+ strcpy(opts_str, "context=");
+ strcat(opts_str, &data->context[0]);
+ rc = security_sb_parse_opts_str(opts_str, &args->lsm_opts);
+ kfree(opts_str);
+ if (rc)
+ return rc;
+#else
+ return -EINVAL;
+#endif
+ }
+
break;
default: {
unsigned int len;
@@ -1476,6 +1520,8 @@ static int nfs_get_sb(struct file_system_type *fs_type,
};
int error;
+ security_init_mnt_opts(&data.lsm_opts);
+
/* Validate the mount data */
error = nfs_validate_mount_data(raw_data, &data, &mntfh, dev_name);
if (error < 0)
@@ -1515,6 +1561,10 @@ static int nfs_get_sb(struct file_system_type *fs_type,
goto error_splat_super;
}
+ error = security_sb_set_mnt_opts(s, &data.lsm_opts);
+ if (error)
+ goto error_splat_root;
+
s->s_flags |= MS_ACTIVE;
mnt->mnt_sb = s;
mnt->mnt_root = mntroot;
@@ -1523,12 +1573,15 @@ static int nfs_get_sb(struct file_system_type *fs_type,
out:
kfree(data.nfs_server.hostname);
kfree(data.mount_server.hostname);
+ security_free_mnt_opts(&data.lsm_opts);
return error;
out_err_nosb:
nfs_free_server(server);
goto out;
+error_splat_root:
+ dput(mntroot);
error_splat_super:
up_write(&s->s_umount);
deactivate_super(s);
@@ -1608,6 +1661,9 @@ static int nfs_xdev_get_sb(struct file_system_type *fs_type, int flags,
mnt->mnt_sb = s;
mnt->mnt_root = mntroot;
+ /* clone any lsm security options from the parent to the new sb */
+ security_sb_clone_mnt_opts(data->sb, s);
+
dprintk("<-- nfs_xdev_get_sb() = 0\n");
return 0;
@@ -1850,6 +1906,8 @@ static int nfs4_get_sb(struct file_system_type *fs_type,
};
int error;
+ security_init_mnt_opts(&data.lsm_opts);
+
/* Validate the mount data */
error = nfs4_validate_mount_data(raw_data, &data, dev_name);
if (error < 0)
@@ -1898,6 +1956,7 @@ out:
kfree(data.client_address);
kfree(data.nfs_server.export_path);
kfree(data.nfs_server.hostname);
+ security_free_mnt_opts(&data.lsm_opts);
return error;
out_free:
On Wed, 2008-03-05 at 10:31 -0500, Eric Paris wrote:
> NFS and SELinux worked together previously because SELinux had NFS
> specific knowledge built in. This design was approved by both groups
> back in 2004 but the recent NFS changes to use nfs_parsed_mount_data and
> the usage of nfs_clone_mount_data showed this to be a poor fragile
> solution. This patch fixes the NFS functionality regression by making
> use of the new LSM interfaces to allow an FS to explicitly set its own
> mount options.
>
> The explicit setting of mount options is done in the nfs get_sb
> functions which are called before the generic vfs hooks try to set mount
> options for filesystems which use text mount data.
>
> This does not currently support NFSv4 as that functionality did not
> exist in previous kernels and thus there is no regression. I will be
> adding the needed code, which I believe to be the exact same as the v3
> code, in nfs4_get_sb for 2.6.26.
>
> Signed-off-by: Eric Paris <[email protected]>
>
> ---
>
> fs/nfs/internal.h | 3 ++
> fs/nfs/super.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 0f56196..9319927 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -3,6 +3,7 @@
> */
>
> #include <linux/mount.h>
> +#include <linux/security.h>
>
> struct nfs_string;
>
> @@ -57,6 +58,8 @@ struct nfs_parsed_mount_data {
> char *export_path;
> int protocol;
> } nfs_server;
> +
> + struct security_mnt_opts lsm_opts;
> };
>
> /* client.c */
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 1fb3818..a097adf 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -684,8 +684,9 @@ static void nfs_parse_server_address(char *value,
> static int nfs_parse_mount_options(char *raw,
> struct nfs_parsed_mount_data *mnt)
> {
> - char *p, *string;
> + char *p, *string, *secdata;
> unsigned short port = 0;
> + int rc;
>
> if (!raw) {
> dfprintk(MOUNT, "NFS: mount options string was NULL.\n");
> @@ -693,6 +694,20 @@ static int nfs_parse_mount_options(char *raw,
> }
> dfprintk(MOUNT, "NFS: nfs mount opts='%s'\n", raw);
>
> + secdata = alloc_secdata();
> + if (!secdata)
> + goto out_nomem;
> +
> + rc = security_sb_copy_data(raw, secdata);
> + if (rc)
> + goto out_security_failure;
> +
> + rc = security_sb_parse_opts_str(secdata, &mnt->lsm_opts);
> + if (rc)
> + goto out_security_failure;
> +
> + free_secdata(secdata);
> +
> while ((p = strsep(&raw, ",")) != NULL) {
> substring_t args[MAX_OPT_ARGS];
> int option, token;
> @@ -1042,7 +1057,10 @@ static int nfs_parse_mount_options(char *raw,
> out_nomem:
> printk(KERN_INFO "NFS: not enough memory to parse option\n");
> return 0;
> -
> +out_security_failure:
> + free_secdata(secdata);
> + printk(KERN_INFO "NFS: security options invalid: %d\n", rc);
> + return 0;
> out_unrec_vers:
> printk(KERN_INFO "NFS: unrecognized NFS version number\n");
> return 0;
> @@ -1214,6 +1232,32 @@ static int nfs_validate_mount_data(void *options,
> args->namlen = data->namlen;
> args->bsize = data->bsize;
> args->auth_flavors[0] = data->pseudoflavor;
> +
> + /*
> + * The legacy version 6 binary mount data from userspace has a
> + * field used only to transport selinux information into the
> + * the kernel. To continue to support that functionality we
> + * have a touch of selinux knowledge here in the NFS code. The
> + * userspace code converted context=blah to just blah so we are
> + * converting back to the full string selinux understands.
> + */
> + if (data->context[0]){
> +#ifdef CONFIG_SECURITY_SELINUX
> + int rc;
> + char *opts_str = kmalloc(sizeof(data->context) + 8, GFP_KERNEL);
> + if (!opts_str)
> + return -ENOMEM;
> + strcpy(opts_str, "context=");
> + strcat(opts_str, &data->context[0]);
Shouldn't this be a strncat? I can't see that we're sanity checking the
data->context string for \NUL termination anywhere.
> + rc = security_sb_parse_opts_str(opts_str, &args->lsm_opts);
> + kfree(opts_str);
> + if (rc)
> + return rc;
> +#else
> + return -EINVAL;
Is this really necessary?
> +#endif
> + }
> +
> break;
> default: {
> unsigned int len;
> @@ -1476,6 +1520,8 @@ static int nfs_get_sb(struct file_system_type *fs_type,
> };
> int error;
>
> + security_init_mnt_opts(&data.lsm_opts);
> +
> /* Validate the mount data */
> error = nfs_validate_mount_data(raw_data, &data, &mntfh, dev_name);
> if (error < 0)
> @@ -1515,6 +1561,10 @@ static int nfs_get_sb(struct file_system_type *fs_type,
> goto error_splat_super;
> }
>
> + error = security_sb_set_mnt_opts(s, &data.lsm_opts);
> + if (error)
> + goto error_splat_root;
> +
> s->s_flags |= MS_ACTIVE;
> mnt->mnt_sb = s;
> mnt->mnt_root = mntroot;
> @@ -1523,12 +1573,15 @@ static int nfs_get_sb(struct file_system_type *fs_type,
> out:
> kfree(data.nfs_server.hostname);
> kfree(data.mount_server.hostname);
> + security_free_mnt_opts(&data.lsm_opts);
> return error;
>
> out_err_nosb:
> nfs_free_server(server);
> goto out;
>
> +error_splat_root:
> + dput(mntroot);
> error_splat_super:
> up_write(&s->s_umount);
> deactivate_super(s);
> @@ -1608,6 +1661,9 @@ static int nfs_xdev_get_sb(struct file_system_type *fs_type, int flags,
> mnt->mnt_sb = s;
> mnt->mnt_root = mntroot;
>
> + /* clone any lsm security options from the parent to the new sb */
> + security_sb_clone_mnt_opts(data->sb, s);
> +
> dprintk("<-- nfs_xdev_get_sb() = 0\n");
> return 0;
>
> @@ -1850,6 +1906,8 @@ static int nfs4_get_sb(struct file_system_type *fs_type,
> };
> int error;
>
> + security_init_mnt_opts(&data.lsm_opts);
> +
> /* Validate the mount data */
> error = nfs4_validate_mount_data(raw_data, &data, dev_name);
> if (error < 0)
> @@ -1898,6 +1956,7 @@ out:
> kfree(data.client_address);
> kfree(data.nfs_server.export_path);
> kfree(data.nfs_server.hostname);
> + security_free_mnt_opts(&data.lsm_opts);
> return error;
>
> out_free:
>
>
>
On Wed, 2008-03-05 at 13:25 -0500, Trond Myklebust wrote:
> On Wed, 2008-03-05 at 10:31 -0500, Eric Paris wrote:
> > @@ -1214,6 +1232,32 @@ static int nfs_validate_mount_data(void *options,
> > args->namlen = data->namlen;
> > args->bsize = data->bsize;
> > args->auth_flavors[0] = data->pseudoflavor;
> > +
> > + /*
> > + * The legacy version 6 binary mount data from userspace has a
> > + * field used only to transport selinux information into the
> > + * the kernel. To continue to support that functionality we
> > + * have a touch of selinux knowledge here in the NFS code. The
> > + * userspace code converted context=blah to just blah so we are
> > + * converting back to the full string selinux understands.
> > + */
> > + if (data->context[0]){
> > +#ifdef CONFIG_SECURITY_SELINUX
> > + int rc;
> > + char *opts_str = kmalloc(sizeof(data->context) + 8, GFP_KERNEL);
> > + if (!opts_str)
> > + return -ENOMEM;
> > + strcpy(opts_str, "context=");
> > + strcat(opts_str, &data->context[0]);
>
> Shouldn't this be a strncat? I can't see that we're sanity checking the
> data->context string for \NUL termination anywhere.
No good reason not to make sure since we know the size of the static
context array. I just assumed NFS was doing checks on the hostname and
the context array the same way to make sure they were both valid. I'll
send a new patch which uses strncat.
> > + rc = security_sb_parse_opts_str(opts_str, &args->lsm_opts);
> > + kfree(opts_str);
> > + if (rc)
> > + return rc;
> > +#else
> > + return -EINVAL;
>
> Is this really necessary?
If someone used context= with the binary data interface the kernel needs
to tell them that it didn't understand. It's really no different than
nfs_parse_mount_options() having the out_unknown error case instead of
just ignoring unknown. If they used the text interface they would fail
on the out_unknown case if !CONFIG_SELINUX.
> > +#endif
> > + }
> > +
> > break;
> > default: {
> > unsigned int len;