Re: bin/58887

["Christos Zoulas via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR bin/58887; it has been noted by GNATS.

From: Christos Zoulas <christos%zoulas.com@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: gnats-admin%netbsd.org@localhost,
 netbsd-bugs%netbsd.org@localhost,
 jlduran%gmail.com@localhost
Subject: Re: bin/58887
Date: Wed, 17 Dec 2025 09:27:05 -0500

 --Apple-Mail=_42EC9238-67A7-49BB-B8F3-2653272BA1D7
 Content-Type: multipart/mixed;
 	boundary="Apple-Mail=_2718C70A-4F70-4571-9B0A-968C2A9DBDF7"
 
 
 --Apple-Mail=_2718C70A-4F70-4571-9B0A-968C2A9DBDF7
 Content-Transfer-Encoding: quoted-printable
 Content-Type: text/plain;
 	charset=us-ascii
 
 The problem is that addchild(..., centry) may free centry and use =
 samename instead, and then the caller site does: last =3D centry; =
 pointing to free memory. The correct fix I think is to have addchild =
 return the proper NODE to be assigned as last, instead of leaking memory =
 and using the old node.
 
 
 --Apple-Mail=_2718C70A-4F70-4571-9B0A-968C2A9DBDF7
 Content-Disposition: attachment;
 	filename=spec.c.diff
 Content-Type: application/octet-stream;
 	name=spec.c.diff;
 	x-unix-mode=0664
 Content-Transfer-Encoding: 7bit
 
 ? mt
 ? spec.c.dif
 ? spec.c.diff
 Index: spec.c
 ===================================================================
 RCS file: /cvsroot/src/usr.sbin/mtree/spec.c,v
 retrieving revision 1.93
 diff -u -p -u -r1.93 spec.c
 --- spec.c	13 Dec 2025 18:50:58 -0000	1.93
 +++ spec.c	17 Dec 2025 14:23:35 -0000
 @@ -100,7 +100,7 @@ static	dev_t	parsedev(char *);
  static	void	replacenode(NODE *, NODE *);
  static	void	set(char *, NODE *);
  static	void	unset(char *, NODE *);
 -static	void	addchild(NODE *, NODE *);
 +static	NODE	*addchild(NODE *, NODE *);
  static	int	nodecmp(const NODE *, const NODE *);
  static	int	appendfield(FILE *, int, const char *, ...) __printflike(3, 4);
  
 @@ -239,8 +239,7 @@ noparent:		mtree_err("no parent node");
  				 * full path entry; add or replace
  				 */
  			centry->parent = pathparent;
 -			addchild(pathparent, centry);
 -			last = centry;
 +			last = addchild(pathparent, centry);
  		} else if (strcmp(centry->name, ".") == 0) {
  				/*
  				 * duplicate "." entry; always replace
 @@ -252,8 +251,7 @@ noparent:		mtree_err("no parent node");
  				 * add or replace
  				 */
  			centry->parent = last;
 -			addchild(last, centry);
 -			last = centry;
 +			last = addchild(last, centry);
  		} else {
  				/*
  				 * new relative child in parent dir
 @@ -261,8 +259,7 @@ noparent:		mtree_err("no parent node");
  				 * add or replace
  				 */
  			centry->parent = last->parent;
 -			addchild(last->parent, centry);
 -			last = centry;
 +			last = addchild(last->parent, centry);
  		}
  	}
  	return (root);
 @@ -721,7 +718,7 @@ unset(char *t, NODE *ip)
   *	a duplicate, insert it into the linked list referenced by
   *	pathparent->child.  Keep the list sorted if Sflag is set.
   */
 -static void
 +static NODE *
  addchild(NODE *pathparent, NODE *centry)
  {
  	NODE *samename;      /* node with the same name as centry */
 @@ -740,7 +737,7 @@ addchild(NODE *pathparent, NODE *centry)
  	if (cur == NULL) {
  		/* centry is pathparent's first and only child node so far */
  		pathparent->child = centry;
 -		return;
 +		return centry;
  	}
  
  	/*
 @@ -785,7 +782,7 @@ addchild(NODE *pathparent, NODE *centry)
  		replacenode(samename, centry);
  		if (samename == replacepos) {
  			/* The just-replaced node was in the correct position */
 -			return;
 +			return samename;
  		}
  		if (samename == insertpos || samename->prev == insertpos) {
  			/*
 @@ -793,7 +790,7 @@ addchild(NODE *pathparent, NODE *centry)
  			 * or just after the replaced node, but that would
  			 * be equivalent to just retaining the replaced node.
  			 */
 -			return;
 +			return samename;
  		}
  
  		/*
 @@ -833,7 +830,7 @@ addchild(NODE *pathparent, NODE *centry)
  		if (centry->next)
  			centry->next->prev = centry;
  	}
 -	return;
 +	return centry;
  }
  
  /*
 
 --Apple-Mail=_2718C70A-4F70-4571-9B0A-968C2A9DBDF7
 Content-Transfer-Encoding: 7bit
 Content-Type: text/plain;
 	charset=us-ascii
 
 
 
 christos
 --Apple-Mail=_2718C70A-4F70-4571-9B0A-968C2A9DBDF7--
 
 --Apple-Mail=_42EC9238-67A7-49BB-B8F3-2653272BA1D7
 Content-Transfer-Encoding: 7bit
 Content-Disposition: attachment;
 	filename=signature.asc
 Content-Type: application/pgp-signature;
 	name=signature.asc
 Content-Description: Message signed with OpenPGP
 
 -----BEGIN PGP SIGNATURE-----
 Comment: GPGTools - http://gpgtools.org
 
 iF0EARECAB0WIQS+BJlbqPkO0MDBdsRxESqxbLM7OgUCaUK9uQAKCRBxESqxbLM7
 OvVrAJ4/mjaklfFdWw/ghF5vXUAFVrs4LACg1rr6RD6I9iaMQSYnDdImTLNzRoQ=
 =F63A
 -----END PGP SIGNATURE-----
 
 --Apple-Mail=_42EC9238-67A7-49BB-B8F3-2653272BA1D7--