Tarantool development patches archive
 help / color / mirror / Atom feed
From: mechanik20051988 via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: v.shpilevoy@tarantool.org, vdavydov@tarantool.org
Cc: tarantool-patches@dev.tarantool.org,
	mechanik20051988 <mechanik20.05.1988@gmail.com>
Subject: [Tarantool-patches] [PATCH 2/7] salad: fix segfault in case when mhash table allocation failure
Date: Thu,  5 Aug 2021 21:17:40 +0300	[thread overview]
Message-ID: <9283d4de4f1cfdc3a912d2730b8303ea7ff52b2b.1628184138.git.mechanik20.05.1988@gmail.com> (raw)
In-Reply-To: <cover.1628184138.git.mechanik20.05.1988@gmail.com>

From: mechanik20051988 <mechanik20.05.1988@gmail.com>

There was no check for successful memory allocation in `new` and `clear`
functions for mhash table. And if the memory was not allocated, a null
pointer dereference occured.
---
 src/lib/salad/mhash.h  | 99 +++++++++++++++++++++++++++---------------
 test/unit/mhash_body.c |  4 +-
 2 files changed, 66 insertions(+), 37 deletions(-)

diff --git a/src/lib/salad/mhash.h b/src/lib/salad/mhash.h
index b555cad4c..74235eeaa 100644
--- a/src/lib/salad/mhash.h
+++ b/src/lib/salad/mhash.h
@@ -157,7 +157,7 @@ struct _mh(t) {
 #define MH_DENSITY 0.7
 
 struct _mh(t) * _mh(new)();
-void _mh(clear)(struct _mh(t) *h);
+int _mh(clear)(struct _mh(t) *h);
 void _mh(delete)(struct _mh(t) *h);
 void _mh(resize)(struct _mh(t) *h, mh_arg_t arg);
 int _mh(start_resize)(struct _mh(t) *h, mh_int_t buckets, mh_int_t batch,
@@ -399,23 +399,50 @@ _mh(del_resize)(struct _mh(t) *h, mh_int_t x,
 struct _mh(t) *
 _mh(new)()
 {
-	struct _mh(t) *h = (struct _mh(t) *) calloc(1, sizeof(*h));
-	h->shadow = (struct _mh(t) *) calloc(1, sizeof(*h));
+	struct _mh(t) *h = (struct _mh(t) *)calloc(1, sizeof(*h));
+	if (h == NULL)
+		return NULL;
+	h->shadow = (struct _mh(t) *)calloc(1, sizeof(*h));
+	if (h->shadow == NULL)
+		goto fail;
 	h->prime = 0;
 	h->n_buckets = __ac_prime_list[h->prime];
-	h->p = (mh_node_t *) calloc(h->n_buckets, sizeof(mh_node_t));
+	h->p = (mh_node_t *)calloc(h->n_buckets, sizeof(mh_node_t));
+	if (h->p == NULL)
+		goto fail;
 #if !mh_bytemap
-	h->b = (uint32_t *) calloc(h->n_buckets / 16 + 1, sizeof(uint32_t));
+	h->b = (uint32_t *)calloc(h->n_buckets / 16 + 1, sizeof(uint32_t));
 #else
-	h->b = (uint8_t *) calloc(h->n_buckets, sizeof(uint8_t));
+	h->b = (uint8_t *)calloc(h->n_buckets, sizeof(uint8_t));
 #endif
+	if (h->b == NULL)
+		goto fail;
 	h->upper_bound = h->n_buckets * MH_DENSITY;
 	return h;
+
+fail:
+	free(h->p);
+	free(h->shadow);
+	free(h);
+	return NULL;
 }
 
-void
+int
 _mh(clear)(struct _mh(t) *h)
 {
+	mh_int_t n_buckets = __ac_prime_list[h->prime];
+	mh_node_t *p = (mh_node_t *)calloc(n_buckets, sizeof(mh_node_t));
+	if (p == NULL)
+		return -1;
+#if !mh_bytemap
+	uint32_t *b = (uint32_t *)calloc(n_buckets / 16 + 1, sizeof(uint32_t));
+#else
+	uint8_t *b = (uint8_t *)calloc(n_buckets, sizeof(uint8_t));
+#endif
+	if (b == NULL) {
+		free(p);
+		return -1;
+	}
 	if (h->shadow->p) {
 		free(h->shadow->p);
 		free(h->shadow->b);
@@ -424,15 +451,12 @@ _mh(clear)(struct _mh(t) *h)
 	free(h->p);
 	free(h->b);
 	h->prime = 0;
-	h->n_buckets = __ac_prime_list[h->prime];
-	h->p = (mh_node_t *) calloc(h->n_buckets, sizeof(mh_node_t));
-#if !mh_bytemap
-	h->b = (uint32_t *) calloc(h->n_buckets / 16 + 1, sizeof(uint32_t));
-#else
-	h->b = (uint8_t *) calloc(h->n_buckets, sizeof(uint8_t));
-#endif
+	h->n_buckets = n_buckets;
+	h->p = p;
+	h->b = b;
 	h->size = 0;
 	h->upper_bound = h->n_buckets * MH_DENSITY;
+	return 0;
 }
 
 void
@@ -515,42 +539,47 @@ _mh(start_resize)(struct _mh(t) *h, mh_int_t buckets, mh_int_t batch,
 		/* hash size is already greater than requested */
 		return 0;
 	}
-	while (h->prime < __ac_HASH_PRIME_SIZE - 1) {
-		if (__ac_prime_list[h->prime] >= buckets)
+	mh_int_t new_prime = h->prime;
+	while (new_prime < __ac_HASH_PRIME_SIZE - 1) {
+		if (__ac_prime_list[new_prime] >= buckets)
 			break;
-		h->prime += 1;
+		new_prime += 1;
 	}
-
-	h->batch = batch > 0 ? batch : h->n_buckets / (256 * 1024);
-	if (h->batch < 256) {
+	mh_int_t new_batch = batch > 0 ? batch : h->n_buckets / (256 * 1024);
+	if (new_batch < 256) {
 		/*
 		 * Minimal batch must be greater or equal to
 		 * 1 / (1 - f), where f is upper bound percent
 		 * = MH_DENSITY
 		 */
-		h->batch = 256;
+		new_batch = 256;
 	}
 
-	struct _mh(t) *s = h->shadow;
-	memcpy(s, h, sizeof(*h));
-	s->resize_position = 0;
-	s->n_buckets = __ac_prime_list[h->prime];
-	s->upper_bound = s->n_buckets * MH_DENSITY;
-	s->n_dirty = 0;
-	s->size = 0;
-	s->p = (mh_node_t *) malloc(s->n_buckets * sizeof(mh_node_t));
-	if (s->p == NULL)
+	mh_int_t n_buckets = __ac_prime_list[new_prime];
+	mh_node_t *p = (mh_node_t *)malloc(n_buckets * sizeof(mh_node_t));
+	if (p == NULL)
 		return -1;
 #if !mh_bytemap
-	s->b = (uint32_t *) calloc(s->n_buckets / 16 + 1, sizeof(uint32_t));
+	uint32_t *b = (uint32_t *)calloc(n_buckets / 16 + 1, sizeof(uint32_t));
 #else
-	s->b = (uint8_t *) calloc(s->n_buckets, sizeof(uint8_t));
+	uint8_t *b = (uint8_t *)calloc(n_buckets, sizeof(uint8_t));
 #endif
-	if (s->b == NULL) {
-		free(s->p);
-		s->p = NULL;
+	if (b == NULL) {
+		free(p);
 		return -1;
 	}
+
+	h->prime = new_prime;
+	h->batch = new_batch;
+	struct _mh(t) *s = h->shadow;
+	memcpy(s, h, sizeof(*h));
+	s->resize_position = 0;
+	s->n_buckets = n_buckets;
+	s->upper_bound = s->n_buckets * MH_DENSITY;
+	s->n_dirty = 0;
+	s->size = 0;
+	s->p = p;
+	s->b = b;
 	_mh(resize)(h, arg);
 
 	return 0;
diff --git a/test/unit/mhash_body.c b/test/unit/mhash_body.c
index 458817fb1..324c72a43 100644
--- a/test/unit/mhash_body.c
+++ b/test/unit/mhash_body.c
@@ -23,7 +23,7 @@ h = init();
 destroy(h);
 
 h = init();
-clear(h);
+fail_unless(clear(h) == 0);
 
 /* access not yet initialized hash */
 clr(9);
@@ -59,7 +59,7 @@ tst(7);
 tst(8);
 tst(9);
 
-clear(h);
+fail_unless(clear(h) == 0);
 
 /* after clear no items should exist */
 clr(1);
-- 
2.20.1


  parent reply	other threads:[~2021-08-05 18:18 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05 18:17 [Tarantool-patches] [PATCH 0/7] implement iproto streams mechanik20051988 via Tarantool-patches
2021-08-05 18:17 ` [Tarantool-patches] [PATCH 1/7] iproto: implement stream id in binary iproto protocol mechanik20051988 via Tarantool-patches
2021-08-06  8:20   ` Vladimir Davydov via Tarantool-patches
2021-08-05 18:17 ` mechanik20051988 via Tarantool-patches [this message]
2021-08-06  8:33   ` [Tarantool-patches] [PATCH 2/7] salad: fix segfault in case when mhash table allocation failure Vladimir Davydov via Tarantool-patches
2021-08-05 18:17 ` [Tarantool-patches] [PATCH 3/7] txn: detach transaction from fiber mechanik20051988 via Tarantool-patches
2021-08-06  8:51   ` Vladimir Davydov via Tarantool-patches
2021-08-05 18:17 ` [Tarantool-patches] [PATCH 4/7] iproto: implement streams in iproto mechanik20051988 via Tarantool-patches
2021-08-06 10:30   ` Vladimir Davydov via Tarantool-patches
2021-08-05 18:17 ` [Tarantool-patches] [PATCH 5/7] net.box: add stream support to net.box mechanik20051988 via Tarantool-patches
2021-08-06 12:03   ` Vladimir Davydov via Tarantool-patches
2021-08-05 18:17 ` [Tarantool-patches] [PATCH 6/7] iproto: implement interactive transactions over iproto streams mechanik20051988 via Tarantool-patches
2021-08-06 12:59   ` Vladimir Davydov via Tarantool-patches
2021-08-09 10:39     ` Vladimir Davydov via Tarantool-patches
2021-08-09 10:40       ` [Tarantool-patches] [PATCH 1/2] xrow: remove unused call_request::header Vladimir Davydov via Tarantool-patches
2021-08-09 10:40         ` [Tarantool-patches] [PATCH 2/2] iproto: clear request::header for client requests Vladimir Davydov via Tarantool-patches
2021-08-09 11:27           ` Evgeny Mekhanik via Tarantool-patches
2021-08-09 11:26         ` [Tarantool-patches] [PATCH 1/2] xrow: remove unused call_request::header Evgeny Mekhanik via Tarantool-patches
2021-08-05 18:17 ` [Tarantool-patches] [PATCH 7/7] net.box: add interactive transaction support in net.box mechanik20051988 via Tarantool-patches
2021-08-06 14:04   ` Vladimir Davydov via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9283d4de4f1cfdc3a912d2730b8303ea7ff52b2b.1628184138.git.mechanik20.05.1988@gmail.com \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=mechanik20.05.1988@gmail.com \
    --cc=mechanik20051988@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --cc=vdavydov@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 2/7] salad: fix segfault in case when mhash table allocation failure' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox