[PATCH] malloc: check tcache mem size in tcache_get_n to avoid arbitrary mem allocation

[dbgbgtf]

Hi

If you want to prove that the check in tcache_put is necessary, please write a testcase and demonstrate that my check does not prevent it.
And if your testcase cann't result in an arbitrary memory allocation caused by a corrupted linked list in tcachebin, I suggest open a separate commit to discuss that issue.
As stated in my title, this patch only focuses on preventing arbitrary memory allocation caused by linked list corruption in tcachebin.

Pardon my rudeness, but if you only speak without providing any code to support your idea, we won't make any progress, and I won't be able to understand your point of view.  
So if you really want to have a discussion here about tcache_put, at least show me your code instead of just talking.

Finally, if you're unable to provide code but still want to participate in the discussion, you could at least help by benchmarking my patch.
Wilco mentioned in earlier emails that he couldn't reproduce the performance result I observer.
So it would be helpful if you can assist in evaluting the patch's impact.

Again, I apologize if my words came across as impolite.
But if the discussion continues in this manner, we won't make any progress. I hope you'll consider my suggestion.

Thanks
dbgbgtf

Signed-off-by: dbgbgtf <dudududuMaxVer@gmail.com>
---
 malloc/malloc.c                     |  7 +--
 malloc/tst-tcache-arbitrary-alloc.c | 90 +++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 5 deletions(-)
 create mode 100644 malloc/tst-tcache-arbitrary-alloc.c

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 9d860eac9c..6ad2f212dc 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -3186,6 +3186,8 @@ tcache_get_n (size_t tc_idx, tcache_entry **ep)
 
   if (__glibc_unlikely (!aligned_OK (e)))
     malloc_printerr ("malloc(): unaligned tcache chunk detected");
+  if (__glibc_unlikely (tc_idx != csize2tidx( chunksize_nomask (mem2chunk(e)))))
+    malloc_printerr ("malloc(): tcache mem size vs request size");
 
   if (ep == &(tcache->entries[tc_idx]))
       *ep = REVEAL_PTR (e->next);
@@ -3956,11 +3958,6 @@ _int_malloc (mstate av, size_t bytes)
 		  while (tcache->counts[tc_idx] < mp_.tcache_count
 			 && (tc_victim = *fb) != NULL)
 		    {
-		      if (__glibc_unlikely (misaligned_chunk (tc_victim)))
-			malloc_printerr ("malloc(): unaligned fastbin chunk detected 3");
-		      size_t victim_tc_idx = csize2tidx (chunksize (tc_victim));
-		      if (__glibc_unlikely (tc_idx != victim_tc_idx))
-			malloc_printerr ("malloc(): chunk size mismatch in fastbin");
 		      if (SINGLE_THREAD_P)
 			*fb = REVEAL_PTR (tc_victim->fd);
 		      else
diff --git a/malloc/tst-tcache-arbitrary-alloc.c b/malloc/tst-tcache-arbitrary-alloc.c
new file mode 100644
index 0000000000..b785006984
--- /dev/null
+++ b/malloc/tst-tcache-arbitrary-alloc.c
@@ -0,0 +1,90 @@
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/cdefs.h>
+#define INTERNAL_SIZE_T size_t
+
+#define mem2chunk(p) ((void *)((char *)(p) - CHUNK_HDR_SZ))
+#define SIZE_SZ (sizeof (INTERNAL_SIZE_T))
+#define CHUNK_HDR_SZ (2 * SIZE_SZ)
+
+struct malloc_chunk
+{
+
+  INTERNAL_SIZE_T mchunk_prev_size; /* Size of previous chunk (if free).  */
+  INTERNAL_SIZE_T mchunk_size;      /* Size in bytes, including overhead. */
+
+  struct malloc_chunk *fd; /* double links -- used only if free. */
+  struct malloc_chunk *bk;
+
+  /* Only used for large blocks: pointer to next larger size.  */
+  struct malloc_chunk *fd_nextsize; /* double links -- used only if free. */
+  struct malloc_chunk *bk_nextsize;
+};
+
+typedef struct malloc_chunk *mchunkptr;
+
+void danger()
+{
+  printf("here are some danger instructions\n");
+  void *ptr[4];
+  ptr[0] = malloc (0x100);
+  ptr[1] = malloc (0x100);
+  // malloc aleast two chunk at the same size
+
+  free (ptr[1]);
+  free (ptr[0]);
+  // free the second one, and the the first one
+  // the tcachebin will be tcache->ptr[0]->ptr[1]
+
+  ptr[0] = mem2chunk (ptr[0]);
+
+  // checking if ptr[2] address is correctly aligned like 0x7fff ffff fff0
+  // if ptr[2] is not correctly aligned, use ptr[3] instead
+  // which should be a correctly aligned address
+  bool is_wrong_align = (INTERNAL_SIZE_T)&ptr[2] & 0x8;
+  if (is_wrong_align)
+    {
+      printf ("target address at: %p\n", &ptr[3]);
+      ((mchunkptr)ptr[0])->fd = (mchunkptr)(((INTERNAL_SIZE_T)ptr[0] >> 12)
+                                            ^ (INTERNAL_SIZE_T)(&ptr[3]));
+    }
+  else
+    {
+      printf ("target address at: %p\n", &ptr[2]);
+      ((mchunkptr)ptr[0])->fd = (mchunkptr)(((INTERNAL_SIZE_T)ptr[0] >> 12)
+                                            ^ (INTERNAL_SIZE_T)(&ptr[2]));
+    }
+  // if we change the ptr[0]->fd
+  // the bin will be tcache->ptr[0]->anywhere
+  // I use ptr[2] or ptr[3] address as example
+
+  ptr[1] = malloc (0x100);
+  ptr[0] = malloc (0x100);
+  // and the malloc them, take the tcachebin out
+  // to see if we have target address
+
+  printf ("ptr[0] point at: %p", ptr[0]);
+  // so we get target address now
+}
+
+void safe()
+{
+  printf("here are some safe instructions\n");
+  void *ptr[4];
+  ptr[0] = malloc(0x100);
+  ptr[1] = malloc(0x100);
+
+  free (ptr[1]);
+  free (ptr[0]);
+}
+
+int
+main ()
+{
+  safe();
+
+  danger();
+}
-- 
2.49.0