Subject: bin/1822: rdist dies when verifying modified links
To: None <gnats-bugs@gnats.netbsd.org>
From: Brian C. Grayson <bgrayson@pine.ece.utexas.edu>
List: netbsd-bugs
Date: 12/07/1995 14:40:30
>Number:         1822
>Category:       bin
>Synopsis:       rdist improperly handles verifying links that are different
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    bin-bug-people (Utility Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Dec  7 15:50:01 1995
>Last-Modified:
>Originator:     Brian Grayson
>Organization:
	  Graduate Student, Electrical and Computer Engineering
	  The University of Texas at Austin
	  Office:  ENS 406       (512) 471-8011
>Release:        NetBSD-1.1<NetBSD-current source date>
>Environment:
	
System: NetBSD marvin 1.1 NetBSD 1.1 (MARVIN) #6: Wed Dec 6 15:58:17 CST 1995 chase@c3p0:/a/orac/home/orac/src/sys/arch/i386/compile/MARVIN i386


>Description:
        When one uses rdist to verify ('install -v') a remote-machine's
	directory against the local version, if both the local and remote
	directories have symbolic links, and if the two links have
	different values, the remote process ('rdist -Server')
	inadvertently(*) closes the communication socket, thus
	making the local rdist process exit immediately with an error
	message.  

	The problem also arises if the remote file is not a link (i.e., it
	is a regular file or directory).

        A simple fix is included below.

	(*) Reason for the accidental closure of the link:  
	The code in /usr/src/usr.bin/rdist/server.c:recvf() uses 'goto's,
	presumably to prevent code duplication.  However, one of the goto's
	assumes that it was reached while handling an ordinary file that
	has already been opened, with its file descriptor stored in the
	int 'f'.  This goto can be reached through a different chain of
	goto's, namely, when verifying files (rather than updating) and a
	remote link has a different value (ala readlink()) from the local
	link, in which case 'f' is uninitialized, and typically 0.  A
	close(0) is performed, which, since rdist uses stdin and stdout
	for its communication, ends communication between the two
	processes.
	
>How-To-Repeat:
	The following (13-line!  :)) uu-encoded, gzip'd tar file
	demonstrates the problem.  Extract the tar file at your home
	directory -- it will create the directory rdist.  Inside ~/rdist
	is a short README that discusses how to modify the
	'break.distfile' to work on your machine.  When rdist is run, it
	should exit with the message "lost connection".   If you have any
	other problems, mail me!
begin 644 rdist.tar.gz
M'XL("/),QS`"`W)D:7-T+G1A<@#MU\MJPD`4!N!9YRG.HI"-EYED+HO20D$+
M@MVT?8%H)Q?4B4RB$$K[[$UB+500$3II*^?;1)Q@T#__R6A?LJ(<$J>`4R4$
M$("`*MX<F1*L.>Y1`$DEDUP&2M3+`>62@"`=V!1E9`'(++%15>3FV'GS-"IT
M8M?DLM@V_YG5T6+0O(RSI?[I:S!*)>?'\V>4[_,/%&7U&UPQ08!B_L[=3Z;C
M)[B!]^'N3EA%1:FMYUV]MBMOT+^%562WF?$R4_]4RR7TMU]GS_-U=>T1],_[
M_SB^&SV,75WC9/]%<##_0TH9]K\+$P-EJN'[`Z`'];<UB09_5WT?RARJ?&,A
MS8L23+32`X#G5)L>V(V!6>65U3HS"?CM_03]^.`3_0%.B3_<_V:.N]L$GK/_
M"YEJ^]\\_W'_UVG^<9X[FO_!N?D+QCF!8)3%\30S"\S?<?Z[7=_P]_M?K\NF
M_V&(__^ZS]_1!#BC_Y_Y<ZF:_KON/N:/$$(((8000@@AA!!"Z$)]`)SD9J,`
#*```
`
end
	
>Fix:
        The following patch to /usr/src/usr.bin/rdist/server.c fixes the
	problem.  A better fix would be to rewrite recvf() (which is over
	250 lines long) into several helper functions, one for directories,
	one for links, etc., and eliminate all the goto's.  Anybody got
	some free time?  :)
*** server.c	Thu Dec  7 14:15:47 1995
--- server.c.dist	Thu Dec  7 10:21:03 1995
***************
*** 704,710 ****
  
  	cp = cmd;
  	opts = 0;
- 	f = 0;		/*  Initialize, so for links it remains 0.  */
  	while (*cp >= '0' && *cp <= '7')
  		opts = (opts << 3) | (*cp++ - '0');
  	if (*cp++ != ' ') {
--- 704,709 ----
***************
*** 918,926 ****
  		note("%s: utimes failed %s: %s\n", host, new, strerror(errno));
  
  	if (fchog(f, new, owner, group, mode) < 0) {
! badnew2:	
! 		if (f)		/*  Don't close if f hasn't been opened.  */
! 			(void) close(f);
  		(void) unlink(new);
  		return;
  	}
--- 917,923 ----
  		note("%s: utimes failed %s: %s\n", host, new, strerror(errno));
  
  	if (fchog(f, new, owner, group, mode) < 0) {
! badnew2:	(void) close(f);
  		(void) unlink(new);
  		return;
  	}
	
>Audit-Trail:
>Unformatted: