Hi Bruce,
On Mon, Jun 15, 2020 at 10:42:12PM -0400, J. Bruce Fields wrote:
> On Mon, Jun 15, 2020 at 10:38:20PM -0400, J. Bruce Fields wrote:
> > Thanks for the detailed reproducer.
> >
> > It's weird, as the server is basically just setting the transmitted
> > umask and then calling into the vfs to handle the rest, so it's not much
> > different from any other user. But the same reproducer run just on the
> > ext4 filesystem does give the right permissions....
> >
> > Oh, but looking at the system call, fs_namei.c:do_mkdirat(), it does:
> >
> > if (!IS_POSIXACL(path.dentry->d_inode))
> > mode &= ~current_umask();
> > error = security_path_mkdir(&path, dentry, mode);
> > if (!error)
> > error = vfs_mkdir(path.dentry->d_inode, dentry, mode);
> >
> > whereas nfsd just calls into vfs_mkdir().
> >
> > And that IS_POSIXACL() check is exactly a check whether the filesystem
> > supports ACLs. So I guess it's the responsibility of the caller of
> > vfs_mkdir() to handle that case.
>
> But, that's unsatisfying: why isn't vfs_mkdir() taking care of this
> itself? And what about that security_path_mkdir() call? And are the
> other cases of that switch in fs/nfsd/vfs.c:nfsd_create_locked()
> correct? I think there may be some more cleanup here called for, I'll
> poke around tomorrow.
This might be unneeded to test but as additional datapoint which
confirms the suspect: I tried check the commit around 47057abde515
("nfsd: add support for the umask attribute") in 4.10-rc1
A kernel built with 47057abde515~1, and mounting from an enough recent
client which has at least dff25ddb4808 ("nfs: add support for the
umask attribute") does not show the observed behaviour, the server
built with 47057abde515 does.
Regards,
Salvatore
