From 014daf9684e85566cfcf44ec107c8cb792df3b14 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Fri, 31 Mar 2017 20:36:45 +0200 Subject: lib: Some fixes in CDAP --- src/lib/cdap.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/cdap.c b/src/lib/cdap.c index 824f2c5d..f0db2419 100644 --- a/src/lib/cdap.c +++ b/src/lib/cdap.c @@ -183,8 +183,8 @@ static struct cdap_rcvd * cdap_rcvd_get_by_key(struct cdap * instance, list_for_each_safe(p, h, &instance->rcvd) { rcvd = list_entry(p, struct cdap_rcvd, next); if (rcvd->key == key) { - pthread_mutex_unlock(&instance->rcvd_lock); list_del(&rcvd->next); + pthread_mutex_unlock(&instance->rcvd_lock); return rcvd; } } @@ -669,6 +669,7 @@ cdap_key_t * cdap_request_send(struct cdap * instance, pthread_rwlock_unlock(&instance->flows_lock); release_id(instance, *key); release_id(instance, iid); + *key = INVALID_CDAP_KEY; return keys; } @@ -678,13 +679,17 @@ cdap_key_t * cdap_request_send(struct cdap * instance, cdap_sent_del(instance, req); release_id(instance, *key); release_id(instance, iid); + *key = INVALID_CDAP_KEY; return keys; } if (ret < 0) { + pthread_rwlock_unlock(&instance->flows_lock); cdap_sent_del(instance, req); release_id(instance, *key); release_id(instance, iid); + *key = INVALID_CDAP_KEY; + return keys; } ++key; @@ -717,6 +722,7 @@ int cdap_reply_wait(struct cdap * instance, if (ret < 0) { cdap_sent_del(instance, r); release_id(instance, iid); + release_id(instance, key); return ret; } @@ -731,6 +737,7 @@ int cdap_reply_wait(struct cdap * instance, cdap_sent_del(instance, r); release_id(instance, iid); + release_id(instance, key); return ret; } @@ -766,6 +773,8 @@ cdap_key_t cdap_request_wait(struct cdap * instance, } } + assert(rcv->proc == false); + rcv->proc = true; list_del(&rcv->next); list_add_tail(&rcv->next, &instance->rcvd); -- cgit v1.2.3 From 8913ac0a36c068c012cd0be9591cfad63a1af44e Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Fri, 31 Mar 2017 20:38:15 +0200 Subject: lib: Fix missing assignment in flow_alloc --- src/lib/dev.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'src/lib') diff --git a/src/lib/dev.c b/src/lib/dev.c index e19083c3..5acbada2 100644 --- a/src/lib/dev.c +++ b/src/lib/dev.c @@ -555,6 +555,7 @@ int flow_alloc(const char * dst_name, ai.flows[fd].api = recv_msg->api; ai.flows[fd].cube = recv_msg->qoscube; + ai.ports[recv_msg->port_id].fd = fd; ai.ports[recv_msg->port_id].state = PORT_ID_ASSIGNED; pthread_rwlock_unlock(&ai.flows_lock); @@ -921,7 +922,7 @@ struct fqueue * fqueue_create() if (fq == NULL) return NULL; - memset(fq->fqueue, -1, SHM_BUFFER_SIZE); + memset(fq->fqueue, -1, (SHM_BUFFER_SIZE) * sizeof(*fq->fqueue)); fq->fqsize = 0; fq->next = 0; @@ -1021,11 +1022,8 @@ int fqueue_next(struct fqueue * fq) if (fq == NULL) return -EINVAL; - if (fq->next == fq->fqsize) { - fq->fqsize = 0; - fq->next = 0; + if (fq->fqsize == 0) return -EPERM; - } pthread_rwlock_rdlock(&ai.data_lock); pthread_rwlock_rdlock(&ai.flows_lock); @@ -1035,6 +1033,11 @@ int fqueue_next(struct fqueue * fq) pthread_rwlock_unlock(&ai.flows_lock); pthread_rwlock_unlock(&ai.data_lock); + if (fq->next == fq->fqsize) { + fq->fqsize = 0; + fq->next = 0; + } + return fd; } @@ -1319,6 +1322,9 @@ int ipcp_flow_read(int fd, int port_id = -1; struct shm_rbuff * rb; + assert(fd >=0); + assert(sdb); + pthread_rwlock_rdlock(&ai.data_lock); pthread_rwlock_rdlock(&ai.flows_lock); @@ -1427,6 +1433,8 @@ ssize_t local_flow_read(int fd) { ssize_t ret; + assert(fd >= 0); + pthread_rwlock_rdlock(&ai.data_lock); pthread_rwlock_rdlock(&ai.flows_lock); -- cgit v1.2.3 From 304bf4f90f58f28d6941d3e3b14bb04d48f52392 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Fri, 31 Mar 2017 22:35:51 +0200 Subject: lib: Fix use-after-free when destroying cdap_req --- src/lib/cdap_req.c | 6 +++++- src/lib/cdap_req.h | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) (limited to 'src/lib') diff --git a/src/lib/cdap_req.c b/src/lib/cdap_req.c index df748058..4eab6fa6 100644 --- a/src/lib/cdap_req.c +++ b/src/lib/cdap_req.c @@ -76,6 +76,7 @@ void cdap_req_destroy(struct cdap_req * creq) creq->state = REQ_NULL; pthread_cond_broadcast(&creq->cond); break; + case REQ_INIT_PENDING: case REQ_PENDING: case REQ_RESPONSE: creq->state = REQ_DESTROY; @@ -151,7 +152,10 @@ void cdap_req_respond(struct cdap_req * creq, pthread_mutex_lock(&creq->lock); - while (creq->state == REQ_INIT) + if (creq->state == REQ_INIT) + creq->state = REQ_INIT_PENDING; + + while (creq->state == REQ_INIT_PENDING) pthread_cond_wait(&creq->cond, &creq->lock); if (creq->state != REQ_PENDING) { diff --git a/src/lib/cdap_req.h b/src/lib/cdap_req.h index 648ebc75..b21467f3 100644 --- a/src/lib/cdap_req.h +++ b/src/lib/cdap_req.h @@ -36,6 +36,7 @@ typedef cdap_key_t invoke_id_t; enum creq_state { REQ_NULL = 0, REQ_INIT, + REQ_INIT_PENDING, REQ_PENDING, REQ_RESPONSE, REQ_DONE, -- cgit v1.2.3 From 67fcb9107ae73fd1a4ccb30e4922f0dee0bd29a5 Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Fri, 31 Mar 2017 22:41:43 +0200 Subject: lib: Fix data race in rdrbuff The blocks should be accessed inside the lock (or later with CAS). --- src/lib/shm_rdrbuff.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'src/lib') diff --git a/src/lib/shm_rdrbuff.c b/src/lib/shm_rdrbuff.c index b8d73650..9dffdf74 100644 --- a/src/lib/shm_rdrbuff.c +++ b/src/lib/shm_rdrbuff.c @@ -380,6 +380,8 @@ ssize_t shm_rdrbuff_write(struct shm_rdrbuff * rdrb, sdb->flags = SDB_VALID; sdb->idx = *rdrb->head; #ifdef SHM_RDRB_MULTI_BLOCK + sdb->blocks = blocks; + *rdrb->head = (*rdrb->head + blocks) & ((SHM_BUFFER_SIZE) - 1); #else *rdrb->head = (*rdrb->head + 1) & ((SHM_BUFFER_SIZE) - 1); @@ -389,9 +391,7 @@ ssize_t shm_rdrbuff_write(struct shm_rdrbuff * rdrb, sdb->size = size; sdb->du_head = headspace; sdb->du_tail = sdb->du_head + len; -#ifdef SHM_RDRB_MULTI_BLOCK - sdb->blocks = blocks; -#endif + memcpy(((uint8_t *) (sdb + 1)) + headspace, data, len); return sdb->idx; @@ -461,6 +461,8 @@ ssize_t shm_rdrbuff_write_b(struct shm_rdrbuff * rdrb, sdb->flags = SDB_VALID; sdb->idx = *rdrb->head; #ifdef SHM_RDRB_MULTI_BLOCK + sdb->blocks = blocks; + *rdrb->head = (*rdrb->head + blocks) & ((SHM_BUFFER_SIZE) - 1); #else *rdrb->head = (*rdrb->head + 1) & ((SHM_BUFFER_SIZE) - 1); @@ -470,12 +472,8 @@ ssize_t shm_rdrbuff_write_b(struct shm_rdrbuff * rdrb, sdb->size = size; sdb->du_head = headspace; sdb->du_tail = sdb->du_head + len; -#ifdef SHM_RDRB_MULTI_BLOCK - sdb->blocks = blocks; -#endif memcpy(((uint8_t *) (sdb + 1)) + headspace, data, len); - return sdb->idx; } -- cgit v1.2.3 From 47b6ff3333fb3fcc3f5f76459c356c79e4bb111c Mon Sep 17 00:00:00 2001 From: dimitri staessens Date: Sat, 1 Apr 2017 13:44:41 +0200 Subject: lib: Add a check if a bitmap ID is in use --- include/ouroboros/bitmap.h | 5 +++- src/lib/bitmap.c | 61 +++++++++++++++++++++++++++++---------------- src/lib/tests/bitmap_test.c | 35 +++++++++++++++++++------- 3 files changed, 69 insertions(+), 32 deletions(-) (limited to 'src/lib') diff --git a/include/ouroboros/bitmap.h b/include/ouroboros/bitmap.h index cb62312a..d6bb250b 100644 --- a/include/ouroboros/bitmap.h +++ b/include/ouroboros/bitmap.h @@ -33,7 +33,7 @@ struct bmp; struct bmp * bmp_create(size_t bits, ssize_t offset); -int bmp_destroy(struct bmp * b); +void bmp_destroy(struct bmp * b); ssize_t bmp_allocate(struct bmp * instance); @@ -43,4 +43,7 @@ int bmp_release(struct bmp * instance, bool bmp_is_id_valid(struct bmp * b, ssize_t id); +bool bmp_is_id_used(struct bmp * b, + ssize_t id); + #endif /* OUROBOROS_BITMAP_H */ diff --git a/src/lib/bitmap.c b/src/lib/bitmap.c index 93ffda77..bf9bb99d 100644 --- a/src/lib/bitmap.c +++ b/src/lib/bitmap.c @@ -38,7 +38,8 @@ #define BITS_TO_LONGS(nr) \ DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(size_t)) -static size_t find_next_zero_bit(const size_t * addr, size_t nbits) +static size_t find_next_zero_bit(const size_t * addr, + size_t nbits) { size_t tmp; size_t start = 0; @@ -65,13 +66,15 @@ static size_t find_next_zero_bit(const size_t * addr, size_t nbits) return (start * BITS_PER_LONG) + pos; } -static void bitmap_zero(size_t * dst, size_t nbits) +static void bitmap_zero(size_t * dst, + size_t nbits) { size_t len = BITS_TO_LONGS(nbits) * sizeof(size_t); memset(dst, 0, len); } -static void bitmap_clear(size_t * map, size_t start) +static void bitmap_clear(size_t * map, + size_t start) { size_t * p = map + BIT_WORD(start); size_t mask = ~(1UL << (start % (BITS_PER_LONG))); @@ -79,7 +82,8 @@ static void bitmap_clear(size_t * map, size_t start) *p &= mask; } -static void bitmap_set(size_t * map, size_t start) +static void bitmap_set(size_t * map, + size_t start) { size_t * p = map + BIT_WORD(start); size_t mask = 1UL << (start % (BITS_PER_LONG)); @@ -94,7 +98,8 @@ struct bmp { size_t * bitmap; }; -struct bmp * bmp_create(size_t bits, ssize_t offset) +struct bmp * bmp_create(size_t bits, + ssize_t offset) { struct bmp * tmp; @@ -118,20 +123,15 @@ struct bmp * bmp_create(size_t bits, ssize_t offset) return tmp; } -int bmp_destroy(struct bmp * b) +void bmp_destroy(struct bmp * b) { if (b == NULL) - return -1; + return; - if (b->bitmap == NULL) { - free(b); - return -1; - } + if (b->bitmap != NULL) + free(b->bitmap); - free(b->bitmap); free(b); - - return 0; } static ssize_t bad_id(struct bmp * b) @@ -158,7 +158,8 @@ ssize_t bmp_allocate(struct bmp * b) return id + b->offset; } -static bool is_id_valid(struct bmp * b, ssize_t id) +static bool is_id_valid(struct bmp * b, + ssize_t id) { assert(b); @@ -168,7 +169,17 @@ static bool is_id_valid(struct bmp * b, ssize_t id) return true; } -bool bmp_is_id_valid(struct bmp * b, ssize_t id) +static bool is_id_used(size_t * map, + size_t start) +{ + size_t * p = map + BIT_WORD(start); + size_t mask = 1UL << (start % (BITS_PER_LONG)); + + return (*p & mask) != 0; +} + +bool bmp_is_id_valid(struct bmp * b, + ssize_t id) { if (b == NULL) return false; @@ -176,19 +187,25 @@ bool bmp_is_id_valid(struct bmp * b, ssize_t id) return is_id_valid(b, id); } -int bmp_release(struct bmp * b, ssize_t id) +int bmp_release(struct bmp * b, + ssize_t id) { - size_t rid; - if (b == NULL) return -1; if (!is_id_valid(b, id)) return -1; - rid = id - b->offset; - - bitmap_clear(b->bitmap, rid); + bitmap_clear(b->bitmap, id - b->offset); return 0; } + +bool bmp_is_id_used(struct bmp * b, + ssize_t id) +{ + if (b == NULL) + return false; + + return is_id_used(b->bitmap, id - b->offset); +} diff --git a/src/lib/tests/bitmap_test.c b/src/lib/tests/bitmap_test.c index 7480600e..e438f217 100644 --- a/src/lib/tests/bitmap_test.c +++ b/src/lib/tests/bitmap_test.c @@ -23,6 +23,7 @@ #include "bitmap.c" #include #include +#include #define BITMAP_SIZE 200 @@ -41,40 +42,56 @@ int bitmap_test(int argc, char ** argv) srand(time(NULL)); bmp = bmp_create(bits, offset); - if (bmp == NULL) + if (bmp == NULL) { + printf("Failed to create bmp.\n"); return -1; + } - if (bmp_destroy(bmp)) - return -1; + bmp_destroy(bmp); bmp = bmp_create(bits, offset); - if (bmp == NULL) + if (bmp == NULL) { + printf("Failed to re-create bmp.\n"); return -1; + } for (i = offset; i < BITMAP_SIZE + 5 + offset; i++) { id = bmp_allocate(bmp); if (!bmp_is_id_valid(bmp, id)) continue; - if (id != i) + if (!bmp_is_id_used(bmp, id)) { + printf("ID not marked in use.\n"); + bmp_destroy(bmp); return -1; + } + + if (id != i) { + printf("Wrong ID returned.\n"); + bmp_destroy(bmp); + return -1; + } } for (i = 0; i < BITMAP_SIZE + 5; i++) { r = (ssize_t) (rand() % BITMAP_SIZE) + offset; - if (bmp_release(bmp, r)) + if (bmp_release(bmp, r)) { + printf("Failed to release ID.\n"); return -1; + } id = bmp_allocate(bmp); if (!bmp_is_id_valid(bmp, id)) continue; - if (id != r) + if (id != r) { + printf("Wrong prev ID returned.\n"); + bmp_destroy(bmp); return -1; + } } - if (bmp_destroy(bmp)) - return -1; + bmp_destroy(bmp); return 0; } -- cgit v1.2.3