[PATCH] nfsd4: permit layoutget of executable-only files

[PATCH] nfsd4: permit layoutget of executable-only files

[Benjamin Coddington]

Clients must be able to read a file in order to execute it, and for pNFS
that means the client needs to be able to perform a LAYOUTGET on the file.

This behavior for executable-only files was added for OPEN in commit
a043226bc140 "nfsd4: permit read opens of executable-only files".

This fixes up xfstests generic/126 on block/scsi layouts.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
---
 fs/nfsd/nfs4proc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 8487486ec496..8ce60986fb75 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1372,14 +1372,14 @@ nfsd4_layoutget(struct svc_rqst *rqstp,
 	const struct nfsd4_layout_ops *ops;
 	struct nfs4_layout_stateid *ls;
 	__be32 nfserr;
-	int accmode;
+	int accmode = NFSD_MAY_READ_IF_EXEC;
 
 	switch (lgp->lg_seg.iomode) {
 	case IOMODE_READ:
-		accmode = NFSD_MAY_READ;
+		accmode |= NFSD_MAY_READ;
 		break;
 	case IOMODE_RW:
-		accmode = NFSD_MAY_READ | NFSD_MAY_WRITE;
+		accmode |= NFSD_MAY_READ | NFSD_MAY_WRITE;
 		break;
 	default:
 		dprintk("%s: invalid iomode %d\n",
-- 
2.9.3

Re: [PATCH] nfsd4: permit layoutget of executable-only files

[J. Bruce Fields]

On Tue, Dec 19, 2017 at 09:35:25AM -0500, Benjamin Coddington wrote:
> Clients must be able to read a file in order to execute it, and for pNFS
> that means the client needs to be able to perform a LAYOUTGET on the file.
> 
> This behavior for executable-only files was added for OPEN in commit
> a043226bc140 "nfsd4: permit read opens of executable-only files".
> 
> This fixes up xfstests generic/126 on block/scsi layouts.

Thanks, applied.  So the server was returning NFS4ERR_ACCESS and the
client was returning that to the application?  I was wondering for a
moment whether the client should instead try falling back to MDS IO,
but.... But I don't think that makes sense.  The client's probably
correct to interpret ACCESS as just meaning that user can't read the
file.  The server has plenty of other errors to choose from if it just
wants to deny the layout for some reason.  OK.

--b.

> 
> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
> ---
>  fs/nfsd/nfs4proc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 8487486ec496..8ce60986fb75 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1372,14 +1372,14 @@ nfsd4_layoutget(struct svc_rqst *rqstp,
>  	const struct nfsd4_layout_ops *ops;
>  	struct nfs4_layout_stateid *ls;
>  	__be32 nfserr;
> -	int accmode;
> +	int accmode = NFSD_MAY_READ_IF_EXEC;
>  
>  	switch (lgp->lg_seg.iomode) {
>  	case IOMODE_READ:
> -		accmode = NFSD_MAY_READ;
> +		accmode |= NFSD_MAY_READ;
>  		break;
>  	case IOMODE_RW:
> -		accmode = NFSD_MAY_READ | NFSD_MAY_WRITE;
> +		accmode |= NFSD_MAY_READ | NFSD_MAY_WRITE;
>  		break;
>  	default:
>  		dprintk("%s: invalid iomode %d\n",
> -- 
> 2.9.3

Re: [PATCH] nfsd4: permit layoutget of executable-only files

[Trond Myklebust]

T24gVHVlLCAyMDE3LTEyLTE5IGF0IDEwOjQzIC0wNTAwLCBKLiBCcnVjZSBGaWVsZHMgd3JvdGU6
DQo+IE9uIFR1ZSwgRGVjIDE5LCAyMDE3IGF0IDA5OjM1OjI1QU0gLTA1MDAsIEJlbmphbWluIENv
ZGRpbmd0b24gd3JvdGU6DQo+ID4gQ2xpZW50cyBtdXN0IGJlIGFibGUgdG8gcmVhZCBhIGZpbGUg
aW4gb3JkZXIgdG8gZXhlY3V0ZSBpdCwgYW5kIGZvcg0KPiA+IHBORlMNCj4gPiB0aGF0IG1lYW5z
IHRoZSBjbGllbnQgbmVlZHMgdG8gYmUgYWJsZSB0byBwZXJmb3JtIGEgTEFZT1VUR0VUIG9uDQo+
ID4gdGhlIGZpbGUuDQo+ID4gDQo+ID4gVGhpcyBiZWhhdmlvciBmb3IgZXhlY3V0YWJsZS1vbmx5
IGZpbGVzIHdhcyBhZGRlZCBmb3IgT1BFTiBpbg0KPiA+IGNvbW1pdA0KPiA+IGEwNDMyMjZiYzE0
MCAibmZzZDQ6IHBlcm1pdCByZWFkIG9wZW5zIG9mIGV4ZWN1dGFibGUtb25seSBmaWxlcyIuDQo+
ID4gDQo+ID4gVGhpcyBmaXhlcyB1cCB4ZnN0ZXN0cyBnZW5lcmljLzEyNiBvbiBibG9jay9zY3Np
IGxheW91dHMuDQo+IA0KPiBUaGFua3MsIGFwcGxpZWQuICBTbyB0aGUgc2VydmVyIHdhcyByZXR1
cm5pbmcgTkZTNEVSUl9BQ0NFU1MgYW5kIHRoZQ0KPiBjbGllbnQgd2FzIHJldHVybmluZyB0aGF0
IHRvIHRoZSBhcHBsaWNhdGlvbj8gIEkgd2FzIHdvbmRlcmluZyBmb3IgYQ0KPiBtb21lbnQgd2hl
dGhlciB0aGUgY2xpZW50IHNob3VsZCBpbnN0ZWFkIHRyeSBmYWxsaW5nIGJhY2sgdG8gTURTIElP
LA0KPiBidXQuLi4uIEJ1dCBJIGRvbid0IHRoaW5rIHRoYXQgbWFrZXMgc2Vuc2UuICBUaGUgY2xp
ZW50J3MgcHJvYmFibHkNCj4gY29ycmVjdCB0byBpbnRlcnByZXQgQUNDRVNTIGFzIGp1c3QgbWVh
bmluZyB0aGF0IHVzZXIgY2FuJ3QgcmVhZCB0aGUNCj4gZmlsZS4gIFRoZSBzZXJ2ZXIgaGFzIHBs
ZW50eSBvZiBvdGhlciBlcnJvcnMgdG8gY2hvb3NlIGZyb20gaWYgaXQNCj4ganVzdA0KPiB3YW50
cyB0byBkZW55IHRoZSBsYXlvdXQgZm9yIHNvbWUgcmVhc29uLiAgT0suDQo+IA0KDQpUaGVyZSBh
cmUgc2V2ZXJhbCBlcnJvcnMgd2hpY2ggY2FuIGJlIHJldHVybmVkIGJ5IExBWU9VVEdFVCwgYW5k
IHdoaWNoDQp0aGUgY2xpZW50IHdpbGwgcGFzcyBiYWNrIHRvIHRoZSBhcHBsaWNhdGlvbi4gTkZT
NEVSUl9BQ0NFU1MgaXMgb25lLg0KTkZTNEVSUl9EUVVPVCwgTkZTNEVSUl9JTywgTkZTNEVSUl9O
T1NQQywgTkZTNEVSUl9TVEFMRSwgYXJlIG90aGVycw0KdGhhdCB5b3UgbWlnaHQgd2FudCB0byBi
ZSBhd2FyZSBvZi4uLg0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNsaWVudCBt
YWludGFpbmVyLCBQcmltYXJ5RGF0YQ0KdHJvbmQubXlrbGVidXN0QHByaW1hcnlkYXRhLmNvbQ0K

Re: [PATCH] nfsd4: permit layoutget of executable-only files

[Benjamin Coddington]

On 19 Dec 2017, at 11:00, Trond Myklebust wrote:

> On Tue, 2017-12-19 at 10:43 -0500, J. Bruce Fields wrote:
>> On Tue, Dec 19, 2017 at 09:35:25AM -0500, Benjamin Coddington wrote:
>>> Clients must be able to read a file in order to execute it, and for
>>> pNFS
>>> that means the client needs to be able to perform a LAYOUTGET on
>>> the file.
>>>
>>> This behavior for executable-only files was added for OPEN in
>>> commit
>>> a043226bc140 "nfsd4: permit read opens of executable-only files".
>>>
>>> This fixes up xfstests generic/126 on block/scsi layouts.
>>
>> Thanks, applied.  So the server was returning NFS4ERR_ACCESS and the
>> client was returning that to the application?  I was wondering for a
>> moment whether the client should instead try falling back to MDS IO,
>> but.... But I don't think that makes sense.  The client's probably
>> correct to interpret ACCESS as just meaning that user can't read the
>> file.  The server has plenty of other errors to choose from if it
>> just
>> wants to deny the layout for some reason.  OK.
>>
>
> There are several errors which can be returned by LAYOUTGET, and which
> the client will pass back to the application. NFS4ERR_ACCESS is one.

And just to be perfectly clear, yes, that's what I see.

Ben

Re: [PATCH] nfsd4: permit layoutget of executable-only files

[J. Bruce Fields]

On Tue, Dec 19, 2017 at 12:10:48PM -0500, Benjamin Coddington wrote:
> On 19 Dec 2017, at 11:00, Trond Myklebust wrote:
> 
> > On Tue, 2017-12-19 at 10:43 -0500, J. Bruce Fields wrote:
> >> On Tue, Dec 19, 2017 at 09:35:25AM -0500, Benjamin Coddington wrote:
> >>> Clients must be able to read a file in order to execute it, and for
> >>> pNFS
> >>> that means the client needs to be able to perform a LAYOUTGET on
> >>> the file.
> >>>
> >>> This behavior for executable-only files was added for OPEN in
> >>> commit
> >>> a043226bc140 "nfsd4: permit read opens of executable-only files".
> >>>
> >>> This fixes up xfstests generic/126 on block/scsi layouts.
> >>
> >> Thanks, applied.  So the server was returning NFS4ERR_ACCESS and the
> >> client was returning that to the application?  I was wondering for a
> >> moment whether the client should instead try falling back to MDS IO,
> >> but.... But I don't think that makes sense.  The client's probably
> >> correct to interpret ACCESS as just meaning that user can't read the
> >> file.  The server has plenty of other errors to choose from if it
> >> just
> >> wants to deny the layout for some reason.  OK.
> >>
> >
> > There are several errors which can be returned by LAYOUTGET, and which
> > the client will pass back to the application. NFS4ERR_ACCESS is one.
> 
> And just to be perfectly clear, yes, that's what I see.

Got it, thanks.b

--b.