Skip to content

Commit

Permalink
ovs-rcu: Avoid flushing callbacks during postponing.
Browse files Browse the repository at this point in the history
ovsrcu_flush_cbset() call during ovsrcu_postpone() could cause
use after free in case the caller sets new pointer only after
postponing free for the old one:

 ------------------  ------------------  -------------------
 Thread 1            Thread 2            RCU Thread
 ------------------  ------------------  -------------------
 pointer = A

 ovsrcu_quiesce():
  thread->seqno = 30
  global_seqno = 31
  quiesced

 read pointer A
 postpone(free(A)):
   flush cbset
                                         pop flushed_cbsets
                                         ovsrcu_synchronize:
                                           target_seqno = 31
                     ovsrcu_quiesce():
                      thread->seqno = 31
                      global_seqno = 32
                      quiesced

                     read pointer A
                     use pointer A

                     ovsrcu_quiesce():
                      thread->seqno = 32
                      global_seqno = 33
                      quiesced

                     read pointer A
 pointer = B

 ovsrcu_quiesce():
  thread->seqno = 33
  global_seqno = 34
  quiesced

                                         target_seqno exceeded
                                         by all threads
                                         call cbs to free A
                     use pointer A
                     (use after free)
 -----------------------------------------------------------

Fix that by using dynamically re-allocated array without flushing
to the global flushed_cbsets until writer enters quiescent state.

Fixes: 0f2ea84 ("ovs-rcu: New library.")
Reported-by: Linhaifeng <haifeng.lin@huawei.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-June/371265.html
Acked-by: Ben Pfaff <blp@ovn.org>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
  • Loading branch information
igsilya committed Jun 10, 2020
1 parent e485dce commit 312359a
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 5 deletions.
1 change: 1 addition & 0 deletions AUTHORS.rst
Expand Up @@ -545,6 +545,7 @@ Krishna Miriyala miriyalak@vmware.com
Krishna Mohan Elluru elluru.kri.mohan@hpe.com
László Sürü laszlo.suru@ericsson.com
Len Gao leng@vmware.com
Linhaifeng haifeng.lin@huawei.com
Logan Rosen logatronico@gmail.com
Luca Falavigna dktrkranz@debian.org
Luiz Henrique Ozaki luiz.ozaki@gmail.com
Expand Down
17 changes: 12 additions & 5 deletions lib/ovs-rcu.c
Expand Up @@ -30,14 +30,17 @@

VLOG_DEFINE_THIS_MODULE(ovs_rcu);

#define MIN_CBS 16

struct ovsrcu_cb {
void (*function)(void *aux);
void *aux;
};

struct ovsrcu_cbset {
struct ovs_list list_node;
struct ovsrcu_cb cbs[16];
struct ovsrcu_cb *cbs;
size_t n_allocated;
int n_cbs;
};

Expand Down Expand Up @@ -310,16 +313,19 @@ ovsrcu_postpone__(void (*function)(void *aux), void *aux)
cbset = perthread->cbset;
if (!cbset) {
cbset = perthread->cbset = xmalloc(sizeof *perthread->cbset);
cbset->cbs = xmalloc(MIN_CBS * sizeof *cbset->cbs);
cbset->n_allocated = MIN_CBS;
cbset->n_cbs = 0;
}

if (cbset->n_cbs == cbset->n_allocated) {
cbset->cbs = x2nrealloc(cbset->cbs, &cbset->n_allocated,
sizeof *cbset->cbs);
}

cb = &cbset->cbs[cbset->n_cbs++];
cb->function = function;
cb->aux = aux;

if (cbset->n_cbs >= ARRAY_SIZE(cbset->cbs)) {
ovsrcu_flush_cbset(perthread);
}
}

static bool
Expand All @@ -341,6 +347,7 @@ ovsrcu_call_postponed(void)
for (cb = cbset->cbs; cb < &cbset->cbs[cbset->n_cbs]; cb++) {
cb->function(cb->aux);
}
free(cbset->cbs);
free(cbset);
}

Expand Down

0 comments on commit 312359a

Please sign in to comment.