From 814a8da0ed70187bf06618ee3a545ca3361b5933 Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Wed, 27 May 2020 17:18:07 +0900 Subject: devices: fs: Use 2 stage create and mkdir When creating a file or directory the virtio-fs server changes its effective uid and gid to the uid and gid of the process that made the call. This ensures that the file or directory has the correct owner and group when it is created and also serves as an access check to ensure that the process that made the call has permission to modify the parent directory. However, this causes an EACCES error when the following conditions are met: * The parent directory has g+rw permissions with gid A * The process has gid B but has A in its list of supplementary groups In this case the fuse context only contains gid B, which doesn't have permission to modify the parent directory. Unfortunately there's no way for us to detect this on the server side so instead we just have to rely on the permission checks carried out by the kernel driver. If the server receives a create call, then assume that the kernel has verified that the process is allowed to create that file/directory and just create it without changing the server thread's uid and gid. Additionally, in order to ensure that a newly created file appears atomically in the parent directory with the proper owner and group, change the create implementation to use `O_TMPFILE` and `linkat` as described in the open(2) manpage. There is no `O_TMPFILE` equivalent for directories so create a "hidden" directory with a randomly generated name, modify the uid/gid and mode, and then rename it into place. BUG=b:156696212 TEST=tast run $DUT vm.Virtiofs TEST=Create a test directory with group wayland and permissions g+rw. Then run `su -s /bin/bash -c 'touch ${dir}/foo' - crosvm` and `su -s /bin/bash -c 'mkdir ${dir}/bar' - crosvm`. Change-Id: If5fbcb1b011664c7c1ac29542a2f90d129c34962 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2217534 Reviewed-by: Chirantan Ekbote Commit-Queue: Chirantan Ekbote Tested-by: Chirantan Ekbote --- seccomp/aarch64/fs_device.policy | 2 ++ seccomp/arm/fs_device.policy | 3 +++ seccomp/x86_64/fs_device.policy | 3 +++ 3 files changed, 8 insertions(+) (limited to 'seccomp') diff --git a/seccomp/aarch64/fs_device.policy b/seccomp/aarch64/fs_device.policy index 7bf794a..adeb9b6 100644 --- a/seccomp/aarch64/fs_device.policy +++ b/seccomp/aarch64/fs_device.policy @@ -6,7 +6,9 @@ copy_file_range: 1 fallocate: 1 +fchmod: 1 fchmodat: 1 +fchown: 1 fchownat: 1 fdatasync: 1 lgetxattr: 1 diff --git a/seccomp/arm/fs_device.policy b/seccomp/arm/fs_device.policy index 661883a..5290afa 100644 --- a/seccomp/arm/fs_device.policy +++ b/seccomp/arm/fs_device.policy @@ -6,7 +6,9 @@ copy_file_range: 1 fallocate: 1 +fchmod: 1 fchmodat: 1 +fchown32: 1 fchownat: 1 fdatasync: 1 lgetxattr: 1 @@ -23,6 +25,7 @@ geteuid32: 1 ioctl: arg1 == FS_IOC_GET_ENCRYPTION_POLICY || arg1 == FS_IOC_SET_ENCRYPTION_POLICY linkat: 1 _llseek: 1 +mkdir: 1 mkdirat: 1 mknodat: 1 open: return ENOENT diff --git a/seccomp/x86_64/fs_device.policy b/seccomp/x86_64/fs_device.policy index 1c10601..1454770 100644 --- a/seccomp/x86_64/fs_device.policy +++ b/seccomp/x86_64/fs_device.policy @@ -6,7 +6,9 @@ copy_file_range: 1 fallocate: 1 +fchmod: 1 fchmodat: 1 +fchown: 1 fchownat: 1 fdatasync: 1 lgetxattr: 1 @@ -22,6 +24,7 @@ geteuid: 1 ioctl: arg1 == FS_IOC_GET_ENCRYPTION_POLICY || arg1 == FS_IOC_SET_ENCRYPTION_POLICY linkat: 1 lseek: 1 +mkdir: 1 mkdirat: 1 mknodat: 1 newfstatat: 1 -- cgit 1.4.1