Re: Unmounting with MNT_DOOMED flag can lead to a kernel panic

[Todd C. Miller]

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

[Alexander Hall]

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

[Alexander Bluhm]

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

[Todd C. Miller]

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

[Tim Newsham]

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