Bug 202977

Summary:	newlocale(3) double free in example
Product:	Documentation	Reporter:	piotr
Component:	man-pages	Assignee:	documentation_man-pages (documentation_man-pages)
Status:	RESOLVED CODE_FIX
Severity:	normal	CC:	mtk.manpages
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:		Subsystem:
Regression:	No	Bisected commit-id:

Description piotr 2019-03-20 19:30:14 UTC

There's an example in newlocale man page that calls freelocale before calling exit(). It looks like uselocale is taking ownership of the loc object and is freeing it during process destruction. It can be seen by running the example under valgrind. 
==8895== Invalid read of size 8
==8895==    at 0x4FB0188: _nl_locale_subfreeres (in /lib64/libc-2.27.so)
==8895==    by 0x4FAFF08: free_mem (in /lib64/libc-2.27.so)
==8895==    by 0x4FB0879: __libc_freeres (in /lib64/libc-2.27.so)
==8895==    by 0x4A2968C: _vgnU_freeres (vg_preloaded.c:77)
==8895==    by 0x4E74891: __run_exit_handlers (in /lib64/libc-2.27.so)
==8895==    by 0x4E748C9: exit (in /lib64/libc-2.27.so)
==8895==    by 0x108B08: main (tmp.c:68)
==8895==  Address 0x596a320 is 0 bytes inside a block of size 238 free'd
==8895==    at 0x4C3011B: free (vg_replace_malloc.c:530)
==8895==    by 0x108AFE: main (tmp.c:67)
==8895==  Block was alloc'd at
==8895==    at 0x4C2EEEF: malloc (vg_replace_malloc.c:299)
==8895==    by 0x4E68A5A: newlocale (in /lib64/libc-2.27.so)
==8895==    by 0x1089B3: main (tmp.c:27)

I guess that "freelocale(loc);" line should be removed from the example.

OS: Gentoo Linux
glibc version: 2.27-r6

Comment 1 piotr 2019-03-20 20:34:58 UTC

FWIW if the same test is run in a thread one will need to call freelocale. Maybe the locale should be changed before it's freed. Maybe the cleanup code should look as follows:
uselocale(LC_GLOBAL_HANDLE);
freelocale(loc);

exit(EXIT_SUCCESS);

Comment 2 Michael Kerrisk 2020-04-23 12:54:40 UTC

(In reply to piotr from comment #1)
> FWIW if the same test is run in a thread one will need to call freelocale.
> Maybe the locale should be changed before it's freed. Maybe the cleanup code
> should look as follows:
> uselocale(LC_GLOBAL_HANDLE);
> freelocale(loc);
> 
> exit(EXIT_SUCCESS);

Thanks for the report and that further info.

It looks like you are correct. Grepping the Fedora package source code, I see a number of instances of that pattern. I applied the patch below.

Closing this report now.

--- a/man3/newlocale.3
+++ b/man3/newlocale.3
@@ -366,6 +366,7 @@ main(int argc, char *argv[])
 
     /* Free the locale object */
 
+    uselocale(LC_GLOBAL_HANDLE);    /* So 'loc' is no longer in use */
     freelocale(loc);
 
     exit(EXIT_SUCCESS);