Re: Unmounting with MNT_DOOMED flag can lead to a kernel panic
On Wed, 13 Jul 2016 03:26:34 +0200, Alexander Hall wrote: > Then wouldn't EINVAL be a reasonable response? Am I missing something? We typically ignore flags that don't make sense. For example, chmod(2) doesn't return an error if you pass in a mode with the directory bit set, it just masks it out. Since unmount(2) uses the same flags as mount(2) it seems reasonable to just ignore the ones that don't make sense. - todd
Re: Unmounting with MNT_DOOMED flag can lead to a kernel panic
On July 12, 2016 8:55:50 PM GMT+02:00, "Todd C. Miller"wrote: >On Tue, 12 Jul 2016 07:22:57 -1000, Tim Newsham wrote: > >> Here's another root-only (unless kern.usermount is set) panic issue. >We >> exercise it through tmpfs but it might be more general than that. >We're >> not sure what the proper remediation should be here. > >The only valid flag for umount(2) is MNT_FORCE. Then wouldn't EINVAL be a reasonable response? Am I missing something? /Alexander > > - todd > >Index: vfs_syscalls.c >=== >RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v >retrieving revision 1.261 >diff -u -p -u -r1.261 vfs_syscalls.c >--- vfs_syscalls.c 6 Jul 2016 19:26:35 - 1.261 >+++ vfs_syscalls.c 12 Jul 2016 18:55:09 - >@@ -412,7 +412,7 @@ sys_unmount(struct proc *p, void *v, reg > if (vfs_busy(mp, VB_WRITE|VB_WAIT)) > return (EBUSY); > >- return (dounmount(mp, SCARG(uap, flags), p, vp)); >+ return (dounmount(mp, SCARG(uap, flags) & MNT_FORCE, p, vp)); > } > > /*
Re: Unmounting with MNT_DOOMED flag can lead to a kernel panic
On Tue, Jul 12, 2016 at 12:55:50PM -0600, Todd C. Miller wrote: > On Tue, 12 Jul 2016 07:22:57 -1000, Tim Newsham wrote: > > > Here's another root-only (unless kern.usermount is set) panic issue. We > > exercise it through tmpfs but it might be more general than that. We're > > not sure what the proper remediation should be here. > > The only valid flag for umount(2) is MNT_FORCE. OK bluhm@ > > - todd > > Index: vfs_syscalls.c > === > RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v > retrieving revision 1.261 > diff -u -p -u -r1.261 vfs_syscalls.c > --- vfs_syscalls.c6 Jul 2016 19:26:35 - 1.261 > +++ vfs_syscalls.c12 Jul 2016 18:55:09 - > @@ -412,7 +412,7 @@ sys_unmount(struct proc *p, void *v, reg > if (vfs_busy(mp, VB_WRITE|VB_WAIT)) > return (EBUSY); > > - return (dounmount(mp, SCARG(uap, flags), p, vp)); > + return (dounmount(mp, SCARG(uap, flags) & MNT_FORCE, p, vp)); > } > > /*
Re: Unmounting with MNT_DOOMED flag can lead to a kernel panic
On Tue, 12 Jul 2016 07:22:57 -1000, Tim Newsham wrote: > Here's another root-only (unless kern.usermount is set) panic issue. We > exercise it through tmpfs but it might be more general than that. We're > not sure what the proper remediation should be here. The only valid flag for umount(2) is MNT_FORCE. - todd Index: vfs_syscalls.c === RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v retrieving revision 1.261 diff -u -p -u -r1.261 vfs_syscalls.c --- vfs_syscalls.c 6 Jul 2016 19:26:35 - 1.261 +++ vfs_syscalls.c 12 Jul 2016 18:55:09 - @@ -412,7 +412,7 @@ sys_unmount(struct proc *p, void *v, reg if (vfs_busy(mp, VB_WRITE|VB_WAIT)) return (EBUSY); - return (dounmount(mp, SCARG(uap, flags), p, vp)); + return (dounmount(mp, SCARG(uap, flags) & MNT_FORCE, p, vp)); } /*
Unmounting with MNT_DOOMED flag can lead to a kernel panic
Here's another root-only (unless kern.usermount is set) panic issue. We exercise it through tmpfs but it might be more general than that. We're not sure what the proper remediation should be here. /* * unmount_panic.c *Demonstrate a panic through the unmount system call. * * gcc -g unmount_panic.c -o unmount_panic */ #ifdef BUG_WRITEUP //--- Unmounting with MNT_DOOMED flag can lead to a kernel panic Impact: Root users or users on systems with kern.usermount set to true can trigger a kernel panic when unmounting a filesystem. Description: When the unmount system call is called with the MNT_DOOMED flag set, it does not sync vnodes. This can lead to a condition where there is still a vnode on the mnt_vnodelist, which triggers a panic in dounmount(). if (!LIST_EMPTY(>mnt_vnodelist)) panic("unmount: dangling vnode"); This conditoin can only be triggered by users who are allowed to unmount a filesystem. Normally this is the root user, but if the kern.usernmount sysctl variable has been set to true, any user could trigger this panic. Reproduction: Run the attached unmount_panic.c program. It will mount a new tmpfs on /mnt, open a file on it, and then unmount /mnt with the MNT_DOOMED flag. This will lead to a panic of "unmount: dangling vnode". NCC Group was able to reproduce this issue on OpenBSD 5.9 release running amd64. Recommendation: TBD Reported: 2016-07-12 Fixed: notyet #endif // BUG_WRITEUP --- #include #include #include #include #include void xperror(int cond, char *msg) { if(cond) { perror(msg); exit(1); } } int main(int argc, char **argv) { struct tmpfs_args args = { TMPFS_ARGS_VERSION, 0, 0, 0, 0, 0 }; int x, fd; x = mount("tmpfs", "/mnt", 0, ); xperror(x == -1, "mount"); fd = open("/mnt/somefile", O_RDWR | O_CREAT, 0666); xperror(fd == -1, "/mnt/somefile"); x = unmount("/mnt", MNT_DOOMED); xperror(fd == -1, "unmount"); printf("no crash!\n"); return 0; } -- Tim Newsham Distinguished Security Engineer, Security Consulting NCC Group Tim.Newsham@nccgroup.trust | PGP: B415 550D BEE9 07DB B4C9 F96C 8EFE CB2F 402D 3DF0