From: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Vasily Isaenko <vasily.isaenko@oracle.com>,
linux-nfs@vger.kernel.org, xfs@oss.sgi.com
Subject: Re: nfs vs xfstests 193
Date: Fri, 06 Dec 2013 17:20:34 +0400 [thread overview]
Message-ID: <52A1CF22.106@oracle.com> (raw)
In-Reply-To: <20131106115648.GA24804@infradead.org>
On 11/06/2013 03:56 PM, Christoph Hellwig wrote:
> I've noticed that xfstests 193 fails when run over NFS talking to an
> XFS-based Linux server. The test checks that we behave correctly
> vs Posix 1003.1 for the various operations that end up in ->setattr.
>
> Without the no_root_squash export flag we're not even able to run
> something resembling the test as we get permission problems all through
> it, see the first attachment for details.
>
> With the root_squash export op we fail to clear the setuid/setgid
> bits in various truncate and chown subtests, see the second attachment
> for details.
Hi!
I've come across the same issue. But NFS server is backed by ext4 file
system in my environment.
The test case quotes POSIX:
"If the specified file is a regular file, one or more of the S_IXUSR,
S_IXGRP, or S_IXOTH bits of the
file mode are set, and the process has appropriate privileges, it is
implementation-defined whether the set-user-ID and set-group-ID
bits are altered."
So the difference that what we have now:
between nfs:
~# touch file; chmod ug+s file; chmod u+x file; ls -l file; chown root
file; ls -l file; rm -f file
-rwsr-Sr-- 1 root root 0 Dec 6 04:49 file
-rwsr-Sr-- 1 root root 0 Dec 6 04:49 file
and ext3, ext4, xfs, btrfs:
~# touch file; chmod ug+s file; chmod u+x file; ls -l file; chown root
file; ls -l file; rm -f file
-rwsr-Sr-- 1 root root 0 Dec 6 04:49 file
-rwxr-Sr-- 1 root root 0 Dec 6 04:49 file
is not a violation of this POSIX statement. But It's just an
"implementation-defined" behaviour.
I suppose that the difference raises because of this part of code in
fs/nfsd/vfs.c:
/* Revoke setuid/setgid on chown */
if (!S_ISDIR(inode->i_mode) &&
(((iap->ia_valid & ATTR_UID) && !uid_eq(iap->ia_uid,
inode->i_uid)) ||
((iap->ia_valid & ATTR_GID) && !gid_eq(iap->ia_gid,
inode->i_gid)))) {
iap->ia_valid |= ATTR_KILL_PRIV;
if (iap->ia_valid & ATTR_MODE) {
/* we're setting mode too, just clear the s*id
bits */
iap->ia_mode &= ~S_ISUID;
if (iap->ia_mode & S_IXGRP)
iap->ia_mode &= ~S_ISGID;
} else {
/* set ATTR_KILL_* bits and let VFS handle it */
iap->ia_valid |= (ATTR_KILL_SUID | ATTR_KILL_SGID);
}
}
uid_eq() and gid_eq() checkings allow removal of s*id bits only if the
owner/group of the file is changed during chown().
I.e. on nfs:
~# touch file; chmod ug+s file; chmod u+x file; ls -l file; chown bin
file; ls -l file; rm -f file
-rwsr-Sr-- 1 root root 0 Dec 6 05:02 file
-rwxr-Sr-- 1 bin root 0 Dec 6 05:02 file
Is it acceptable to change NFS kernel server behaviour by removal of
!uid_eq(iap->ia_uid, inode->i_uid) and !gid_eq(iap->ia_gid,
inode->i_gid) from the condition above?
Just to make the behaviour more consistent between NFS and other "local"
file systems as It was done by
commit
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=0953e620de0538cbd081f1b45126f6098112a598
Thank you!
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
WARNING: multiple messages have this Message-ID (diff)
From: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: xfs@oss.sgi.com, linux-nfs@vger.kernel.org,
Vasily Isaenko <vasily.isaenko@oracle.com>
Subject: Re: nfs vs xfstests 193
Date: Fri, 06 Dec 2013 17:20:34 +0400 [thread overview]
Message-ID: <52A1CF22.106@oracle.com> (raw)
In-Reply-To: <20131106115648.GA24804@infradead.org>
On 11/06/2013 03:56 PM, Christoph Hellwig wrote:
> I've noticed that xfstests 193 fails when run over NFS talking to an
> XFS-based Linux server. The test checks that we behave correctly
> vs Posix 1003.1 for the various operations that end up in ->setattr.
>
> Without the no_root_squash export flag we're not even able to run
> something resembling the test as we get permission problems all through
> it, see the first attachment for details.
>
> With the root_squash export op we fail to clear the setuid/setgid
> bits in various truncate and chown subtests, see the second attachment
> for details.
Hi!
I've come across the same issue. But NFS server is backed by ext4 file
system in my environment.
The test case quotes POSIX:
"If the specified file is a regular file, one or more of the S_IXUSR,
S_IXGRP, or S_IXOTH bits of the
file mode are set, and the process has appropriate privileges, it is
implementation-defined whether the set-user-ID and set-group-ID
bits are altered."
So the difference that what we have now:
between nfs:
~# touch file; chmod ug+s file; chmod u+x file; ls -l file; chown root
file; ls -l file; rm -f file
-rwsr-Sr-- 1 root root 0 Dec 6 04:49 file
-rwsr-Sr-- 1 root root 0 Dec 6 04:49 file
and ext3, ext4, xfs, btrfs:
~# touch file; chmod ug+s file; chmod u+x file; ls -l file; chown root
file; ls -l file; rm -f file
-rwsr-Sr-- 1 root root 0 Dec 6 04:49 file
-rwxr-Sr-- 1 root root 0 Dec 6 04:49 file
is not a violation of this POSIX statement. But It's just an
"implementation-defined" behaviour.
I suppose that the difference raises because of this part of code in
fs/nfsd/vfs.c:
/* Revoke setuid/setgid on chown */
if (!S_ISDIR(inode->i_mode) &&
(((iap->ia_valid & ATTR_UID) && !uid_eq(iap->ia_uid,
inode->i_uid)) ||
((iap->ia_valid & ATTR_GID) && !gid_eq(iap->ia_gid,
inode->i_gid)))) {
iap->ia_valid |= ATTR_KILL_PRIV;
if (iap->ia_valid & ATTR_MODE) {
/* we're setting mode too, just clear the s*id
bits */
iap->ia_mode &= ~S_ISUID;
if (iap->ia_mode & S_IXGRP)
iap->ia_mode &= ~S_ISGID;
} else {
/* set ATTR_KILL_* bits and let VFS handle it */
iap->ia_valid |= (ATTR_KILL_SUID | ATTR_KILL_SGID);
}
}
uid_eq() and gid_eq() checkings allow removal of s*id bits only if the
owner/group of the file is changed during chown().
I.e. on nfs:
~# touch file; chmod ug+s file; chmod u+x file; ls -l file; chown bin
file; ls -l file; rm -f file
-rwsr-Sr-- 1 root root 0 Dec 6 05:02 file
-rwxr-Sr-- 1 bin root 0 Dec 6 05:02 file
Is it acceptable to change NFS kernel server behaviour by removal of
!uid_eq(iap->ia_uid, inode->i_uid) and !gid_eq(iap->ia_gid,
inode->i_gid) from the condition above?
Just to make the behaviour more consistent between NFS and other "local"
file systems as It was done by
commit
https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/?id=0953e620de0538cbd081f1b45126f6098112a598
Thank you!
next prev parent reply other threads:[~2013-12-06 13:21 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-06 11:56 nfs vs xfstests 193 Christoph Hellwig
2013-11-06 11:56 ` Christoph Hellwig
2013-12-06 13:20 ` Stanislav Kholmanskikh [this message]
2013-12-06 13:20 ` Stanislav Kholmanskikh
2013-12-06 18:08 ` Christoph Hellwig
2013-12-06 18:08 ` Christoph Hellwig
2013-12-06 20:44 ` J. Bruce Fields
2013-12-06 20:44 ` J. Bruce Fields
2013-12-06 20:47 ` J. Bruce Fields
2013-12-06 20:47 ` J. Bruce Fields
2013-12-10 14:43 ` Stanislav Kholmanskikh
2013-12-10 14:43 ` Stanislav Kholmanskikh
2013-12-11 10:16 ` [PATCH] nfsd: revoking of suid/sgid bits after chown() in a consistent way Stanislav Kholmanskikh
2013-12-11 10:16 ` Stanislav Kholmanskikh
2013-12-11 11:00 ` Stanislav Kholmanskikh
2013-12-11 11:00 ` Stanislav Kholmanskikh
2013-12-12 3:38 ` J. Bruce Fields
2013-12-12 3:38 ` J. Bruce Fields
2013-12-12 8:13 ` Christoph Hellwig
2013-12-12 8:13 ` Christoph Hellwig
2013-12-12 11:44 ` Stanislav Kholmanskikh
2013-12-12 11:44 ` Stanislav Kholmanskikh
2013-12-12 16:01 ` J. Bruce Fields
2013-12-12 16:01 ` J. Bruce Fields
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=52A1CF22.106@oracle.com \
--to=stanislav.kholmanskikh@oracle.com \
--cc=hch@infradead.org \
--cc=linux-nfs@vger.kernel.org \
--cc=vasily.isaenko@oracle.com \
--cc=xfs@oss.sgi.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.