diff options
author | David Barksdale <amatus@amat.us> | 2017-12-18 18:39:03 -0600 |
---|---|---|
committer | David Barksdale <amatus@amat.us> | 2017-12-18 18:39:03 -0600 |
commit | 012ff13acc0cb2f5d7210aa48819395fecf12a3d (patch) | |
tree | b602b34b4e05fee76f1c67ac369322b6f729ad82 /src | |
parent | 2492acd4208bf14bf7747d235cfb9971f2cfbf99 (diff) | |
download | gnunet-012ff13acc0cb2f5d7210aa48819395fecf12a3d.tar.gz gnunet-012ff13acc0cb2f5d7210aa48819395fecf12a3d.zip |
Fix use-after-free in loop over modified list
This loop saved the next pointer which is OK if the current element is
being freed, but the callback can cause other elements to be freed which
was detected by ASAN.
Diffstat (limited to 'src')
-rw-r--r-- | src/cadet/gnunet-service-cadet_peer.c | 51 |
1 files changed, 34 insertions, 17 deletions
diff --git a/src/cadet/gnunet-service-cadet_peer.c b/src/cadet/gnunet-service-cadet_peer.c index 71c7c67d0..c4e2c0ccf 100644 --- a/src/cadet/gnunet-service-cadet_peer.c +++ b/src/cadet/gnunet-service-cadet_peer.c | |||
@@ -532,32 +532,49 @@ GCP_set_mq (struct CadetPeer *cp, | |||
532 | GCP_2s (cp), | 532 | GCP_2s (cp), |
533 | mq); | 533 | mq); |
534 | cp->core_mq = mq; | 534 | cp->core_mq = mq; |
535 | for (struct GCP_MessageQueueManager *mqm = cp->mqm_head, *next; | 535 | /* Since these callbacks can remove any items from this list, we must take a |
536 | * snapshot and then test each one to see if it's still in the list. */ | ||
537 | int count = 0; | ||
538 | for (struct GCP_MessageQueueManager *mqm = cp->mqm_head; | ||
536 | NULL != mqm; | 539 | NULL != mqm; |
537 | mqm = next) | 540 | mqm = mqm->next) |
541 | ++count; | ||
542 | struct GCP_MessageQueueManager *mqms[count]; | ||
543 | int i = 0; | ||
544 | for (struct GCP_MessageQueueManager *mqm = cp->mqm_head; | ||
545 | NULL != mqm; | ||
546 | mqm = mqm->next) | ||
547 | mqms[i++] = mqm; | ||
548 | for (i = 0; i < count; ++i) | ||
538 | { | 549 | { |
539 | /* Save next pointer in case mqm gets freed by the callback */ | 550 | for (struct GCP_MessageQueueManager *mqm = cp->mqm_head; |
540 | next = mqm->next; | 551 | NULL != mqm; |
541 | if (NULL == mq) | 552 | mqm = mqm->next) |
542 | { | 553 | { |
543 | if (NULL != mqm->env) | 554 | if (mqms[i] != mqm) |
555 | continue; | ||
556 | if (NULL == mq) | ||
544 | { | 557 | { |
545 | GNUNET_MQ_discard (mqm->env); | 558 | if (NULL != mqm->env) |
546 | mqm->env = NULL; | 559 | { |
547 | mqm->cb (mqm->cb_cls, | 560 | GNUNET_MQ_discard (mqm->env); |
548 | GNUNET_SYSERR); | 561 | mqm->env = NULL; |
562 | mqm->cb (mqm->cb_cls, | ||
563 | GNUNET_SYSERR); | ||
564 | } | ||
565 | else | ||
566 | { | ||
567 | mqm->cb (mqm->cb_cls, | ||
568 | GNUNET_NO); | ||
569 | } | ||
549 | } | 570 | } |
550 | else | 571 | else |
551 | { | 572 | { |
573 | GNUNET_assert (NULL == mqm->env); | ||
552 | mqm->cb (mqm->cb_cls, | 574 | mqm->cb (mqm->cb_cls, |
553 | GNUNET_NO); | 575 | GNUNET_YES); |
554 | } | 576 | } |
555 | } | 577 | break; |
556 | else | ||
557 | { | ||
558 | GNUNET_assert (NULL == mqm->env); | ||
559 | mqm->cb (mqm->cb_cls, | ||
560 | GNUNET_YES); | ||
561 | } | 578 | } |
562 | } | 579 | } |
563 | if ( (NULL != mq) || | 580 | if ( (NULL != mq) || |