Re: [PATCH 2/3] xfs: sanitize remount options

[Brian Foster <bfoster@redhat.com>]

On Wed, Feb 10, 2016 at 05:40:05PM -0600, Eric Sandeen wrote:
> Perform basic sanitization of remount options by
> passing the option string and a dummy mount structure
> through xfs_parseargs and returning the result.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 934233a..d1cd4fa 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1168,6 +1168,27 @@ xfs_quiesce_attr(
>  }
>  
>  STATIC int
> +xfs_test_remount_options(
> +	struct super_block	*sb,
> +	struct xfs_mount	*mp,
> +	char			*options)
> +{
> +	int			error = 0;
> +	struct xfs_mount	*tmp;
> +
> +	tmp = kmem_zalloc(sizeof(*tmp), KM_MAYFAIL);
> +	if (!tmp)
> +		return -ENOMEM;
> +
> +	tmp->m_super = sb;
> +	error = xfs_parseargs(tmp, options);
> +	xfs_free_fsname(tmp);
> +	kfree(tmp);
> +
> +	return error;
> +}

This seems fine:

Reviewed-by: Brian Foster <bfoster@redhat.com>

... though I get a little bit nervous about xfs_parseargs() changing
some other data structure down the road. I wonder if we should constify
the sb pointer in xfs_parseargs() with a quick comment as to why..?

Brian

> +
> +STATIC int
>  xfs_fs_remount(
>  	struct super_block	*sb,
>  	int			*flags,
> @@ -1179,6 +1200,11 @@ xfs_fs_remount(
>  	char			*p;
>  	int			error;
>  
> +	/* First, check for complete junk; i.e. invalid options */
> +	error = xfs_test_remount_options(sb, mp, options);
> +	if (error)
> +		return error;
> +
>  	sync_filesystem(sb);
>  	while ((p = strsep(&options, ",")) != NULL) {
>  		int token;
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs