On Fri, Oct 11, 2013 at 02:11:18PM -0500, Eric Sandeen wrote:
> There's a long comment about handling non-remountable
> options in xfs_fs_remount, but nothing addresses the case
> of completely bogus mount options at remount time, which
> can lead to some severe strangeness:
>
> # for I in `seq 1 10`; do mount -o remount,noacl /mnt/test2; done
> # for I in `seq 1 10`; do mount -o remount,badoption /mnt/test2; done
> # grep sdb4 /etc/mtab
> /dev/sdb4 /mnt/test2 xfs rw,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,noacl,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption,badoption 0 0
>
> This is a bit of a hack, but we can re-use xfs_parseargs()
> with a dummy mount struct to just vet all of the remount
> options which were passed in. With this, we get a saner
> result:
>
> [44898.102990] EXT4-fs (sdb4): Unrecognized mount option "badoption" or missing value
ext4? Really? :)
> +++ b/fs/xfs/xfs_super.c
> @@ -1202,11 +1202,25 @@ xfs_fs_remount(
> int *flags,
> char *options)
> {
> - struct xfs_mount *mp = XFS_M(sb);
> + struct xfs_mount *mp = XFS_M(sb), *dummy_mp;
> substring_t args[MAX_OPT_ARGS];
> char *p;
> int error;
>
> + /*
> + * Check all the mount options presented to be sure
> + * there's nothing too crazy in there. Non-remountable
> + * but valid options are a different issue.
> + */
> + dummy_mp = kmem_zalloc(sizeof(*dummy_mp), KM_MAYFAIL);
> + if (dummy_mp) {
> + dummy_mp->m_super = sb;
> + error = xfs_parseargs(dummy_mp, options);
> + kfree(dummy_mp);
> + if (error)
> + return -error;
This, at minimum, leaks dummy_mp->m_fsname, and it will leak other
strings that are also kstrdup()d by xfs_parseargs().
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
