linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Coddington <bcodding@redhat.com>
To: trond.myklebust@hammerspace.com, anna.schumaker@netapp.com,
	tibbs@math.uh.edu
Cc: linux-nfs@vger.kernel.org, bfields@fieldses.org,
	chuck.lever@oracle.com, km@cm4all.com
Subject: [PATCH 1/2] SUNRPC: Fix buffer handling of GSS MIC with less slack
Date: Thu, 12 Sep 2019 13:07:56 -0400	[thread overview]
Message-ID: <043d2ca649c3d81cdf0b43b149cd43069ad1c1e2.1568307763.git.bcodding@redhat.com> (raw)

The GSS Message Integrity Check data for krb5i may lie partially in the XDR
reply buffer's pages and tail.  If so, we try to copy the entire MIC into
free space in the tail.  But as the estimations of the slack space required
for authentication and verification have improved there may be less free
space in the tail to complete this copy -- see commit 2c94b8eca1a2
("SUNRPC: Use au_rslack when computing reply buffer size").  In fact, there
may only be room in the tail for a single copy of the MIC, and not part of
the MIC and then another complete copy.

The real world failure reported is that `ls` of a directory on NFS may
sometimes return -EIO, which can be traced back to xdr_buf_read_netobj()
failing to find available free space in the tail to copy the MIC.

Fix this by checking for the case of the MIC crossing the boundaries of
head, pages, and tail. If so, shift the buffer until the MIC is contained
completely within the pages or tail.  This allows the remainder of the
function to create a sub buffer that directly address the complete MIC.

Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
Cc: stable@vger.kernel.org
---
 net/sunrpc/xdr.c | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index 48c93b9e525e..6e05a9693568 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -1237,39 +1237,48 @@ xdr_encode_word(struct xdr_buf *buf, unsigned int base, u32 obj)
 EXPORT_SYMBOL_GPL(xdr_encode_word);
 
 /* If the netobj starting offset bytes from the start of xdr_buf is contained
- * entirely in the head or the tail, set object to point to it; otherwise
- * try to find space for it at the end of the tail, copy it there, and
- * set obj to point to it. */
+ * entirely in the head, pages, or tail, set object to point to it; otherwise
+ * shift the buffer until it is contained entirely within the pages or tail.
+ */
 int xdr_buf_read_netobj(struct xdr_buf *buf, struct xdr_netobj *obj, unsigned int offset)
 {
 	struct xdr_buf subbuf;
+	unsigned int len_to_boundary;
 
 	if (xdr_decode_word(buf, offset, &obj->len))
 		return -EFAULT;
-	if (xdr_buf_subsegment(buf, &subbuf, offset + 4, obj->len))
+
+	offset += 4;
+
+	/* Is the obj partially in the head? */
+	len_to_boundary = buf->head->iov_len - offset;
+	if (len_to_boundary > 0 && len_to_boundary < obj->len)
+		xdr_shift_buf(buf, len_to_boundary);
+
+	/* Is the obj partially in the pages? */
+	len_to_boundary = buf->head->iov_len + buf->page_len - offset;
+	if (len_to_boundary > 0 && len_to_boundary < obj->len)
+		xdr_shrink_pagelen(buf, len_to_boundary);
+
+	if (xdr_buf_subsegment(buf, &subbuf, offset, obj->len))
 		return -EFAULT;
 
-	/* Is the obj contained entirely in the head? */
-	obj->data = subbuf.head[0].iov_base;
-	if (subbuf.head[0].iov_len == obj->len)
-		return 0;
-	/* ..or is the obj contained entirely in the tail? */
+	/* Most likely: is the obj contained entirely in the tail? */
 	obj->data = subbuf.tail[0].iov_base;
 	if (subbuf.tail[0].iov_len == obj->len)
 		return 0;
 
-	/* use end of tail as storage for obj:
-	 * (We don't copy to the beginning because then we'd have
-	 * to worry about doing a potentially overlapping copy.
-	 * This assumes the object is at most half the length of the
-	 * tail.) */
+	/* ..or is the obj contained entirely in the head? */
+	obj->data = subbuf.head[0].iov_base;
+	if (subbuf.head[0].iov_len == obj->len)
+		return 0;
+
+	/* obj is in the pages: move to tail */
 	if (obj->len > buf->buflen - buf->len)
 		return -ENOMEM;
-	if (buf->tail[0].iov_len != 0)
-		obj->data = buf->tail[0].iov_base + buf->tail[0].iov_len;
-	else
-		obj->data = buf->head[0].iov_base + buf->head[0].iov_len;
+	obj->data = buf->head[0].iov_base + buf->head[0].iov_len;
 	__read_bytes_from_xdr_buf(&subbuf, obj->data, obj->len);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(xdr_buf_read_netobj);
-- 
2.20.1


             reply	other threads:[~2019-09-12 17:07 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-12 17:07 Benjamin Coddington [this message]
2019-09-12 17:07 ` [PATCH 2/2] SUNRPC: Rename xdr_buf_read_netobj to xdr_buf_read_mic Benjamin Coddington
2019-09-13 15:16   ` Chuck Lever
2019-09-13 17:26     ` Benjamin Coddington
2019-09-13 17:28       ` Chuck Lever
2019-09-13 14:41 ` [PATCH 1/2] SUNRPC: Fix buffer handling of GSS MIC with less slack Chuck Lever
2019-09-13 14:49   ` Benjamin Coddington
2019-09-13 16:05 ` Chuck Lever
2019-09-13 17:39   ` Benjamin Coddington
2019-09-15 14:08     ` Benjamin Coddington

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=043d2ca649c3d81cdf0b43b149cd43069ad1c1e2.1568307763.git.bcodding@redhat.com \
    --to=bcodding@redhat.com \
    --cc=anna.schumaker@netapp.com \
    --cc=bfields@fieldses.org \
    --cc=chuck.lever@oracle.com \
    --cc=km@cm4all.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=tibbs@math.uh.edu \
    --cc=trond.myklebust@hammerspace.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;
as well as URLs for NNTP newsgroup(s).