Re: Intentionally corrupted vfat fs causing BUG

[Richard Weinberger <richard@nod.at>]

Am 13.10.2014 um 09:57 schrieb OGAWA Hirofumi:
> Richard Weinberger <richard.weinberger@gmail.com> writes:
> 
>>>
>>> We would need the way how make corrupted image like testimg.vfat.24.min,
>>> to find the cause of this problem. Base image for reproducing this bug,
>>> and way to do are very helpful.
>>
>> You misunderstood Sami's issue. He corrupted the vfat fs intentionally
>> to find issues
>> in the vfat driver.
>> And as he reports he found an nasty issue.
>> Any user can trigger a BUG_ON() using a crafted vfat image.
>> Please note, if you mount exactly the same image using msdos fs the issue
>> does not occur.
> 
> Ah. 
> 
> BTW, msdos doesn't allow ".*" as filename, so not trigger this. But root
> cause of this is same as double linked dir, "." should not
> matter. I.e. this issue would be able to reproduce on all FSes if made
> corrupted image intentionally.
> 
> If we want to fix intentional corruption like this seriously, I guess we
> would need something like online-fsck to detect like double link of
> dir. If we want to avoid only Oops, it might be enough to remove
> BUG_ON().
> 
> I'm still not sure whether this is right direction or not though,
> because mount operation is root only and untrusted image should run fsck
> before. But, also, Oops is clearly unexpected. Hmmm...

This limitation is not true anymore. Plug in a USB stick into a recent Linux desktop,
it will automatically mount it...
Also think of user namespaces and FUSE.

Thanks,
//richard

> Al?
> 
> 
> [PATCH] Avoid Oops on corrupted dir in may_delete()
> 
> Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> ---
> 
>  fs/namei.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff -puN fs/namei.c~fix-oops-on-corrupted-fs fs/namei.c
> --- linux-3.17/fs/namei.c~fix-oops-on-corrupted-fs	2014-10-13 16:34:28.352999516 +0900
> +++ linux-3.17-hirofumi/fs/namei.c	2014-10-13 16:35:19.196803169 +0900
> @@ -2427,7 +2427,10 @@ static int may_delete(struct inode *dir,
>  		return -ENOENT;
>  	BUG_ON(!inode);
>  
> -	BUG_ON(victim->d_parent->d_inode != dir);
> +	/* Easy check of corrupted dir. */
> +	if (victim->d_parent->d_inode != dir)
> +		return -EBUSY;
> +
>  	audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
>  
>  	error = inode_permission(dir, MAY_WRITE | MAY_EXEC);
> _
>