On 12/23/24 6:22 PM, Rick Macklem wrote:
On Mon, Dec 23, 2024 at 10:07 AM <cel@xxxxxxxxxx> wrote:
From: Chuck Lever <chuck.lever@xxxxxxxxxx>
Build out the patch series to address the longstanding bug pointed
out by J David and Rick Macklem.
At least during NFSv4 COMPOUND encoding, using
write_bytes_to_xdr_buf() seems less brittle than saving a pointer
into the XDR encoding buffer.
I have one more patch to add (not yet included) that addresses the
issue in the NFSv4 READ and READ_PLUS encoders.
It also looks like there is a similar situation in nfsd4_encode_fattr4().
It uses attrlen_p (only a 4byte xdr_reserve_space(), so safe for now.
You might just regret choosing to not wire down the "safe to use
xdr_reserve_space() for 4 bytes" semantic, but it is obviously up to you.
I'm 100% behind the idea of making it hard or impossible to code
things the wrong way.
But I'm not sure what you mean by "wire down". I can't think of a way
to enforce the "only 4 octets or less" rule -- just having a helper
function with that name documents it but doesn't enforce it.
My plan is to replace the obviously unsafe call sites immediately,
document the desired long-term semantics, then replace the other
"safe for now" call sites over time.
I've found a few other potentially unsafe server-side callers:
* gss_wrap_req_integ
* gss_wrap_req_priv
* unx_marshal
I consider these "safe for now" because the reserved space is guaranteed
to be in the XDR buffer's head iovec, far away from page boundaries.
I'm hoping that no-one introduces new unsafe call sites in the meantime.
We should be able to catch that in review.
That also buys some time to come up with something better.
rick
Changes since RFC:
- Document the guarantees around pointer returned by xdr_reserve_space()
- Use write_bytes_to_xdr_buf() instead
Chuck Lever (2):
NFSD: Encode COMPOUND operation status on page boundaries
SUNRPC: Document validity guarantees of the pointer returned by
reserve_space
fs/nfsd/nfs4xdr.c | 20 ++++++++++----------
net/sunrpc/xdr.c | 3 +++
2 files changed, 13 insertions(+), 10 deletions(-)
--
2.47.0
--
Chuck Lever
