Re: [PATCH] rpc.statd: Avoid passing unregistered socket to svc_getreqset.

[Steve Dickson <SteveD@redhat.com>]

On 02/19/2015 11:16 AM, Carlos O'Donell wrote:
> rpc.statd may crash if it receives both a notification reply and a client
> connection at the same time. It crashes because it adds sockfd to SVC_FDSET
> and that violates the API contract. The SVC_FDSET is to be considered read-only
> and must not be modified by user code. The daemon modifies it for expediency
> to avoid having to maintain two distinct fd lists and select on each one.
> It is a practical choice that makes sense.
> 
> Thus, if a notification reply arrives by itself everything works, or if a
> client connection arrives by itself everything works. Both must arrive at
> the same time for sockfd to be set in SVC_FDSET and to be processed by
> svc_getreqset because more than one of readfds is ready.
> 
> It is the processing by svc_getreqset that will crash when it finds an
> unregistered fd in the list that doesn't correlate to any of the internal
> book keeping done by the library. At present the glibc SunRPC library will
> crash, but TIRPC does not (it is robust against invalid API usage in this
> case). However, future RPC libraries may be implemented differently, and
> the questionable API usage should be fixed.
> 
> The simplest fix is for process_reply to *clear* sockfd from the
> ready-to-read fds, since it was never registered with xprt_register.
> This works because the code always calls process_reply before handing the 
> fd set to the RPC layer for processing.
> 
> Compile-tested on x86_64 against master.
> 
> Signed-off-by: Carlos O'Donell <carlos@redhat.com>
Committed!

steved.

> ---
>  rmtcall.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/utils/statd/rmtcall.c b/utils/statd/rmtcall.c
> index fd576d9..66a6eeb 100644
> --- a/utils/statd/rmtcall.c
> +++ b/utils/statd/rmtcall.c
> @@ -221,6 +221,9 @@ process_reply(FD_SET_TYPE *rfds)
>  	if (sockfd == -1 || !FD_ISSET(sockfd, rfds))
>  		return 0;
>  
> +	/* Should not be processed again. */
> +	FD_CLR (sockfd, rfds);
> +
>  	if (!(lp = recv_rply(&port)))
>  		return 1;
>  
>