public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: cel@kernel.org
To: Neil Brown <neilb@suse.de>, Jeff Layton <jlayton@kernel.org>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <dai.ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: <linux-nfs@vger.kernel.org>,
	Rick Macklem <rick.macklem@gmail.com>,
	j.david.lists@gmail.com, Chuck Lever <chuck.lever@oracle.com>
Subject: [PATCH v4 9/9] SUNRPC: Document validity guarantees of the pointer returned by reserve_space
Date: Mon, 30 Dec 2024 19:29:00 -0500	[thread overview]
Message-ID: <20241231002901.12725-10-cel@kernel.org> (raw)
In-Reply-To: <20241231002901.12725-1-cel@kernel.org>

From: Chuck Lever <chuck.lever@oracle.com>

A subtlety of this API is that if the @nbytes region traverses a
page boundary, the next __xdr_commit_encode will shift the data item
in the XDR encode buffer. This makes the returned pointer point to
something else, leading to unexpected behavior.

There are a few cases where the caller saves the returned pointer
and then later uses it to insert a computed value into an earlier
part of the stream. This can be safe only if either:

 - the data item is guaranteed to be in the XDR buffer's head, and
   thus is not ever going to be near a page boundary, or
 - the data item is no larger than 4 octets, since XDR alignment
   rules require all data items to start on 4-octet boundaries

But that safety is only an artifact of the current implementation.
It would be less brittle if these "safe" uses were eventually
replaced.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/xdr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index 62e07c330a66..4e003cb516fe 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -1097,6 +1097,12 @@ static noinline __be32 *xdr_get_next_encode_buffer(struct xdr_stream *xdr,
  * Checks that we have enough buffer space to encode 'nbytes' more
  * bytes of data. If so, update the total xdr_buf length, and
  * adjust the length of the current kvec.
+ *
+ * The returned pointer is valid only until the next call to
+ * xdr_reserve_space() or xdr_commit_encode() on @xdr. The current
+ * implementation of this API guarantees that space reserved for a
+ * four-byte data item remains valid until @xdr is destroyed, but
+ * that might not always be true in the future.
  */
 __be32 * xdr_reserve_space(struct xdr_stream *xdr, size_t nbytes)
 {
-- 
2.47.0


  parent reply	other threads:[~2024-12-31  0:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-31  0:28 [PATCH v4 0/9] Fix XDR encoding near page boundaries cel
2024-12-31  0:28 ` [PATCH v4 1/9] NFSD: Encode COMPOUND operation status on " cel
2024-12-31  0:28 ` [PATCH v4 2/9] NFSD: Insulate nfsd4_encode_read() from page boundaries in the encode buffer cel
2024-12-31  0:28 ` [PATCH v4 3/9] NFSD: Insulate nfsd4_encode_read_plus() " cel
2024-12-31  0:28 ` [PATCH v4 4/9] NFSD: Insulate nfsd4_encode_read_plus_data() " cel
2024-12-31  0:28 ` [PATCH v4 5/9] NFSD: Insulate nfsd4_encode_fattr4() " cel
2024-12-31  0:28 ` [PATCH v4 6/9] NFSD: Insulate nfsd4_encode_readlink() " cel
2024-12-31  0:28 ` [PATCH v4 7/9] NFSD: Refactor nfsd4_do_encode_secinfo() again cel
2024-12-31  0:28 ` [PATCH v4 8/9] NFSD: Insulate nfsd4_encode_secinfo() from page boundaries in the encode buffer cel
2024-12-31  0:29 ` cel [this message]
2025-01-01 21:49   ` [PATCH v4 9/9] SUNRPC: Document validity guarantees of the pointer returned by reserve_space NeilBrown
2025-01-01 23:09     ` Chuck Lever
2025-01-02 13:21   ` Jeff Layton
2025-01-02 13:23 ` [PATCH v4 0/9] Fix XDR encoding near page boundaries Jeff Layton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241231002901.12725-10-cel@kernel.org \
    --to=cel@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=dai.ngo@oracle.com \
    --cc=j.david.lists@gmail.com \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=okorniev@redhat.com \
    --cc=rick.macklem@gmail.com \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox