Skip to content

Commit 80ec79c

Browse files
committed
memory/patcher: updates to memory hooks
This commit fixes bugs that can cause crashes and memory corruption when the mremap hook is called. The problem occurs because of the ellipses (...) in the mremap intercept function. The ellipses cover the optional new_addr argument on Linux. This commit removes the ellipses and adds an explicit 5th argument. This commit also adds a hook for shmdt. The code only works on Linux at the moment as it needs to read /proc/self/maps to determine the size of the shared memory segment. Additionally, this commit removes the mmap hook. There is no apparent benefit for detecting mmap(..., PROT_NONE, ...) and it seems to cause problems when threads are in use. Signed-off-by: Nathan Hjelm <[email protected]>
1 parent 91bcab9 commit 80ec79c

File tree

1 file changed

+111
-20
lines changed

1 file changed

+111
-20
lines changed

opal/mca/memory/patcher/memory_patcher_component.c

Lines changed: 111 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,6 @@ opal_memory_patcher_component_t mca_memory_patcher_component = {
8383
it out) */
8484
};
8585

86-
#if OPAL_MEMORY_PATCHER_HAVE___MMAP && !OPAL_MEMORY_PATCHER_HAVE___MMAP_PROTO
87-
/* prototype for Apple's internal mmap function */
88-
void *__mmap (void *start, size_t length, int prot, int flags, int fd, off_t offset);
89-
#endif
90-
9186
#if OPAL_MEMORY_PATCHER_HAVE___SYSCALL_PROTO && OPAL_MEMORY_PATCHER_HAVE___SYSCALL
9287
/* calling __syscall is preferred on some systems when some arguments may be 64-bit. it also
9388
* has the benefit of having an off_t return type */
@@ -96,6 +91,13 @@ void *__mmap (void *start, size_t length, int prot, int flags, int fd, off_t off
9691
#define memory_patcher_syscall syscall
9792
#endif
9893

94+
#if 0
95+
96+
#if OPAL_MEMORY_PATCHER_HAVE___MMAP && !OPAL_MEMORY_PATCHER_HAVE___MMAP_PROTO
97+
/* prototype for Apple's internal mmap function */
98+
void *__mmap (void *start, size_t length, int prot, int flags, int fd, off_t offset);
99+
#endif
100+
99101
static void *(*original_mmap)(void *, size_t, int, int, int, off_t);
100102

101103
static void *intercept_mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset)
@@ -127,6 +129,8 @@ static void *intercept_mmap(void *start, size_t length, int prot, int flags, int
127129
return result;
128130
}
129131

132+
#endif
133+
130134
static int (*original_munmap) (void *, size_t);
131135

132136
static int intercept_munmap(void *start, size_t length)
@@ -148,27 +152,22 @@ static int intercept_munmap(void *start, size_t length)
148152

149153
#if defined (SYS_mremap)
150154

151-
static void *(*original_mremap) (void *, size_t, size_t, int, ...);
155+
/* on linux this function has an optional extra argument but ... can not be used here because it
156+
* causes issues when intercepting a 4-argument mremap call */
157+
static void *(*original_mremap) (void *, size_t, size_t, int, void *);
152158

153-
static void *intercept_mremap (void *start, size_t oldlen, size_t newlen, int flags, ...)
159+
static void *intercept_mremap (void *start, size_t oldlen, size_t newlen, int flags, void *new_address)
154160
{
155161
OPAL_PATCHER_BEGIN;
156-
void *result = 0;
157-
#ifdef MREMAP_FIXED
158-
va_list ap;
159-
#endif
160-
void *new_address = NULL;
161-
162-
opal_mem_hooks_release_hook (start, oldlen, false);
162+
void *result = MAP_FAILED;
163163

164-
#ifdef MREMAP_FIXED
165-
if (flags & MREMAP_FIXED) {
166-
va_start(ap, flags);
167-
new_address = va_arg(ap, void*);
164+
if (MAP_FAILED != start && oldlen > 0) {
165+
opal_mem_hooks_release_hook (start, oldlen, true);
166+
}
168167

169-
va_end(ap);
168+
if (!(flags & MREMAP_FIXED)) {
169+
new_address = NULL;
170170
}
171-
#endif
172171

173172
if (!original_mremap) {
174173
result = (void *)(intptr_t) memory_patcher_syscall (SYS_mremap, start, oldlen, newlen, flags, new_address);
@@ -261,6 +260,87 @@ static int intercept_brk (void *addr)
261260

262261
#endif
263262

263+
#if defined(SYS_shmdt) && defined(__linux__)
264+
265+
#include <stdio.h>
266+
#include <fcntl.h>
267+
#include <sys/shm.h>
268+
269+
static size_t memory_patcher_get_shm_seg_size (const void *shmaddr)
270+
{
271+
unsigned long start_addr, end_addr;
272+
char *ptr, *newline;
273+
char buffer[1024];
274+
size_t seg_size = 0;
275+
int fd;
276+
277+
seg_size = 0;
278+
279+
fd = open ("/proc/self/maps", O_RDONLY);
280+
assert (fd >= 0);
281+
282+
for (size_t read_offset = 0 ; ; ) {
283+
ssize_t nread = read(fd, buffer + read_offset, sizeof(buffer) - 1 - read_offset);
284+
if (nread <= 0) {
285+
if (errno == EINTR) {
286+
continue;
287+
}
288+
289+
break;
290+
} else {
291+
buffer[nread + read_offset] = '\0';
292+
}
293+
294+
ptr = buffer;
295+
while ( (newline = strchr(ptr, '\n')) != NULL ) {
296+
/* 00400000-0040b000 r-xp ... \n */
297+
int ret = sscanf(ptr, "%lx-%lx ", &start_addr, &end_addr);
298+
if (ret != 2) {
299+
continue;
300+
}
301+
302+
if (start_addr == (uintptr_t)shmaddr) {
303+
seg_size = end_addr - start_addr;
304+
goto out_close;
305+
}
306+
307+
newline = strchr(ptr, '\n');
308+
if (newline == NULL) {
309+
break;
310+
}
311+
312+
ptr = newline + 1;
313+
}
314+
315+
read_offset = strlen(ptr);
316+
memmove(buffer, ptr, read_offset);
317+
}
318+
319+
out_close:
320+
close(fd);
321+
return seg_size;
322+
}
323+
324+
static int (*original_shmdt) (const void *);
325+
326+
static int intercept_shmdt (const void *shmaddr)
327+
{
328+
OPAL_PATCHER_BEGIN;
329+
int result;
330+
331+
opal_mem_hooks_release_hook (shmaddr, memory_patcher_get_shm_seg_size (shmaddr), false);
332+
333+
if (original_shmdt) {
334+
result = original_shmdt (shmaddr);
335+
} else {
336+
result = memory_patcher_syscall (SYS_shmdt, shmaddr);
337+
}
338+
339+
OPAL_PATCHER_END;
340+
return result;
341+
}
342+
#endif
343+
264344
static int patcher_register (void)
265345
{
266346
mca_memory_patcher_priority = 80;
@@ -296,10 +376,14 @@ static int patcher_open (void)
296376
/* set memory hooks support level */
297377
opal_mem_hooks_set_support (OPAL_MEMORY_FREE_SUPPORT | OPAL_MEMORY_MUNMAP_SUPPORT);
298378

379+
#if 0
380+
/* NTH: the only reason to hook mmap would be to detect memory protection. this does not invalidate
381+
* any cache entries in the region. */
299382
rc = opal_patcher->patch_symbol ("mmap", (uintptr_t) intercept_mmap, (uintptr_t *) &original_mmap);
300383
if (OPAL_SUCCESS != rc) {
301384
return rc;
302385
}
386+
#endif
303387

304388
rc = opal_patcher->patch_symbol ("munmap", (uintptr_t)intercept_munmap, (uintptr_t *) &original_munmap);
305389
if (OPAL_SUCCESS != rc) {
@@ -318,6 +402,13 @@ static int patcher_open (void)
318402
return rc;
319403
}
320404

405+
#if defined(SYS_shmdt) && defined(__linux__)
406+
rc = opal_patcher->patch_symbol ("shmdt", (uintptr_t) intercept_shmdt, (uintptr_t *) &original_shmdt);
407+
if (OPAL_SUCCESS != rc) {
408+
return rc;
409+
}
410+
#endif
411+
321412
#if defined (SYS_brk)
322413
rc = opal_patcher->patch_symbol ("brk", (uintptr_t)intercept_brk, (uintptr_t *) &original_brk);
323414
#endif

0 commit comments

Comments
 (0)