>Number:         17390
>Category:       kern
>Synopsis:       error handling in getpeername(2) is sub-optimal....
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Tue Jun 25 11:54:10 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     Greg A. Woods
>Release:        NetBSD-current 2002/06/23
>Organization:
Planix, Inc.; Toronto, Ontario; Canada
>Environment:
System: NetBSD
>Description:

	Due to the way a peer TCP reset is handled and the protocol
	block is freed by this event and lot later when any socket open
	to the connection is closed, getpeername() can fail while an
	application still has a 'valid' socket.  This can result in
	severely confusing error messages from a server that accepts a
	connection (without keeping track of the peer address returned
	by accept()) and then tries to use getpeername() at some later
	point, eg. when reporting a read() error.

	The attached patch doesn't fix this, but it seems to be a
	necessary prerequisite change to try to centralise some of the
	error handling....  Note that this change has not yet been
	extensively tested, but so far it seems to be functioning --
	what I'm concerned about though is that some other PRU_* request
	might now return an "unexpected" error (though in reality this
	would probably be more of a documentation bug than a problem
	with this change per se).

	The real fix to my original problem is probably to copy the peer
	address up from the Internet PCB to the transport specific PCB,
	and to make sure to keep the latter around until the socket is
	closed.  Somewhere I saw discussion about this, either in the
	FreeBSD change logs or code, or perhaps in Stevens' TCP/IP
	Illust. Vol. 2

	Related to all this is the "messy" way of using mbufs to pass
	around struct sockaddr_in's, such as in the badly named
	in_set*addr() functions.  FreeBSD's cleanup in this area seems
	to be quite desirable....

>How-To-Repeat:

	desire to get results from getpeername() even after peer reset!

>Fix:

	also note the DIAGNOSTIC panic() below....


Index: uipc_syscalls.c
===================================================================
RCS file: /cvs/master/m-NetBSD/main/syssrc/sys/kern/uipc_syscalls.c,v
retrieving revision 1.68
diff -c -c -r1.68 uipc_syscalls.c
*** uipc_syscalls.c	11 Feb 2002 18:11:43 -0000	1.68
--- uipc_syscalls.c	25 Jun 2002 17:58:08 -0000
***************
*** 975,984 ****
  	if ((error = getsock(p->p_fd, SCARG(uap, fdes), &fp)) != 0)
  		return (error);
  	so = (struct socket *)fp->f_data;
- 	if ((so->so_state & (SS_ISCONNECTED|SS_ISCONFIRMING)) == 0) {
- 		error = ENOTCONN;
- 		goto out;
- 	}
  	error = copyin((caddr_t)SCARG(uap, alen), (caddr_t)&len, sizeof(len));
  	if (error)
  		goto out;
--- 975,980 ----
Index: tcp_usrreq.c
===================================================================
RCS file: /cvs/master/m-NetBSD/main/syssrc/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.70
diff -c -c -r1.70 tcp_usrreq.c
*** tcp_usrreq.c	11 Mar 2002 10:06:12 -0000	1.70
--- tcp_usrreq.c	25 Jun 2002 17:56:42 -0000
***************
*** 264,276 ****
  	if ((inp == 0 && in6p == 0) && req != PRU_ATTACH)
  #endif
  	{
! 		error = EINVAL;
  		goto release;
  	}
  #ifdef INET
  	if (inp) {
  		tp = intotcpcb(inp);
  		/* WHAT IF TP IS 0? */
  #ifdef KPROF
  		tcp_acounts[tp->t_state][req]++;
  #endif
--- 264,292 ----
  	if ((inp == 0 && in6p == 0) && req != PRU_ATTACH)
  #endif
  	{
! 		/*
! 		 * FreeBSD-4.x unconditionally returns ECONNRESET if the PCB is
! 		 * gone, but that seems too cavalier.  If the reason the PCB is
! 		 * gone is because our peer reset the connection then so_error
! 		 * will have been set to ECONNRESET.  Can it ever be set to
! 		 * anything else in this case?  Otherwise we've not connected
! 		 * yet so ENOTCONN seems to be the right thing to return (and
! 		 * this will mimic the check that was used previously in
! 		 * ../kern/uipc_syscalls.c:sys_getpeername().
! 		 */
! 		if (so->so_error != 0)
! 			error = so->so_error;
! 		else if ((so->so_state & (SS_ISCONNECTED|SS_ISCONFIRMING)) == 0)
! 			error = ENOTCONN;
  		goto release;
  	}
  #ifdef INET
  	if (inp) {
  		tp = intotcpcb(inp);
  		/* WHAT IF TP IS 0? */
+ # ifdef DIAGNOSTIC
+ 		panic("tcp_usrreq: intotcpcp() found NULL struct tcpcb in per-proto PCB");
+ # endif
  #ifdef KPROF
  		tcp_acounts[tp->t_state][req]++;
  #endif
>Release-Note:
>Audit-Trail:
>Unformatted: