Skip to content

Commit ba21e20

Browse files
committed
NFSD: Use svcxdr_encode_opaque_pages() in nfsd4_encode_splice_read()
Commit 15b23ef ("nfsd4: fix corruption of NFSv4 read data") encountered exactly the same issue: after a splice read, a filesystem-owned page is left in rq_pages[]; the symptoms are the same as described there. If the computed number of pages in nfsd4_encode_splice_read() is not exactly the same as the actual number of pages that were consumed by nfsd_splice_actor() (say, because of a bug) then hilarity ensues. Instead of recomputing the page offset based on the size of the payload, use rq_next_page, which is already properly updated by nfsd_splice_actor(), to cause svc_rqst_release_pages() to operate correctly in every instance. This is a defensive change since we believe that after commit 27c934d ("nfsd: don't replace page in rq_pages if it's a continuation of last page") has been applied, there are no known opportunities for nfsd_splice_actor() to screw up. So I'm not marking it for stable backport. Reported-by: Andy Zlotek <[email protected]> Suggested-by: Calum Mackay <[email protected]> Signed-off-by: Chuck Lever <[email protected]>
1 parent 82078b9 commit ba21e20

File tree

1 file changed

+21
-22
lines changed

1 file changed

+21
-22
lines changed

fs/nfsd/nfs4xdr.c

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4032,6 +4032,11 @@ nfsd4_encode_open_downgrade(struct nfsd4_compoundres *resp, __be32 nfserr,
40324032
return nfsd4_encode_stateid(xdr, &od->od_stateid);
40334033
}
40344034

4035+
/*
4036+
* The operation of this function assumes that this is the only
4037+
* READ operation in the COMPOUND. If there are multiple READs,
4038+
* we use nfsd4_encode_readv().
4039+
*/
40354040
static __be32 nfsd4_encode_splice_read(
40364041
struct nfsd4_compoundres *resp,
40374042
struct nfsd4_read *read,
@@ -4042,8 +4047,12 @@ static __be32 nfsd4_encode_splice_read(
40424047
int status, space_left;
40434048
__be32 nfserr;
40444049

4045-
/* Make sure there will be room for padding if needed */
4046-
if (xdr->end - xdr->p < 1)
4050+
/*
4051+
* Make sure there is room at the end of buf->head for
4052+
* svcxdr_encode_opaque_pages() to create a tail buffer
4053+
* to XDR-pad the payload.
4054+
*/
4055+
if (xdr->iov != xdr->buf->head || xdr->end - xdr->p < 1)
40474056
return nfserr_resource;
40484057

40494058
nfserr = nfsd_splice_read(read->rd_rqstp, read->rd_fhp,
@@ -4052,38 +4061,28 @@ static __be32 nfsd4_encode_splice_read(
40524061
read->rd_length = maxcount;
40534062
if (nfserr)
40544063
goto out_err;
4064+
svcxdr_encode_opaque_pages(read->rd_rqstp, xdr, buf->pages,
4065+
buf->page_base, maxcount);
40554066
status = svc_encode_result_payload(read->rd_rqstp,
40564067
buf->head[0].iov_len, maxcount);
40574068
if (status) {
40584069
nfserr = nfserrno(status);
40594070
goto out_err;
40604071
}
40614072

4062-
buf->page_len = maxcount;
4063-
buf->len += maxcount;
4064-
xdr->page_ptr += (buf->page_base + maxcount + PAGE_SIZE - 1)
4065-
/ PAGE_SIZE;
4066-
4067-
/* Use rest of head for padding and remaining ops: */
4068-
buf->tail[0].iov_base = xdr->p;
4069-
buf->tail[0].iov_len = 0;
4070-
xdr->iov = buf->tail;
4071-
if (maxcount&3) {
4072-
int pad = 4 - (maxcount&3);
4073-
4074-
*(xdr->p++) = 0;
4075-
4076-
buf->tail[0].iov_base += maxcount&3;
4077-
buf->tail[0].iov_len = pad;
4078-
buf->len += pad;
4079-
}
4080-
4073+
/*
4074+
* Prepare to encode subsequent operations.
4075+
*
4076+
* xdr_truncate_encode() is not safe to use after a successful
4077+
* splice read has been done, so the following stream
4078+
* manipulations are open-coded.
4079+
*/
40814080
space_left = min_t(int, (void *)xdr->end - (void *)xdr->p,
40824081
buf->buflen - buf->len);
40834082
buf->buflen = buf->len + space_left;
40844083
xdr->end = (__be32 *)((void *)xdr->end + space_left);
40854084

4086-
return 0;
4085+
return nfs_ok;
40874086

40884087
out_err:
40894088
/*

0 commit comments

Comments
 (0)