Linux NFS development
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: linux-rdma@vger.kernel.org, linux-nfs@vger.kernel.org
Subject: [PATCH v2 10/11] xprtrdma: Invalidate in the RPC reply handler
Date: Mon, 30 Nov 2015 17:15:18 -0500	[thread overview]
Message-ID: <20151130221518.12235.50333.stgit@manet.1015granger.net> (raw)
In-Reply-To: <20151130221349.12235.29565.stgit@manet.1015granger.net>

There is a window between the time the RPC reply handler wakes the
waiting RPC task and when xprt_release() invokes ops->buf_free.
During this time, memory regions containing the data payload may
still be accessed by a broken or malicious server, but the RPC
application has already been allowed access to the memory containing
the RPC request's data payloads.

The server should be fenced from client memory containing RPC data
payloads _before_ the RPC application is allowed to continue.

This change also more strongly enforces send queue accounting. There
is a maximum number of RPC calls allowed to be outstanding. When an
RPC/RDMA transport is set up, just enough send queue resources are
allocated to handle registration, Send, and invalidation WRs for
each those RPCs at the same time.

Before, additional RPC calls could be dispatched while invalidation
WRs were still consuming send WQEs. When invalidation WRs backed
up, dispatching additional RPCs resulted in a send queue overrun.

Now, the reply handler prevents RPC dispatch until invalidation is
complete. This prevents RPC call dispatch until there are enough
send queue resources to proceed.

Still to do: If an RPC exits early (say, ^C), the reply handler has
no opportunity to perform invalidation. Currently, xprt_rdma_free()
still frees remaining RDMA resources, which could deadlock.
Additional changes are needed to handle invalidation properly in this
case.

Reported-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/xprtrdma/rpc_rdma.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c
index 0bc8c39..3d00c5d 100644
--- a/net/sunrpc/xprtrdma/rpc_rdma.c
+++ b/net/sunrpc/xprtrdma/rpc_rdma.c
@@ -891,6 +891,16 @@ badheader:
 		break;
 	}
 
+	/* Invalidate and flush the data payloads before waking the
+	 * waiting application. This guarantees the memory region is
+	 * properly fenced from the server before the application
+	 * accesses the data. It also ensures proper send flow
+	 * control: waking the next RPC waits until this RPC has
+	 * relinquished all its Send Queue entries.
+	 */
+	if (req->rl_nchunks)
+		r_xprt->rx_ia.ri_ops->ro_unmap_sync(r_xprt, req);
+
 	credits = be32_to_cpu(headerp->rm_credit);
 	if (credits == 0)
 		credits = 1;	/* don't deadlock */


  parent reply	other threads:[~2015-11-30 22:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-30 22:13 [PATCH v2 00/11] NFS/RDMA client patches for 4.5 Chuck Lever
2015-11-30 22:14 ` [PATCH v2 01/11] xprtrdma: Fix additional uses of spin_lock_irqsave(rb_lock) Chuck Lever
2015-11-30 22:14 ` [PATCH v2 02/11] xprtrdma: xprt_rdma_free() must not release backchannel reqs Chuck Lever
2015-11-30 22:14 ` [PATCH v2 03/11] xprtrdma: Disable RPC/RDMA backchannel debugging messages Chuck Lever
2015-11-30 22:14 ` [PATCH v2 04/11] xprtrdma: Move struct ib_send_wr off the stack Chuck Lever
2015-11-30 22:14 ` [PATCH v2 05/11] xprtrdma: Introduce ro_unmap_sync method Chuck Lever
2015-11-30 22:14 ` [PATCH v2 06/11] xprtrdma: Add ro_unmap_sync method for FRWR Chuck Lever
2015-11-30 22:14 ` [PATCH v2 07/11] xprtrdma: Add ro_unmap_sync method for FMR Chuck Lever
2015-11-30 22:15 ` [PATCH v2 08/11] xprtrdma: Add ro_unmap_sync method for all-physical registration Chuck Lever
2015-11-30 22:15 ` [PATCH v2 09/11] SUNRPC: Introduce xprt_commit_rqst() Chuck Lever
2015-11-30 22:15 ` Chuck Lever [this message]
2015-11-30 22:15 ` [PATCH v2 11/11] xprtrdma: Revert commit e7104a2a9606 ('xprtrdma: Cap req_cqinit') Chuck Lever

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=20151130221518.12235.50333.stgit@manet.1015granger.net \
    --to=chuck.lever@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    /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