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 <eparis@xxxxxxxxxx>
>
> ---
>
> 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:
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
