Re: NFSv3 may inappropriately return EPERM for fsetxattr

[NeilBrown <neilb@suse.com>]

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

On Tue, Aug 14 2018, Bruce Fields wrote:

> On Tue, Aug 14, 2018 at 07:03:14PM +1000, NeilBrown wrote:
>> On Mon, Aug 13 2018, NeilBrown wrote:
>> 
>> > On Sun, Aug 12 2018, Bruce Fields wrote:
>> >> OK, so not too important.  Still, it sounds like
>> >> inode_owner_or_capable() is something people expect to work for any
>> >> filesystem, so I wonder if there's a way to do that.  Or at least
>> >> disable it.
>> >
>> > We could add a new flag - MAY_OWN (or something) - to the flags
>> > recognised by inode_permission() and i_op->permission().
>> >
>> > If ->permission isn't set, inode_permission() uses
>> > inode_owner_or_capable().
>> > If it is, it gets to call that, or do whatever is appropriate.
>> >
>> > Is this flag the same as NFS_MAY_OWNER_OVERRIDE or not....??
>> >
>> 
>> Pursuing this thought...
>>   NFSD_MAY_OWNER_OVERRIDE means "an operation is requested which
>>    may always be performed by the owner of the file, even if they
>>    don't have explicit permission via DAC setting."
>> 
>> I think this is a reasonable description of how inode_owner_or_capable()
>> is used.  It is sometimes used on its own, where there is no permission
>> but that is relevant such as O_NOATIME or set_posix_acl(), or is used
>> as a precursor to and inode_permission() check, as in notify_change().
>> 
>> The biggest difference is that NFSD_MAY_OWNER_OVERRIDE does have the
>> "or_capable".
>> As nfsd drops CAP_FOWNER, and the extra test won't hurt it.
>> 
>> So I now think that a good solution to this problem would be to hoist
>> NFSD_MAY_OWNER_OVERRIDE into the VFS and change inode_permission() and
>> various i_op->permission functions to handle it.
>> 
>> All we need is a good name....
>>   MAY_BY_OWNER  ???
>>   MAY_IF_OWNER
>>   MAY_BE_OWNER ???
>> 
>> MAY_READ means "may I please read this file".  The flag needs to say
>> "may I act as the owner of this file", so
>>   MAY_ACT_AS_OWNER ????
>
> It's still a little different from the other permission bits in that I
> believe
>
> 	permission(., READ|WRITE)
> 		== permission(., READ) && permission(., WRITE)
>
> but
>
> 	permission(., READ|OWNER_OVERRIDE)
> 		== permission(., READ) || permission(., OWNER_OVERRIDE)
>

A little different from some other permission bits.
We have

#define MAY_EXEC		0x00000001
#define MAY_WRITE		0x00000002
#define MAY_READ		0x00000004
#define MAY_APPEND		0x00000008
#define MAY_ACCESS		0x00000010
#define MAY_OPEN		0x00000020
#define MAY_CHDIR		0x00000040
/* called from RCU mode, don't block */
#define MAY_NOT_BLOCK		0x00000080

MAY_CHDIR says something like "test the other bits, but first make sure
your cache is up-to-date".
MAY_NOT_BLOCK says "test the other bits, but not if you would need to
block.

MAY_OWNER would be "test the other bits, but only if not the owner".

So: not much more ad-hoc than other bits.

> ?
>
> Anyway, naming aside....  I don't know, sounds like it might work?
> Honestly I'm not completely sure I understand the proposal.

I guess I should supply a patch...

NeilBrown

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