On 02/13/2015 05:43 PM, Trond Myklebust wrote: > On Thu, 2015-02-12 at 10:14 -0500, Chuck Lever wrote: >> Dan Carpenter's static checker pointed out: >> >> net/sunrpc/xprtrdma/rpc_rdma.c:879 rpcrdma_reply_handler() >> warn: can 'credits' be negative? >> >> "credits" is defined as an int. The credits value comes from the >> server as a 32-bit unsigned integer. >> >> A malicious or broken server can plant a large unsigned integer in >> that field which would result in an underflow in the following >> logic, potentially triggering a deadlock of the mount point by >> blocking the client from issuing more RPC requests. >> >> net/sunrpc/xprtrdma/rpc_rdma.c: >> >> 876 credits = be32_to_cpu(headerp->rm_credit); >> 877 if (credits == 0) >> 878 credits = 1; /* don't deadlock */ >> 879 else if (credits > r_xprt->rx_buf.rb_max_requests) >> 880 credits = r_xprt->rx_buf.rb_max_requests; >> 881 >> 882 cwnd = xprt->cwnd; >> 883 xprt->cwnd = credits << RPC_CWNDSHIFT; >> 884 if (xprt->cwnd > cwnd) >> 885 xprt_release_rqst_cong(rqst->rq_task); >> >> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> >> Fixes: eba8ff660b2d ("xprtrdma: Move credit update to RPC . . .") >> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx> >> --- >> net/sunrpc/xprtrdma/rpc_rdma.c | 3 ++- >> net/sunrpc/xprtrdma/xprt_rdma.h | 2 +- >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/net/sunrpc/xprtrdma/rpc_rdma.c b/net/sunrpc/xprtrdma/rpc_rdma.c >> index 7e9acd9..91ffde8 100644 >> --- a/net/sunrpc/xprtrdma/rpc_rdma.c >> +++ b/net/sunrpc/xprtrdma/rpc_rdma.c >> @@ -738,8 +738,9 @@ rpcrdma_reply_handler(struct rpcrdma_rep *rep) >> struct rpc_xprt *xprt = rep->rr_xprt; >> struct rpcrdma_xprt *r_xprt = rpcx_to_rdmax(xprt); >> __be32 *iptr; >> - int credits, rdmalen, status; >> + int rdmalen, status; >> unsigned long cwnd; >> + u32 credits; >> >> /* Check status. If bad, signal disconnect and return rep to pool */ >> if (rep->rr_len == ~0U) { >> diff --git a/net/sunrpc/xprtrdma/xprt_rdma.h b/net/sunrpc/xprtrdma/xprt_rdma.h >> index d1b7039..0a16fb6 100644 >> --- a/net/sunrpc/xprtrdma/xprt_rdma.h >> +++ b/net/sunrpc/xprtrdma/xprt_rdma.h >> @@ -285,7 +285,7 @@ rpcr_to_rdmar(struct rpc_rqst *rqst) >> */ >> struct rpcrdma_buffer { >> spinlock_t rb_lock; /* protects indexes */ >> - int rb_max_requests;/* client max requests */ >> + u32 rb_max_requests;/* client max requests */ >> struct list_head rb_mws; /* optional memory windows/fmrs/frmrs */ >> struct list_head rb_all; >> int rb_send_index; >> > > Anna, do you want me to take this one directly, or are you preparing a > pull request? I'll make a pull request. Thanks, Anna > > Cheers > Trond > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html