linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Salvatore Bonaccorso <carnil@debian.org>
To: "J. Bruce Fields" <bfields@redhat.com>
Cc: Elliott Mitchell <ehem+debian@m5p.com>,
	962254@bugs.debian.org, linux-nfs@vger.kernel.org,
	agruenba@redhat.com
Subject: Re: Umask ignored when mounting NFSv4.2 share of an exported Filesystem with noacl (was: Re: Bug#962254: NFS(v4) broken at 4.19.118-2)
Date: Tue, 16 Jun 2020 07:28:35 +0200	[thread overview]
Message-ID: <20200616052835.GA19246@lorien.valinor.li> (raw)
In-Reply-To: <20200616023820.GB214986@pick.fieldses.org>

Hi Bruce,

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.
> 
> So the obvious fix is something like (untested!)
> 
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 0aa02eb18bd3..dabdcca58969 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -1234,6 +1234,8 @@ nfsd_create_locked(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  			nfsd_check_ignore_resizing(iap);
>  		break;
>  	case S_IFDIR:
> +		if (!IS_POSIXACL(dirp))
> +			iap->ia_mode &= ~current_umask();
>  		host_err = vfs_mkdir(dirp, dchild, iap->ia_mode);
>  		if (!host_err && unlikely(d_unhashed(dchild))) {
>  			struct dentry *d;

Thank you!

Tested your patch on top, and it would solve the directory case, but
the underlying problem is more general (and as you said proably needs
further checking in other places):

root@nfs-test:~# mount -t nfs 192.168.122.150:/srv/data /mnt
root@nfs-test:~# mkdir /mnt/foo && ls -ld /mnt/foo && rmdir /mnt/foo
drwxr-xr-x 2 root root 4096 Jun 16 07:24 /mnt/foo
root@nfs-test:~# touch /mnt/foo && ls -ld /mnt/foo && rm /mnt/foo
-rw-rw-rw- 1 root root 0 Jun 16 07:25 /mnt/foo
root@nfs-test:~# umount /mnt
root@nfs-test:~#

So when creating files the umask is still ignored in the noacl mounted
case.

Regards,
Salvatore

  parent reply	other threads:[~2020-06-16  5:28 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200605051607.GA34405@mattapan.m5p.com>
     [not found] ` <20200605064426.GA1538868@eldamar.local>
     [not found]   ` <20200605174349.GA40135@mattapan.m5p.com>
     [not found]     ` <20200605183631.GA1720057@eldamar.local>
     [not found]       ` <20200611223711.GA37917@mattapan.m5p.com>
2020-06-13 12:54         ` Umask ignored when mounting NFSv4.2 share of an exported ZFS (with acltype=off) (was: Re: Bug#962254: NFS(v4) broken at 4.19.118-2) Salvatore Bonaccorso
2020-06-13 18:45           ` Elliott Mitchell
2020-06-15 14:50             ` J. Bruce Fields
2020-06-15 18:53               ` Umask ignored when mounting NFSv4.2 share of an exported Filesystem with noacl " Salvatore Bonaccorso
2020-06-16  2:38                 ` J. Bruce Fields
2020-06-16  2:42                   ` J. Bruce Fields
2020-06-16  5:32                     ` Salvatore Bonaccorso
2020-06-16 16:16                     ` Salvatore Bonaccorso
2020-06-17  0:58                       ` J. Bruce Fields
2020-06-17  4:58                         ` Umask ignored when mounting NFSv4.2 share of an exported Filesystem with noacl Salvatore Bonaccorso
2020-06-17 12:46                           ` J. Bruce Fields
2020-06-17 14:42                       ` Umask ignored when mounting NFSv4.2 share of an exported Filesystem with noacl (was: Re: Bug#962254: NFS(v4) broken at 4.19.118-2) Andreas Gruenbacher
2020-06-17 15:31                         ` J. Bruce Fields
2020-06-17 16:50                           ` Andreas Gruenbacher
2020-06-16  5:28                   ` Salvatore Bonaccorso [this message]
2020-06-16  1:57               ` Umask ignored when mounting NFSv4.2 share of an exported ZFS (with acltype=off) " Elliott Mitchell
2020-06-15 11:55           ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200616052835.GA19246@lorien.valinor.li \
    --to=carnil@debian.org \
    --cc=962254@bugs.debian.org \
    --cc=agruenba@redhat.com \
    --cc=bfields@redhat.com \
    --cc=ehem+debian@m5p.com \
    --cc=linux-nfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).