[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freebsd-fs
Subject:    Re: UFS panic when attempting to mount wrong device
From:       Allan Jude <allanjude () freebsd ! org>
Date:       2018-02-19 18:28:41
Message-ID: a77e28c0-c603-eec0-bd3e-108582d17e89 () freebsd ! org
[Download RAW message or body]

[Attachment #2 (multipart/mixed)]


On 2018-02-19 05:57, Konstantin Belousov wrote:
> On Sun, Feb 18, 2018 at 08:14:48PM -0500, Allan Jude wrote:
>> I accidentally forgot to specify -t cd9660 when mounting a CD image, and
>> UFS panicked the machine:
>>
>> Unread portion of the kernel message buffer:
>> panic: vtopde on a uva/gpa 0x0
>> cpuid = 1
>> KDB: stack backtrace:
>> db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame
>> 0xfffffe0034409550
>> vpanic() at vpanic+0x18d/frame 0xfffffe00344095b0
>> vpanic() at vpanic/frame 0xfffffe0034409630
>> pmap_kextract() at pmap_kextract+0x121/frame 0xfffffe0034409660
>> free() at free+0x5e/frame 0xfffffe00344096a0
>> ffs_mount() at ffs_mount+0xe2f/frame 0xfffffe0034409840
>> vfs_donmount() at vfs_donmount+0xf56/frame 0xfffffe0034409a80
>> sys_nmount() at sys_nmount+0x72/frame 0xfffffe0034409ac0
>> amd64_syscall() at amd64_syscall+0x79b/frame 0xfffffe0034409bf0
>> fast_syscall_common() at fast_syscall_common+0x101/frame 0x7fffffffd990
>>
>>
>>
>> (kgdb) bt
>> #0  __curthread () at ./machine/pcpu.h:230
>> #1  doadump (textdump=1) at
>> /zroot/zfs_zstd/head/sys/kern/kern_shutdown.c:347
>> #2  0xffffffff80ac9242 in kern_reboot (howto=260) at
>> /zroot/zfs_zstd/head/sys/kern/kern_shutdown.c:416
>> #3  0xffffffff80ac980d in vpanic (fmt=<optimized out>,
>> ap=0xfffffe00344095f0) at /zroot/zfs_zstd/head/sys/kern/kern_shutdown.c:812
>> #4  0xffffffff80ac9620 in kassert_panic (fmt=0xffffffff81157632 "vtopde
>> on a uva/gpa 0x%0lx") at /zroot/zfs_zstd/head/sys/kern/kern_shutdown.c:698
>> #5  0xffffffff80f683a1 in vtopde (va=0) at
>> /zroot/zfs_zstd/head/sys/amd64/amd64/pmap.c:835
>> #6  pmap_kextract (va=0) at /zroot/zfs_zstd/head/sys/amd64/amd64/pmap.c:2237
>> #7  0xffffffff80aa3f2e in vtoslab (va=0) at
>> /zroot/zfs_zstd/head/sys/vm/uma_int.h:455
>> #8  free (addr=0x8, mtp=0xffffffff8189bb20 <M_UFSMNT>) at
>> /zroot/zfs_zstd/head/sys/kern/kern_malloc.c:701
>> #9  0xffffffff80dc278f in ffs_mountfs (devvp=<optimized out>,
>> mp=<optimized out>, td=<optimized out>)
>>     at /zroot/zfs_zstd/head/sys/ufs/ffs/ffs_vfsops.c:1047
>> #10 ffs_mount (mp=0xfffff80085dda000) at
>> /zroot/zfs_zstd/head/sys/ufs/ffs/ffs_vfsops.c:531
>> #11 0xffffffff80b8ebc6 in vfs_domount_first (td=<optimized out>,
>> fspath=0xfffff80003723800 "/mnt", vp=0xfffff80085baf938, vfsp=<optimized
>> out>,
>>     fsflags=<optimized out>, optlist=<optimized out>) at
>> /zroot/zfs_zstd/head/sys/kern/vfs_mount.c:827
>> #12 vfs_domount (td=<optimized out>, fstype=<optimized out>,
>> fspath=<optimized out>, fsflags=<optimized out>, optlist=<optimized out>)
>>     at /zroot/zfs_zstd/head/sys/kern/vfs_mount.c:1117
>> #13 vfs_donmount (td=0xfffff800139c6560, fsflags=<optimized out>,
>> fsoptions=0xfffff800054d6e00) at
>> /zroot/zfs_zstd/head/sys/kern/vfs_mount.c:684
>> #14 0xffffffff80b8dc42 in sys_nmount (td=0xfffff800139c6560,
>> uap=0xfffff800139c6918) at /zroot/zfs_zstd/head/sys/kern/vfs_mount.c:427
>> #15 0xffffffff80f7ed0b in syscallenter (td=0xfffff800139c6560) at
>> /zroot/zfs_zstd/head/sys/amd64/amd64/../../kern/subr_syscall.c:134
>> #16 amd64_syscall (td=0xfffff800139c6560, traced=0) at
>> /zroot/zfs_zstd/head/sys/amd64/amd64/trap.c:935
>> #17 0xffffffff80f5a66d in fast_syscall_common () at
>> /zroot/zfs_zstd/head/sys/amd64/amd64/exception.S:480
>> #18 0x0000000800c78000 in ?? ()
>>
>>
>> That that maybe a double free?
> More likely, a free of the uninitialized pointer.  Try this.
> 
> diff --git a/sys/ufs/ffs/ffs_subr.c b/sys/ufs/ffs/ffs_subr.c
> index 40db8bf01b1..4e167d98b65 100644
> --- a/sys/ufs/ffs/ffs_subr.c
> +++ b/sys/ufs/ffs/ffs_subr.c
> @@ -174,8 +174,12 @@ ffs_sbget(void *devfd, struct fs **fsp, off_t altsuperblock,
>  
>  	*fsp = NULL;
>  	if (altsuperblock != -1) {
> -		if ((ret = readsuper(devfd, fsp, altsuperblock, readfunc)) != 0)
> +		if ((ret = readsuper(devfd, fsp, altsuperblock, readfunc))
> +		    != 0) {
> +			if (*fsp != NULL)
> +				(*fsp)->fs_csp = NULL;
>  			return (ret);
> +		}
>  	} else {
>  		for (i = 0; sblock_try[i] != -1; i++) {
>  			if ((ret = readsuper(devfd, fsp, sblock_try[i],
> @@ -183,10 +187,15 @@ ffs_sbget(void *devfd, struct fs **fsp, off_t altsuperblock,
>  				break;
>  			if (ret == ENOENT)
>  				continue;
> +			if (*fsp != NULL)
> +				(*fsp)->fs_csp = NULL;
>  			return (ret);
>  		}
> -		if (sblock_try[i] == -1)
> +		if (sblock_try[i] == -1) {
> +			if (*fsp != NULL)
> +				(*fsp)->fs_csp = NULL;
>  			return (ENOENT);
> +		}
>  	}
>  	/*
>  	 * If not filling in summary information, NULL out fs_csp and return.
> 

This first patch solved the panic, but returns the wrong error message:

# mount /dev/cd0 /mnt
mount: /dev/cd0: No such file or directory

It does exist, and `mount -t cd9660 /dev/cd0 /mnt` works

I think we should return EINVAL in this case instead of ENOENT?


-- 
Allan Jude


["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]