Hi.
On 06/04/2015 03:43 PM, Kinglong Mee wrote:
On 6/3/2015 12:09 AM, Stanislav Kholmanskikh wrote:
On 06/02/2015 12:23 AM, bfields@xxxxxxxxxxxx wrote:
On Mon, Jun 01, 2015 at 06:01:02PM +0300, Stanislav Kholmanskikh wrote:
Hello.
As the man page for utime/utimes states [1], EPERM is returned if
the second argument of utime/utimes is not NULL and:
* the caller's effective user id does not match the owner of the file
* the caller does not have write access to the file
* the caller is not privileged
However, I don't see this behavior with NFS, I see EACCES is
generated instead.
Agreed that it's probably a server bug. (Have you run across a case
where this makes a difference?)
Thank you.
No, I've not seen such a real-word scenario.
I have these LTP test cases failing:
* https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/utime/utime06.c
* https://github.com/linux-test-project/ltp/blob/master/testcases/kernel/syscalls/utimes/utimes01.c
and it makes me a bit nervous :)
Looking at nfsd_setattr().... The main work is done by notify_change(),
which is probably doing the right thing. But before that there's an
fh_verify()--looks like that is expected to fail in your case. I bet
that's the cause.
Yes, it is.
nfsd do the permission checking before notify_change() as,
/* This assumes NFSD_MAY_{READ,WRITE,EXEC} == MAY_{READ,WRITE,EXEC} */
err = inode_permission(inode, acc & (MAY_READ|MAY_WRITE|MAY_EXEC));
return -EACCES for non-owner user.
I doubt I can fix it by myself (at least quickly). So I am happy if anyone more experienced will look at it as well :)
Anyway, if nobody is interested, I'll give it a try, but later.
Here is a diff patch for this problem, please try testing.
If okay, I will send an official patch.
Note: must apply the following patch first in the url,
http://git.linux-nfs.org/?p=bfields/linux.git;a=commitdiff;h=cc265089ce1b176dde963c74b53593446ee7f99a
thanks,
Kinglong Mee
-------------------------------------------------------------
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index 84d770b..2533088 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -407,10 +371,23 @@ nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap,
bool get_write_count;
int size_change = 0;
- if (iap->ia_valid & (ATTR_ATIME | ATTR_MTIME | ATTR_SIZE))
+ if (iap->ia_valid & ATTR_SIZE) {
accmode |= NFSD_MAY_WRITE|NFSD_MAY_OWNER_OVERRIDE;
- if (iap->ia_valid & ATTR_SIZE)
ftype = S_IFREG;
+ }
+
+ /*
+ * According to utimes_common(),
+ *
+ * If times is NULL (or both times are UTIME_NOW),
+ * then we need to check permissions, because
+ * inode_change_ok() won't do it.
+ */
+ if (iap->ia_valid & (ATTR_ATIME | ATTR_MTIME)) {
+ accmode |= NFSD_MAY_OWNER_OVERRIDE;
+ if (!(iap->ia_valid & (ATTR_ATIME_SET | ATTR_MTIME_SET)))
+ accmode |= NFSD_MAY_WRITE;
+ }
/* Callers that do fh_verify should do the fh_want_write: */
get_write_count = !fhp->fh_dentry;
Tested with v4.1-rc6 plust the above two patches.
The issue is fixed.
My utime_test now reports EPERM. LTP's utime06, utimes01 now pass,
other LTP syscall test cases don't show any regression either.
Thank you!
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
