Commit cd5d4a6c authored by Yonghong Song's avatar Yonghong Song

fix a race condition between perf_reader munmap and read

Fix issue #1533.

Currently, there exist a race condition between perf_reader
buffer munmap and read if they are happening in two different
threads, crash is possible as in issue #1533.

          thread 1                    thread 2
          perf_reader_event_read      ...
                                      detach_probe
                                      munmap
          access ring buffer

detach_probe may happen as part of bpf object exit cleanup process
at which point thread 1 is still alive. In this case, accessing
ring buffer may cause segfault since the original mmap'ed memory
is not available any more.

It is hard to fix such races outside bcc since user
calls kprobe_poll which has valid BPF object when it is called,
but race happens inside the kprobe_poll.

This patch adds a state of the ring buffer and the read will
not happen once the state comes to the munmap.
Signed-off-by: default avatarYonghong Song <yhs@fb.com>
parent 89aa2948
......@@ -28,6 +28,12 @@
#include "libbpf.h"
#include "perf_reader.h"
enum {
RB_NOT_USED = 0, // ring buffer not usd
RB_USED_IN_MUNMAP = 1, // used in munmap
RB_USED_IN_READ = 2, // used in read
};
struct perf_reader {
perf_reader_cb cb;
perf_reader_raw_cb raw_cb;
......@@ -36,6 +42,7 @@ struct perf_reader {
void *buf; // for keeping segmented data
size_t buf_size;
void *base;
int rb_use_state;
int page_size;
int page_cnt;
int fd;
......@@ -63,6 +70,7 @@ struct perf_reader * perf_reader_new(perf_reader_cb cb,
void perf_reader_free(void *ptr) {
if (ptr) {
struct perf_reader *reader = ptr;
while (!__sync_bool_compare_and_swap(&reader->rb_use_state, RB_NOT_USED, RB_USED_IN_MUNMAP));
munmap(reader->base, reader->page_size * (reader->page_cnt + 1));
if (reader->fd >= 0) {
ioctl(reader->fd, PERF_EVENT_IOC_DISABLE, 0);
......@@ -220,6 +228,9 @@ void perf_reader_event_read(struct perf_reader *reader) {
uint8_t *sentinel = (uint8_t *)reader->base + buffer_size + reader->page_size;
uint8_t *begin, *end;
if (!__sync_bool_compare_and_swap(&reader->rb_use_state, RB_NOT_USED, RB_USED_IN_READ))
return;
// Consume all the events on this ring, calling the cb function for each one.
// The message may fall on the ring boundary, in which case copy the message
// into a malloced buffer.
......@@ -268,6 +279,7 @@ void perf_reader_event_read(struct perf_reader *reader) {
write_data_tail(perf_header, perf_header->data_tail + e->size);
}
reader->rb_use_state = RB_NOT_USED;
}
int perf_reader_poll(int num_readers, struct perf_reader **readers, int timeout) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment