lib/47747: TCP-based RPC client calls no longer terminate when connections break

[tbrehm%dspace.de@localhost]

>Number:         47747
>Category:       lib
>Synopsis:       TCP-based RPC client calls no longer terminate when 
>connections break
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Apr 17 16:05:00 +0000 2013
>Originator:     Thorsten Brehm
>Release:        NetBSD 6.0.1
>Organization:
dSPACE
>Environment:
>Description:
RPC client calls can block indefinitely when using TCP-based RPC connections. 
This happens when the network is interrupted or the remote server processes 
crashes while a RPC call is pending.
 
The problem exists since the following commit:
src/lib/libc/rpc/clnt_vc.c, version 1.18, 2012/03/13 21:13:44
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/rpc/clnt_vc.c?rev=1.18&content-type=text/x-cvsweb-markup&f=h

Version 1.17 still worked ok.

Diff:
http://cvsweb.netbsd.org/bsdweb.cgi/src/lib/libc/rpc/clnt_vc.c.diff?r1=1.17&r2=1.18&f=h

The problem is caused by the change in line 718, replacing
    return len;
with
    return (int) nread;

A few lines above, there is an error condition assigning
    len = -1;
when detecting a "premature eof" error. Before, this caused "read_vc" to return 
-1 (=len) when the error was detected. Now it returns 0 (=nread) instead. 
Eventually this results in "clnt_call" blocking indefinitely since it keeps 
calling "read_vc" trying to grab all data - rather than aborting with a 
connection error.

Obvious fix: replace
    len = -1;
with
    return (-1);
to properly report the error.

Patch against latest trunk below.
>How-To-Repeat:

>Fix:
diff -wru trunk/clnt_vc.c fix/clnt_vc.c
--- original/lib/libc/rpc/clnt_vc.c     Wed Apr 17 13:46:50 2013 UTC
+++ src/lib/libc/rpc/clnt_vc.c  Mon Mar 11 20:19:29 2013 UTC
@@ -713,8 +713,7 @@
                /* premature eof */
                ct->ct_error.re_errno = ECONNRESET;
                ct->ct_error.re_status = RPC_CANTRECV;
-               len = -1;  /* it's really an error */
-               break;
+               return (-1);  /* it's really an error */
 
        case -1:
                ct->ct_error.re_errno = errno;