On 10/13/13 4:52 PM, Dave Chinner wrote:
> 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? :)
uhhh ;)
>> +++ 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().
nnngh. Forgot about that side effect, sorry. Dammit.
Think it's still worth doing this if I handle freeing them all up?
-Eric
> Cheers,
>
> Dave.
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
