Re: NFSv3 may inappropriately return EPERM for fsetxattr

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 2975 bytes --]

On Thu, Aug 16 2018, Bruce Fields wrote:

> On Thu, Aug 16, 2018 at 10:39:35AM +1000, NeilBrown wrote:
>> On Tue, Aug 14 2018, Bruce Fields wrote:
>> > Honestly I'm not completely sure I understand the proposal.
>> 
>> Ok, here is a concrete RFC proposal which should make it easier to
>> understand.
>> I've tested that this fixes the specific problem in that a user with a
>> uid that doesn't match the file, but which the server will give
>> ownership rights to, can now setacl a file.
>
> Thanks, this makes sense to me.
>
> I might try to split this change into a couple steps, but I'm not sure
> exactly how.

I was originally thinking for keeping the nfsd change in a separate
patch, but it was so tiny...


>
> Minor nits:

All nits addressed - thanks.
>> @@ -2038,12 +2038,13 @@ nfsd_permission(struct svc_rqst *rqstp, struct svc_export *exp,
>>  	 * We must trust the client to do permission checking - using "ACCESS"
>>  	 * with NFSv3.
>>  	 */
>> -	if ((acc & NFSD_MAY_OWNER_OVERRIDE) &&
>> -	    uid_eq(inode->i_uid, current_fsuid()))
>> -		return 0;
>>  
>>  	/* This assumes  NFSD_MAY_{READ,WRITE,EXEC} == MAY_{READ,WRITE,EXEC} */
>
> Can we do the same for NFSD_MAY_OWNER_OVERRIDE and drop the extra "if"
> statement?

Probably.  If we apply this change first, then it should be trivial.
It would be a bit nicer if we could use enum for bits (a bit like "go"
allows), but I don't think this is too bad.  Thoughts?
(Do we really need NFSD_MAY_MASK ???)

Thanks,
NeilBrown

diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index a7e107309f76..6ca707511f65 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -13,13 +13,14 @@
  * Flags for nfsd_permission
  */
 #define NFSD_MAY_NOP			0
-#define NFSD_MAY_EXEC			0x001 /* == MAY_EXEC */
-#define NFSD_MAY_WRITE			0x002 /* == MAY_WRITE */
-#define NFSD_MAY_READ			0x004 /* == MAY_READ */
-#define NFSD_MAY_SATTR			0x008
-#define NFSD_MAY_TRUNC			0x010
-#define NFSD_MAY_LOCK			0x020
-#define NFSD_MAY_MASK			0x03f
+#define NFSD_MAY_EXEC			MAY_EXEC
+#define NFSD_MAY_WRITE			MAY_WRITE
+#define NFSD_MAY_READ			MAY_READ
+#define NFSD_MAY_SATTR			(__MAY_UNUSED << 0)
+#define NFSD_MAY_TRUNC			(__MAY_UNUSED << 1)
+#define NFSD_MAY_LOCK			(__MAY_UNUSED << 2)
+#define __NFSD_MAY_UNUSED		(__MAY_UNUSED << 3)
+#define NFSD_MAY_MASK			(__NFSD_MAY_UNUSED - 1)
 
 /* extra hints to permission and open routines: */
 #define NFSD_MAY_OWNER_OVERRIDE		0x040
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1ec33fd0423f..aed6a65f06b8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -93,6 +93,8 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 /* called from RCU mode, don't block */
 #define MAY_NOT_BLOCK		0x00000080
 
+#define	__MAY_UNUSED		0x00000100
+
 /*
  * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond
  * to O_WRONLY and O_RDWR via the strange trick in do_dentry_open()

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]