Bug 17924

Summary:	'free' should not set errno
Product:	glibc	Reporter:	Paul Eggert <eggert>
Component:	malloc	Assignee:	eggert
Status:	RESOLVED FIXED
Severity:	normal	CC:	bruno, dkumar, eblake, eggert, fweimer, gabravier
Priority:	P2	Flags:	fweimer: security-
Version:	unspecified
Target Milestone:	2.33
Host:		Target:
Build:		Last reconfirmed:
Attachments:	Test case for one of the scenarios patch 'free' to preserve errno patch 'free' to preserve errno (take 2) patch 'free' to preserve errno (take 3) patch 'free' to preserve errno (take 4) patch 'free' to preserve errno (take 5)

Description Paul Eggert 2015-02-04 00:06:46 UTC

As mentioned in <http://austingroupbugs.net/view.php?id=385>, the next major POSIX release will require 'free' to not set errno.  Rich Felker points out some scenarios when glibc 'free' might set errno; see <https://sourceware.org/ml/libc-alpha/2015-02/msg00067.html>.  These scenarios should be investigated and fixed.

Also, the glibc documentation should be updated to make it clear to users that glibc 'free' does not set errno, and that any replacements for 'free' should do the same.

Comment 1 Florian Weimer 2017-09-19 21:02:12 UTC

*** Bug 22157 has been marked as a duplicate of this bug. ***

Comment 2 Bruno Haible 2020-12-19 19:30:51 UTC

Created attachment 13064 [details]
Test case for one of the scenarios

Find attached a test case for one of the scenarios, namely when
  1. free() invokes munmap() and
  2. munmap()'s attempt to split a VMA fails due to /proc/sys/vm/max_map_count.

It fails on glibc 2.3.2, 2.23, 2.28, 2.32,
as well as on musl libc 1.1.24.

Comment 3 eggert 2020-12-19 21:05:46 UTC

Created attachment 13065 [details]
patch 'free' to preserve errno

Thanks for the test case, Bruno. Proposed glibc patch attached. It includes your test case.

Comment 4 Bruno Haible 2020-12-19 22:02:47 UTC

I believe there is also another code path that can lead to free() setting errno.

Namely, when glibc is not configured with --disable-experimental-malloc, USE_TCACHE gets defined and enables this code path:
__libc_free -> malloc.c:3147 -> MAYBE_INIT_TCACHE -> malloc.c:3033 -> tcache_init -> malloc.c:3007 -> _int_malloc -> malloc.c:3580 -> sysmalloc

Comment 5 eggert 2020-12-19 23:31:05 UTC

Created attachment 13067 [details]
patch 'free' to preserve errno (take 2)

> I believe there is also another code path that can lead to free() setting errno.


Thanks, I'm attaching a superseding patch that should cover that code path as well.

Comment 6 eggert 2020-12-21 07:15:53 UTC

Created attachment 13071 [details]
patch 'free' to preserve errno (take 3)

Here is v3 of the patch, reflecting comments from Siddhesh Poyarekar <https://sourceware.org/pipermail/libc-alpha/2020-December/120892.html> and from Carlos O'Donell <https://sourceware.org/pipermail/libc-alpha/2020-December/120901.html>. I'll also email this to libc-alpha.

Comment 7 eggert 2020-12-23 05:28:07 UTC

Created attachment 13072 [details]
patch 'free' to preserve errno (take 4)

This reflects comments from Florian Weimer <https://sourceware.org/pipermail/libc-alpha/2020-December/120911.html> by improving the test case. Also, it fixes the bug number in the commit message.

Comment 8 eggert 2020-12-24 00:28:53 UTC

Created attachment 13073 [details]
patch 'free' to preserve errno (take 5)

This reflects Adhemerval Zanella's comments <https://sourceware.org/pipermail/libc-alpha/2020-December/121063.html> by improving the test case.

Comment 9 eggert 2020-12-29 08:47:17 UTC

I installed the patch after further review by Adhemerval Zanella, which resulted in a couple of minor changes to the test case.