Patchwork posix: Call internal free on fallback p{read,write}v on Linux (BZ#22457)

login
register
mail settings
Submitter Adhemerval Zanella Netto
Date Nov. 23, 2017, 12:06 p.m.
Message ID <1511438786-32436-1-git-send-email-adhemerval.zanella@linaro.org>
Download mbox | patch
Permalink /patch/24463/
State New
Headers show

Comments

Adhemerval Zanella Netto - Nov. 23, 2017, 12:06 p.m.
As described in BZ#22457 an interpose malloc can free an invalid
pointer for fallback preadv implementation.  Fortunately this is
just and issue on microblaze-linux-gnu running kernels older than
3.15.

This patch fixes it by calling internal free (__libc_free).  Another
possible option would be replace posix_memalign/free with a mmap/unmap,
however this is pessimization since it will two more syscalls necessary
for each preadv/pwritev call.

Checked on microblaze-linux-gnu check with run-built-tests=no and
by using the sysdeps/posix implementation on x86_64-linux-gnu (just
for sanity test where it shown no regression).

	[BZ #22457]
	* malloc/malloc.c (__libc_malloc, __libc_free, __libc_realloc,
	__libc_memalign, __libc_mallopt): Move declaration and
	libc_hidden_proto to ...
	* include/malloc.h: ... here.
	* sysdeps/posix/preadv_common.c (PREADV): Use __libc_free
	instead of free.
	* sysdeps/posix/pwritev_common.c (PWRITEV): Likewise.
---
 ChangeLog                      | 11 +++++++++++
 include/malloc.h               | 15 +++++++++++++++
 malloc/malloc.c                | 10 ----------
 sysdeps/posix/preadv_common.c  |  2 +-
 sysdeps/posix/pwritev_common.c |  2 +-
 5 files changed, 28 insertions(+), 12 deletions(-)
Florian Weimer - Nov. 23, 2017, 12:25 p.m.
On 11/23/2017 01:06 PM, Adhemerval Zanella wrote:
> As described in BZ#22457 an interpose malloc can free an invalid
> pointer for fallback preadv implementation.  Fortunately this is
> just and issue on microblaze-linux-gnu running kernels older than
> 3.15.
> 
> This patch fixes it by calling internal free (__libc_free).  Another
> possible option would be replace posix_memalign/free with a mmap/unmap,
> however this is pessimization since it will two more syscalls necessary
> for each preadv/pwritev call.

This will break static malloc interposition because some of the 
functions in malloc/malloc.o are not weak.

With an interposed malloc, the glibc malloc may not have been 
initialized, so you will have to supply your own locking here, too.

I think you should use mmap/munmap.

Thanks,
Florian
Adhemerval Zanella Netto - Nov. 23, 2017, 12:43 p.m.
On 23/11/2017 10:25, Florian Weimer wrote:
> On 11/23/2017 01:06 PM, Adhemerval Zanella wrote:
>> As described in BZ#22457 an interpose malloc can free an invalid
>> pointer for fallback preadv implementation.  Fortunately this is
>> just and issue on microblaze-linux-gnu running kernels older than
>> 3.15.
>>
>> This patch fixes it by calling internal free (__libc_free).  Another
>> possible option would be replace posix_memalign/free with a mmap/unmap,
>> however this is pessimization since it will two more syscalls necessary
>> for each preadv/pwritev call.
> 
> This will break static malloc interposition because some of the functions in malloc/malloc.o are not weak.
> 
> With an interposed malloc, the glibc malloc may not have been initialized, so you will have to supply your own locking here, too.
> 
> I think you should use mmap/munmap.

Sigh, so it basically means we can't use internal malloc symbol to
avoid interposition.  Right, mmap seems a better alternative for
this.

Patch

diff --git a/include/malloc.h b/include/malloc.h
index d4cd9a5..4b99dbc 100644
--- a/include/malloc.h
+++ b/include/malloc.h
@@ -14,5 +14,20 @@  extern __typeof (__malloc_check_init) __malloc_check_init attribute_hidden;
 struct malloc_state;
 typedef struct malloc_state *mstate;
 
+extern void * __libc_malloc (size_t) attribute_hidden;
+libc_hidden_proto (__libc_malloc)
+
+extern void  __libc_free (void*) attribute_hidden;
+libc_hidden_proto (__libc_free)
+
+extern void *  __libc_realloc (void *, size_t) attribute_hidden;
+libc_hidden_proto (__libc_realloc)
+
+extern void *  __libc_memalign (size_t, size_t) attribute_hidden;
+libc_hidden_proto (__libc_memalign)
+
+extern int __libc_mallopt (int, int) attribute_hidden;
+libc_hidden_proto (__libc_mallopt)
+
 # endif /* !_ISOMAC */
 #endif
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 2999ac4..5746193 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -500,8 +500,6 @@  void *(*__morecore)(ptrdiff_t) = __default_morecore;
   differs across systems, but is in all cases less than the maximum
   representable value of a size_t.
 */
-void*  __libc_malloc(size_t);
-libc_hidden_proto (__libc_malloc)
 
 /*
   free(void* p)
@@ -514,8 +512,6 @@  libc_hidden_proto (__libc_malloc)
   when possible, automatically trigger operations that give
   back unused memory to the system, thus reducing program footprint.
 */
-void     __libc_free(void*);
-libc_hidden_proto (__libc_free)
 
 /*
   calloc(size_t n_elements, size_t element_size);
@@ -551,8 +547,6 @@  void*  __libc_calloc(size_t, size_t);
   The old unix realloc convention of allowing the last-free'd chunk
   to be used as an argument to realloc is not supported.
 */
-void*  __libc_realloc(void*, size_t);
-libc_hidden_proto (__libc_realloc)
 
 /*
   memalign(size_t alignment, size_t n);
@@ -566,8 +560,6 @@  libc_hidden_proto (__libc_realloc)
 
   Overreliance on memalign is a sure way to fragment space.
 */
-void*  __libc_memalign(size_t, size_t);
-libc_hidden_proto (__libc_memalign)
 
 /*
   valloc(size_t n);
@@ -599,8 +591,6 @@  void*  __libc_valloc(size_t);
   M_MMAP_THRESHOLD -3         128*1024   any   (or 0 if no MMAP support)
   M_MMAP_MAX       -4         65536      any   (0 disables use of mmap)
 */
-int      __libc_mallopt(int, int);
-libc_hidden_proto (__libc_mallopt)
 
 
 /*
diff --git a/sysdeps/posix/preadv_common.c b/sysdeps/posix/preadv_common.c
index 37efdc0..c650435 100644
--- a/sysdeps/posix/preadv_common.c
+++ b/sysdeps/posix/preadv_common.c
@@ -78,6 +78,6 @@  PREADV (int fd, const struct iovec *vector, int count, OFF_T offset)
     }
 
 end:
-  free (buffer);
+  __libc_free (buffer);
   return bytes_read;
 }
diff --git a/sysdeps/posix/pwritev_common.c b/sysdeps/posix/pwritev_common.c
index 0383065..6c364f8 100644
--- a/sysdeps/posix/pwritev_common.c
+++ b/sysdeps/posix/pwritev_common.c
@@ -66,7 +66,7 @@  PWRITEV (int fd, const struct iovec *vector, int count, OFF_T offset)
 
   ssize_t ret = PWRITE (fd, buffer, bytes, offset);
 
-  free (buffer);
+  __libc_free (buffer);
 
   return ret;
 }