Re: [PATCH 1/3] SUNRPC: remove printk when back channel request not found

[Jeff Layton <jlayton@kernel.org>]

On Fri, 2023-12-15 at 11:15 -0800, Dai Ngo wrote:
> If the client interface is down, or there is a network partition between
> the client and server, that prevents the callback request to reach the
> client TCP on the server will keep re-transmitting the callback for about
> ~9 minutes before giving up and closes the connection.
> 
> If the connection between the client and the server is re-established
> before the connection is closed and after the callback timed out (9 secs)
> then the re-transmitted callback request will arrive at the client. When
> the server receives the reply of the callback, receive_cb_reply prints the
> "Got unrecognized reply..." message in the system log since the callback
> request was already removed from the server xprt's recv_queue.
> 
> Even though this scenario has no effect on the server operation, a
> malicious client can take advantage of this behavior and send thousand
> of callback replies with random XIDs to fill up the server's system log.
> 
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  net/sunrpc/svcsock.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 998687421fa6..3e89dc0afbef 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1060,7 +1060,7 @@ static int receive_cb_reply(struct svc_sock *svsk, struct svc_rqst *rqstp)
>  	spin_lock(&bc_xprt->queue_lock);
>  	req = xprt_lookup_rqst(bc_xprt, xid);
>  	if (!req)
> -		goto unlock_notfound;
> +		goto unlock_eagain;
>  
>  	memcpy(&req->rq_private_buf, &req->rq_rcv_buf, sizeof(struct xdr_buf));
>  	/*
> @@ -1077,12 +1077,6 @@ static int receive_cb_reply(struct svc_sock *svsk, struct svc_rqst *rqstp)
>  	rqstp->rq_arg.len = 0;
>  	spin_unlock(&bc_xprt->queue_lock);
>  	return 0;
> -unlock_notfound:
> -	printk(KERN_NOTICE
> -		"%s: Got unrecognized reply: "
> -		"calldir 0x%x xpt_bc_xprt %p xid %08x\n",
> -		__func__, ntohl(calldir),
> -		bc_xprt, ntohl(xid));
>  unlock_eagain:
>  	spin_unlock(&bc_xprt->queue_lock);
>  	return -EAGAIN;

Makes sense. It's a cryptic error message for most admins.

Reviewed-by: Jeff Layton <jlayton@kernel.org>