Re: bin/54222: mount_portal(8) invalid free() after src/sbin/mount_portal/puffs_portal.c,-r1.9

[Leonardo Taccari <leot%NetBSD.org@localhost>]

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

From: Leonardo Taccari <leot%NetBSD.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: bin/54222: mount_portal(8) invalid free() after src/sbin/mount_portal/puffs_portal.c,-r1.9
Date: Thu, 23 May 2019 12:04:16 +0200

 Hello Robert,
 
 Robert Elz writes:
 >  [...]
 >  In that case, I'd add a new func which does the same as
 >  portal_node_reclaim() without the two free() calls, and
 >  call that new func where portal_node_reclaim() is currently
 >  explicitly called.
 >  
 >  Either that or simply inline the two relevant lines in place
 >  of the portal_node_reclaim() call - we already know fd is valid
 >  so no need for an extra test.
 >  [...]
 
 Thank you!  I can confirm that by avoiding calling portal_node_reclaim()
 in portal_node_getattr() and inlining relevant lines instead fixes
 the problem reported (for completeness patch attached).
 
 However, the results can be a bit surprising.  At least in the `cp'
 example (that was also mentioned in the -r1.9 commit message) I
 would expect the file copied containing `foo\n' but the resulting
 file is just an empty file:
 
  % cat /tmp/m/p/e/foo
  foo
  % head -1 /tmp/m/p/e/foo
  % cp /tmp/m/p/e/foo /tmp/
  % cat /tmp/foo
  %
 
 (I think that also `cp' usages described in examples/advanced.1
 and examples/cvs.1 does not do what is probably expected (copying
 real file fetching from ftp:// or CVS.)
 
 
 Thank you again!
 
 
 Index: puffs_portal.c
 ===================================================================
 RCS file: /cvsroot/src/sbin/mount_portal/puffs_portal.c,v
 retrieving revision 1.9
 diff -u -p -r1.9 puffs_portal.c
 --- puffs_portal.c	10 May 2017 16:35:18 -0000	1.9
 +++ puffs_portal.c	23 May 2019 10:03:10 -0000
 @@ -590,7 +590,8 @@ portal_node_getattr(struct puffs_usermou
  		va->va_ctime = st.st_ctim;
  		va->va_mtime = st.st_mtim;
  		va->va_birthtime = st.st_birthtim;
 -		portal_node_reclaim(pu, opc);
 +		puffs_framev_removefd(pu, portn->fd, 0);
 +		close(portn->fd);
  	}
  
  	return 0;