2015-04-21 20:22:18

by J. Bruce Fields

[permalink] [raw]
Subject: patches for 4.1

These fix a couple bugs I noticed (plus one Christoph caught) while
reviewing some nfsd permissions checking (partly for richacl review).

I'll probably send my 4.1 pull request tomorrow, with these (and the
rest mostly small bugfixes too).

--b.



2015-04-21 20:22:18

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 2/3] nfsd4: disallow SEEK with special stateids

From: "J. Bruce Fields" <[email protected]>

If the client uses a special stateid then we'll pass a NULL file to
vfs_llseek.

Fixes: 24bab491220f " NFSD: Implement SEEK"
Cc: Anna Schumaker <[email protected]>
Cc: [email protected]
Reported-by: Christoph Hellwig <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4proc.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index d0848fc6529e..4a8314f08a0e 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1071,6 +1071,8 @@ nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
dprintk("NFSD: nfsd4_seek: couldn't process stateid!\n");
return status;
}
+ if (!file)
+ return nfserr_bad_stateid;

switch (seek->seek_whence) {
case NFS4_CONTENT_DATA:
--
1.9.3


2015-04-21 20:22:19

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 1/3] nfsd4: disallow ALLOCATE with special stateids

From: "J. Bruce Fields" <[email protected]>

vfs_fallocate will hit a NULL dereference if the client tries an
ALLOCATE or DEALLOCATE with a special stateid. Fix that. (We also
depend on the open to have broken any conflicting leases or delegations
for us.)

(If it turns out we need to allow special stateid's then we could do a
temporary open here in the special-stateid case, as we do for read and
write. For now I'm assuming it's not necessary.)

Fixes: 95d871f03cae "nfsd: Add ALLOCATE support"
Cc: [email protected]
Cc: Anna Schumaker <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4proc.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index b401da386604..d0848fc6529e 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1030,6 +1030,8 @@ nfsd4_fallocate(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
dprintk("NFSD: nfsd4_fallocate: couldn't process stateid!\n");
return status;
}
+ if (!file)
+ return nfserr_bad_stateid;

status = nfsd4_vfs_fallocate(rqstp, &cstate->current_fh, file,
fallocate->falloc_offset,
--
1.9.3


2015-04-21 20:22:18

by J. Bruce Fields

[permalink] [raw]
Subject: [PATCH 3/3] nfsd4: fix READ permission checking

From: "J. Bruce Fields" <[email protected]>

In the case we already have a struct file (derived from a stateid), we
still need to do permission-checking; otherwise an unauthorized user
could gain access to a file by sniffing or guessing somebody else's
stateid.

Cc: [email protected]
Fixes: dc97618ddda9 "nfsd4: separate splice and readv cases"
Signed-off-by: J. Bruce Fields <[email protected]>
---
fs/nfsd/nfs4xdr.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index cea6c529434f..a45032ce7b80 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -3422,6 +3422,7 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr,
unsigned long maxcount;
struct xdr_stream *xdr = &resp->xdr;
struct file *file = read->rd_filp;
+ struct svc_fh *fhp = read->rd_fhp;
int starting_len = xdr->buf->len;
struct raparms *ra;
__be32 *p;
@@ -3445,12 +3446,15 @@ nfsd4_encode_read(struct nfsd4_compoundres *resp, __be32 nfserr,
maxcount = min_t(unsigned long, maxcount, (xdr->buf->buflen - xdr->buf->len));
maxcount = min_t(unsigned long, maxcount, read->rd_length);

- if (!read->rd_filp) {
+ if (read->rd_filp)
+ err = nfsd_permission(resp->rqstp, fhp->fh_export,
+ fhp->fh_dentry,
+ NFSD_MAY_READ|NFSD_MAY_OWNER_OVERRIDE);
+ else
err = nfsd_get_tmp_read_open(resp->rqstp, read->rd_fhp,
&file, &ra);
- if (err)
- goto err_truncate;
- }
+ if (err)
+ goto err_truncate;

if (file->f_op->splice_read && test_bit(RQ_SPLICE_OK, &resp->rqstp->rq_flags))
err = nfsd4_encode_splice_read(resp, read, file, maxcount);
--
1.9.3


2015-04-21 20:26:48

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 2/3] nfsd4: disallow SEEK with special stateids

On Tue, Apr 21, 2015 at 04:22:14PM -0400, J. Bruce Fields wrote:
> From: "J. Bruce Fields" <[email protected]>
>
> If the client uses a special stateid then we'll pass a NULL file to
> vfs_llseek.

By the way, Christoph has also pointed out that it's probably worth
fixing nfsd4_preprocess_stateid_op() to do a temporary open itself in
the special stateid case.

If we wanted to get nfsd4_preprocess_stateid_op() to the point where it
also returned a file (if requested) on success, then there'd also be the
possible NULL returns from find_{read|write}able_file() to handle.

I haven't figured that out yet.

--b.

>
> Fixes: 24bab491220f " NFSD: Implement SEEK"
> Cc: Anna Schumaker <[email protected]>
> Cc: [email protected]
> Reported-by: Christoph Hellwig <[email protected]>
> Signed-off-by: J. Bruce Fields <[email protected]>
> ---
> fs/nfsd/nfs4proc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index d0848fc6529e..4a8314f08a0e 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1071,6 +1071,8 @@ nfsd4_seek(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> dprintk("NFSD: nfsd4_seek: couldn't process stateid!\n");
> return status;
> }
> + if (!file)
> + return nfserr_bad_stateid;
>
> switch (seek->seek_whence) {
> case NFS4_CONTENT_DATA:
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html