Re: [PATCH v1 1/2] idmapped-mounts: Add mknodat operation in setgid test

["xuyang2018.jy@fujitsu.com" <xuyang2018.jy@fujitsu.com>]

on 2022/3/31 19:59, Christian Brauner wrote:
> On Thu, Mar 31, 2022 at 05:28:21PM +0800, Yang Xu wrote:
>> Since mknodat can create file, we should also check whether strip S_ISGID.
>> Also add new helper caps_down_fsetid to drop CAP_FSETID because strip S_ISGID
>> depond on this cap and keep other cap(ie CAP_MKNOD) because create character device
>> needs it when using mknod.
>>
>> Only test mknod with character device in setgid_create function because the another
>> two functions will hit EPERM error.
>
> Fwiw, it's not allowed to create devices in userns as that would be a
> massive attack vector. But it is possible since 5.<some version>  to
> create whiteouts in userns for the sake of overlayfs. So iirc that
> creating a whiteout is just passing 0 as dev_t:
>
> mknodat(t_dir1_fd, CHRDEV1, S_IFCHR | S_ISGID | 0755, 0)
>
> but you'd need to detect whether the kernel allows this and skip the
> test on EPERM when it is a userns test.

I have found the kernel commit 	a3c751a50 ("vfs: allow unprivileged 
whiteout creation") in v5.8-rc1.
Thanks. Will create whiteout instead of actual character device on v2.


>
>>
>> Signed-off-by: Yang Xu<xuyang2018.jy@fujitsu.com>
>> ---
>
> Sidenote: I really need to rename the test binary to something other
> than idmapped-mounts.c as this tests a lot of generic vfs stuff that has
> nothing to do with them.
Agree.

Best Regards
Yang Xu
>
> In any case, I pulled and tested this:
>
> Tested-by: Christian Brauner (Microsoft)<brauner@kernel.org>
> Reviewed-by: Christian Brauner (Microsoft)<brauner@kernel.org>