Skip to content

Commit ff431a4

Browse files
thomasbuildsthestinger
authored andcommitted
preserve errno in deallocate_small success path and h_free_sized
deallocate_small captured saved_errno but only restored it on the OOM fallback path. The fast path through label_slab() returned without restoring errno, violating the function's "preserves errno" contract that h_free and h_free_sized rely on. prctl(PR_SET_VMA_ANON_NAME) is only called when CONFIG_LABEL_MEMORY is non-zero (Android debuggable builds), so restore errno conditionally via if (CONFIG_LABEL_MEMORY). h_free_sized's large path called deallocate_large() directly with no save/restore, while h_free already wraps it correctly. This leaked errno via the region quarantine's mmap/munmap/madvise/prctl calls and surfaced through C++17 sized-deallocation (operator delete(void*, size_t)). Replace the valueless -DLABEL_MEMORY flag with CONFIG_LABEL_MEMORY=true/false following the project's existing CONFIG_* convention. Add it to the Makefile and config files (default false). Android.bp sets it to true for debuggable builds via product_variables. Use if (CONFIG_LABEL_MEMORY) in the code to avoid preprocessor use where plain C suffices.
1 parent a6a798d commit ff431a4

7 files changed

Lines changed: 22 additions & 8 deletions

File tree

Android.bp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ cc_library {
6969
},
7070
product_variables: {
7171
debuggable: {
72-
cflags: ["-DLABEL_MEMORY"],
72+
cflags: ["-DCONFIG_LABEL_MEMORY=true"],
7373
},
7474
device_has_arm_mte: {
7575
cflags: ["-DHAS_ARM_MTE", "-march=armv8-a+dotprod+memtag"]

Makefile

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ ifeq (,$(filter $(CONFIG_SELF_INIT),true false))
8989
$(error CONFIG_SELF_INIT must be true or false)
9090
endif
9191

92+
ifeq (,$(filter $(CONFIG_LABEL_MEMORY),true false))
93+
$(error CONFIG_LABEL_MEMORY must be true or false)
94+
endif
95+
9296
CPPFLAGS += \
9397
-DCONFIG_SEAL_METADATA=$(CONFIG_SEAL_METADATA) \
9498
-DZERO_ON_FREE=$(CONFIG_ZERO_ON_FREE) \
@@ -108,7 +112,8 @@ CPPFLAGS += \
108112
-DCONFIG_CLASS_REGION_SIZE=$(CONFIG_CLASS_REGION_SIZE) \
109113
-DN_ARENA=$(CONFIG_N_ARENA) \
110114
-DCONFIG_STATS=$(CONFIG_STATS) \
111-
-DCONFIG_SELF_INIT=$(CONFIG_SELF_INIT)
115+
-DCONFIG_SELF_INIT=$(CONFIG_SELF_INIT) \
116+
-DCONFIG_LABEL_MEMORY=$(CONFIG_LABEL_MEMORY)
112117

113118
$(OUT)/libhardened_malloc$(SUFFIX).so: $(OBJECTS) | $(OUT)
114119
$(CC) $(CFLAGS) $(LDFLAGS) -shared $^ $(LDLIBS) -o $@

config/default.mk

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@ CONFIG_CLASS_REGION_SIZE := 34359738368 # 32GiB
2121
CONFIG_N_ARENA := 4
2222
CONFIG_STATS := false
2323
CONFIG_SELF_INIT := true
24+
CONFIG_LABEL_MEMORY := false

config/light.mk

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@ CONFIG_CLASS_REGION_SIZE := 34359738368 # 32GiB
2121
CONFIG_N_ARENA := 4
2222
CONFIG_STATS := false
2323
CONFIG_SELF_INIT := true
24+
CONFIG_LABEL_MEMORY := false

h_malloc.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,10 @@ static inline void deallocate_small(void *p, const size_t *expected_size) {
928928
stats_slab_deallocate(c, slab_size);
929929
enqueue_free_slab(c, metadata);
930930
mutex_unlock(&c->lock);
931+
if (CONFIG_LABEL_MEMORY) {
932+
// label_slab -> prctl(PR_SET_VMA_ANON_NAME) can clobber errno
933+
errno = saved_errno;
934+
}
931935
return;
932936
}
933937
memory_purge(slab, slab_size);
@@ -1751,7 +1755,9 @@ EXPORT void h_free_sized(void *p, size_t expected_size) {
17511755
return;
17521756
}
17531757

1758+
int saved_errno = errno;
17541759
deallocate_large(p, &expected_size);
1760+
errno = saved_errno;
17551761

17561762
thread_seal_metadata();
17571763
}

memory.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@
22

33
#include <sys/mman.h>
44

5-
#ifdef LABEL_MEMORY
65
#include <sys/prctl.h>
7-
#endif
86

97
#ifndef PR_SET_VMA
108
#define PR_SET_VMA 0x53564d41
@@ -120,9 +118,8 @@ bool memory_purge(void *ptr, size_t size) {
120118
}
121119

122120
bool memory_set_name(UNUSED void *ptr, UNUSED size_t size, UNUSED const char *name) {
123-
#ifdef LABEL_MEMORY
124-
return prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, ptr, size, name);
125-
#else
121+
if (CONFIG_LABEL_MEMORY) {
122+
return prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, ptr, size, name);
123+
}
126124
return false;
127-
#endif
128125
}

memory.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
#define HAVE_COMPATIBLE_MREMAP
99
#endif
1010

11+
#ifndef CONFIG_LABEL_MEMORY
12+
#define CONFIG_LABEL_MEMORY false
13+
#endif
14+
1115
int get_metadata_key(void);
1216

1317
void *memory_map(size_t size);

0 commit comments

Comments
 (0)