On Feb 10, 2015, at 4:45 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> Hello Chuck Lever,
>
> The patch eba8ff660b2d: "xprtrdma: Move credit update to RPC reply
> handler" from Jan 21, 2015, leads to the following static checker
> warning:
>
> net/sunrpc/xprtrdma/rpc_rdma.c:879 rpcrdma_reply_handler()
> warn: can 'credits' be negative?
“credits” cannot be negative unless there’s a bug.
I can send an additional patch once this is committed (so the correct
commit ID is available for the Fixes: tag).
> net/sunrpc/xprtrdma/rpc_rdma.c
> 875
> 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;
>
>
> "credits" and "r_xprt->rx_buf.rb_max_requests" are both type int so this
> test can underflow. If "credits" is less than zero then "xprt->cwnd"
> could be zero, leading to a deadlock (based on the comment), or
> something else out of bounds.
>
> 881
> 882 cwnd = xprt->cwnd;
> 883 xprt->cwnd = credits << RPC_CWNDSHIFT;
> 884 if (xprt->cwnd > cwnd)
> 885 xprt_release_rqst_cong(rqst->rq_task);
> 886
> 887 dprintk("RPC: %s: xprt_complete_rqst(0x%p, 0x%p, %d)\n",
> 888 __func__, xprt, rqst, status);
> 889 xprt_complete_rqst(rqst->rq_task, status);
> 890 spin_unlock(&xprt->transport_lock);
> 891 }
>
> regards,
> dan carpenter
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
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
