Re: [PATCH] memory leak in _reclaim_reent

[Federico Terraneo <fede dot tft at hotmail dot it>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 06/24/2013 11:36 AM, Corinna Vinschen wrote:
> 
> As far as I can see, destructing _sig_func needs synchronization.
> If you free it, a _signal_r call could come in and access the
> free'd memory. If it already has been set to NULL, _signal_r calls
> _init_signal_r and reallocates it.  So we either skip this problem
> for now, or you have to make sure that nobody accesses _sig_func
> during destruction.
> 

Considering that on a second though the problem looks even worse
(malloc isn't guaranteed to by signal-safe...), I opted for a simpler
solution.

This patch has two advantages:
- - it fixes the leak about _signal_buf and _misc.
- - it leaves a comment about the _sig_func issue being unsolved, which I
  presume is better than nothing.

newlib/ChangeLog
2013-06-24  Terraneo Federico  <fede.tft@hotmail.it>

	* libc/reent/reent.c (_reclaim_reent): Free also _misc and
	  _signal_buf.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJRyCkdAAoJECkLFtN5Xr9f6+cH/jLEkP/4Z9GWj30CZc4wBkCM
dD2Nn3yAFB9sURuR3If1KNbLCMH5AxeOT20BI5CkBbf78leh3p83AVooXtJNzzuH
ZViaJpNMUbr+VjqUA92xXibxduFWXPzhQyEbMWvGcUMjE9Ie2YCYPVpOe4A458yB
opwujHsuu8mpIbjKhgpjvSLtZaxyLZWQV3ECn52Zn0dxE/trEFkkscktAZzswjbO
C+ZIlo47DucjoLYTjdKU1Y0n1y4+0zQ+Hm26Eu4gGuMnLuufopmDU4c8ZwqShsyc
umpDzfYAmwRb6dHd7P1gKHRh22YvUceYIyjbwA3GDTTdF6MZV+yiiTSgGCD+UZI=
=kYb6
-----END PGP SIGNATURE-----

diff -ruN a/newlib/libc/reent/reent.c b/newlib/libc/reent/reent.c
--- a/newlib/libc/reent/reent.c	2013-05-16 14:29:28.000000000 +0200
+++ b/newlib/libc/reent/reent.c	2013-06-24 12:59:12.599966562 +0200
@@ -87,6 +87,10 @@
 	_free_r (ptr, ptr->_localtime_buf);
       if (ptr->_asctime_buf)
 	_free_r (ptr, ptr->_asctime_buf);
+	  if (ptr->_signal_buf)
+	_free_r (ptr, ptr->_signal_buf);
+	  if (ptr->_misc)
+	_free_r (ptr, ptr->_misc);
 #endif
 
 #ifndef _REENT_GLOBAL_ATEXIT
@@ -110,6 +114,11 @@
 
       if (ptr->_cvtbuf)
 	_free_r (ptr, ptr->_cvtbuf);
+    /* We should free _sig_func to avoid a memory leak, but how to
+	   do it safely considering that a signal may be delivered immediately
+	   after the free?
+	  if (ptr->_sig_func)
+	_free_r (ptr, ptr->_sig_func);*/
 
       if (ptr->__sdidinit)
 	{

Attachment: reclaim_reent2.patch.sig
Description: Binary data