From 5f13e810b7a1d900bc4f2e78e9ccd7c74166c905 Mon Sep 17 00:00:00 2001 From: Thomas Schoebel-Theuer Date: Tue, 29 Jun 2021 10:52:11 +0200 Subject: [PATCH] all: safeguard mb on indirect calls --- kernel/brick.h | 47 +++++++++++++++++++++++++++++++++----- kernel/lib_log.c | 16 ++++++------- kernel/mars_buf.c | 9 ++++---- kernel/mars_check.c | 7 ++++-- kernel/mars_copy.c | 8 +++---- kernel/mars_dummy.c | 8 +++++-- kernel/mars_if.c | 4 ++-- kernel/mars_server.c | 14 ++++++++---- kernel/mars_trans_logger.c | 14 ++++++------ kernel/mars_usebuf.c | 6 ++--- 10 files changed, 90 insertions(+), 43 deletions(-) diff --git a/kernel/brick.h b/kernel/brick.h index 068c112d..8116a723 100644 --- a/kernel/brick.h +++ b/kernel/brick.h @@ -327,24 +327,59 @@ struct generic_output { GENERIC_OUTPUT(generic); }; -#define GENERIC_OUTPUT_CALL(OUTPUT,OP,ARGS...) \ + +#define _GENERIC_OUTPUT_CALL(OUTPUT,OP,ARGS...) \ ({ \ - mb(); \ - (void)LOCK_CHECK(OP), \ + (void)LOCK_CHECK(OP); \ (OUTPUT) && (OUTPUT)->ops->OP ? \ (OUTPUT)->ops->OP(OUTPUT, ##ARGS) : \ -ENOTCONN; \ }) -#define GENERIC_INPUT_CALL(INPUT,OP,ARGS...) \ +#define GENERIC_OUTPUT_CALL(OUTPUT,OP,ARGS...) \ + ({ \ + int __status; \ + \ + mb(); \ + __status = _GENERIC_OUTPUT_CALL(OUTPUT, OP, ##ARGS); \ + mb(); \ + __status; \ + }) + +#define GENERIC_OUTPUT_CALL_VOID(OUTPUT,OP,ARGS...) \ ({ \ mb(); \ - (void)LOCK_CHECK(OP), \ + _GENERIC_OUTPUT_CALL(OUTPUT, OP, ##ARGS); \ + mb(); \ + }) + + +#define _GENERIC_INPUT_CALL(INPUT,OP,ARGS...) \ + ({ \ + (void)LOCK_CHECK(OP); \ (INPUT) && (INPUT)->connect ? \ - GENERIC_OUTPUT_CALL((INPUT)->connect, OP, ##ARGS) : \ + _GENERIC_OUTPUT_CALL((INPUT)->connect, OP, ##ARGS) : \ -ENOTCONN; \ }) +#define GENERIC_INPUT_CALL(INPUT,OP,ARGS...) \ + ({ \ + int __status; \ + \ + mb(); \ + __status = _GENERIC_INPUT_CALL(INPUT, OP, ##ARGS); \ + mb(); \ + __status; \ + }) + +#define GENERIC_INPUT_CALL_VOID(INPUT,OP,ARGS...) \ + ({ \ + mb(); \ + _GENERIC_INPUT_CALL(INPUT, OP, ##ARGS); \ + mb(); \ + }) + + #define GENERIC_BRICK_OPS(BRITYPE) \ int (*brick_switch)(struct BRITYPE##_brick *brick); \ diff --git a/kernel/lib_log.c b/kernel/lib_log.c index a38bc2cf..b50d2475 100644 --- a/kernel/lib_log.c +++ b/kernel/lib_log.c @@ -51,12 +51,12 @@ void exit_logst(struct log_status *logst) } if (logst->read_mref) { MARS_DBG("putting read_mref\n"); - GENERIC_INPUT_CALL(logst->input, mref_put, logst->read_mref); + GENERIC_INPUT_CALL_VOID(logst->input, mref_put, logst->read_mref); logst->read_mref = NULL; } if (logst->log_mref) { MARS_DBG("putting log_mref\n"); - GENERIC_INPUT_CALL(logst->input, mref_put, logst->log_mref); + GENERIC_INPUT_CALL_VOID(logst->input, mref_put, logst->log_mref); logst->log_mref = NULL; } } @@ -230,8 +230,8 @@ void log_flush(struct log_status *logst) atomic_inc(&logst->mref_flying); atomic_inc(&global_mref_flying); - GENERIC_INPUT_CALL(logst->input, mref_io, mref); - GENERIC_INPUT_CALL(logst->input, mref_put, mref); + GENERIC_INPUT_CALL_VOID(logst->input, mref_io, mref); + GENERIC_INPUT_CALL_VOID(logst->input, mref_put, mref); logst->log_pos += logst->offset; logst->offset = 0; @@ -342,7 +342,7 @@ void *log_reserve(struct log_status *logst, struct log_header *lh) return data + offset; put: - GENERIC_INPUT_CALL(logst->input, mref_put, mref); + GENERIC_INPUT_CALL_VOID(logst->input, mref_put, mref); logst->log_mref = NULL; return NULL; @@ -541,7 +541,7 @@ restart: if (!mref || logst->do_free) { loff_t this_len; if (mref) { - GENERIC_INPUT_CALL(logst->input, mref_put, mref); + GENERIC_INPUT_CALL_VOID(logst->input, mref_put, mref); logst->read_mref = NULL; logst->log_pos += logst->offset; logst->offset = 0; @@ -583,7 +583,7 @@ restart: logst->got = false; logst->do_free = false; - GENERIC_INPUT_CALL(logst->input, mref_io, mref); + GENERIC_INPUT_CALL_VOID(logst->input, mref_io, mref); wait_event_interruptible_timeout(logst->event, logst->got, 60 * HZ); status = -ETIME; @@ -635,7 +635,7 @@ done: done_put: old_offset = logst->offset; if (mref) { - GENERIC_INPUT_CALL(logst->input, mref_put, mref); + GENERIC_INPUT_CALL_VOID(logst->input, mref_put, mref); logst->read_mref = NULL; logst->log_pos += logst->offset; logst->offset = 0; diff --git a/kernel/mars_buf.c b/kernel/mars_buf.c index 197c577b..0e025807 100644 --- a/kernel/mars_buf.c +++ b/kernel/mars_buf.c @@ -576,7 +576,8 @@ static void _buf_ref_put(struct buf_output *output, struct buf_mref_aspect *mref bf = mref_a->rfa_bf; if (!bf) { struct buf_brick *brick = output->brick; - GENERIC_INPUT_CALL(brick->inputs[0], mref_put, mref); + + GENERIC_INPUT_CALL_VOID(brick->inputs[0], mref_put, mref); return; } @@ -673,14 +674,14 @@ static int _buf_make_io(struct buf_brick *brick, struct buf_head *bf, void *star len = mref->ref_len; #ifndef FAKE_IO - GENERIC_INPUT_CALL(input, mref_io, mref); + GENERIC_INPUT_CALL_VOID(input, mref_io, mref); #else // fake IO for testing mref_a->cb.cb_error = status; mref_a->cb.cb_fn(&mref_a->cb); #endif - GENERIC_INPUT_CALL(input, mref_put, mref); + GENERIC_INPUT_CALL_VOID(input, mref_put, mref); start_data += len; start_pos += len; @@ -876,7 +877,7 @@ static void buf_ref_io(struct buf_output *output, struct mref_object *mref) */ bf = mref_a->rfa_bf; if (!bf) { - GENERIC_INPUT_CALL(brick->inputs[0], mref_io, mref); + GENERIC_INPUT_CALL_VOID(brick->inputs[0], mref_io, mref); return; } diff --git a/kernel/mars_check.c b/kernel/mars_check.c index 68daa462..27d57731 100644 --- a/kernel/mars_check.c +++ b/kernel/mars_check.c @@ -190,19 +190,22 @@ static int check_watchdog(void *data) static int check_get_info(struct check_output *output, struct mars_info *info) { struct check_input *input = output->brick->inputs[0]; + return GENERIC_INPUT_CALL(input, mars_get_info, info); } static int check_ref_get(struct check_output *output, struct mref_object *mref) { struct check_input *input = output->brick->inputs[0]; + return GENERIC_INPUT_CALL(input, mref_get, mref); } static void check_ref_put(struct check_output *output, struct mref_object *mref) { struct check_input *input = output->brick->inputs[0]; - GENERIC_INPUT_CALL(input, mref_put, mref); + + GENERIC_INPUT_CALL_VOID(input, mref_put, mref); } static void check_ref_io(struct check_output *output, struct mref_object *mref) @@ -240,7 +243,7 @@ static void check_ref_io(struct check_output *output, struct mref_object *mref) INSERT_CALLBACK(mref, &mref_a->cb, check_endio, mref_a); } - GENERIC_INPUT_CALL(input, mref_io, mref); + GENERIC_INPUT_CALL_VOID(input, mref_io, mref); atomic_inc(&mref_a->call_count); fatal: ; diff --git a/kernel/mars_copy.c b/kernel/mars_copy.c index 35aba477..e5356541 100644 --- a/kernel/mars_copy.c +++ b/kernel/mars_copy.c @@ -167,7 +167,7 @@ void __clear_mref(struct copy_brick *brick, struct mref_object *mref, unsigned q struct copy_input *input; input = queue ? brick->inputs[INPUT_B_COPY] : brick->inputs[INPUT_A_COPY]; - GENERIC_INPUT_CALL(input, mref_put, mref); + GENERIC_INPUT_CALL_VOID(input, mref_put, mref); } static @@ -389,7 +389,7 @@ int _make_mref(struct copy_brick *brick, atomic_inc(&global_copy_read_flight); } - GENERIC_INPUT_CALL(input, mref_io, mref); + GENERIC_INPUT_CALL_VOID(input, mref_io, mref); done: return status; @@ -975,7 +975,7 @@ static void copy_ref_put(struct copy_output *output, struct mref_object *mref) index = _determine_input(brick, mref); input = brick->inputs[index]; - GENERIC_INPUT_CALL(input, mref_put, mref); + GENERIC_INPUT_CALL_VOID(input, mref_put, mref); if (atomic_dec_and_test(&brick->io_flight)) { WRITE_ONCE(brick->trigger, true); wake_up_interruptible(&brick->event); @@ -989,7 +989,7 @@ static void copy_ref_io(struct copy_output *output, struct mref_object *mref) index = _determine_input(output->brick, mref); input = output->brick->inputs[index]; - GENERIC_INPUT_CALL(input, mref_io, mref); + GENERIC_INPUT_CALL_VOID(input, mref_io, mref); } static int copy_switch(struct copy_brick *brick) diff --git a/kernel/mars_dummy.c b/kernel/mars_dummy.c index 1dd3ecca..e9315934 100644 --- a/kernel/mars_dummy.c +++ b/kernel/mars_dummy.c @@ -47,6 +47,7 @@ static int dummy_get_info(struct dummy_output *output, struct mars_info *info) { struct dummy_input *input = output->brick->inputs[0]; + return GENERIC_INPUT_CALL(input, mars_get_info, info); } @@ -54,6 +55,7 @@ static int dummy_ref_get(struct dummy_output *output, struct mref_object *mref) { struct dummy_input *input = output->brick->inputs[0]; + return GENERIC_INPUT_CALL(input, mref_get, mref); } @@ -61,14 +63,16 @@ static void dummy_ref_put(struct dummy_output *output, struct mref_object *mref) { struct dummy_input *input = output->brick->inputs[0]; - GENERIC_INPUT_CALL(input, mref_put, mref); + + GENERIC_INPUT_CALL_VOID(input, mref_put, mref); } static void dummy_ref_io(struct dummy_output *output, struct mref_object *mref) { struct dummy_input *input = output->brick->inputs[0]; - GENERIC_INPUT_CALL(input, mref_io, mref); + + GENERIC_INPUT_CALL_VOID(input, mref_io, mref); } static diff --git a/kernel/mars_if.c b/kernel/mars_if.c index f0dd0f94..e1b73504 100644 --- a/kernel/mars_if.c +++ b/kernel/mars_if.c @@ -365,8 +365,8 @@ void _if_unplug(struct if_input *input) atomic_inc(&input->total_skip_sync_count); #endif - GENERIC_INPUT_CALL(input, mref_io, mref); - GENERIC_INPUT_CALL(input, mref_put, mref); + GENERIC_INPUT_CALL_VOID(input, mref_io, mref); + GENERIC_INPUT_CALL_VOID(input, mref_put, mref); } #ifdef IO_DEBUGGING { diff --git a/kernel/mars_server.c b/kernel/mars_server.c index f5828e7e..770fdd0b 100644 --- a/kernel/mars_server.c +++ b/kernel/mars_server.c @@ -172,7 +172,7 @@ int cb_thread(void *data) mref->ref_data = NULL; } if (mref_a->do_put) { - GENERIC_INPUT_CALL(brick->inputs[0], mref_put, mref); + GENERIC_INPUT_CALL_VOID(brick->inputs[0], mref_put, mref); atomic_dec(&brick->in_flight); } else { mars_free_mref(mref); @@ -271,7 +271,7 @@ int server_io(struct server_brick *brick, struct mars_socket *sock, struct mars_ } mref_a->do_put = true; atomic_inc(&brick->in_flight); - GENERIC_INPUT_CALL(brick->inputs[0], mref_io, mref); + GENERIC_INPUT_CALL_VOID(brick->inputs[0], mref_io, mref); done: return status; @@ -296,7 +296,7 @@ void _clean_list(struct server_brick *brick, struct list_head *start) continue; if (mref_a->do_put) { - GENERIC_INPUT_CALL(brick->inputs[0], mref_put, mref); + GENERIC_INPUT_CALL_VOID(brick->inputs[0], mref_put, mref); atomic_dec(&brick->in_flight); } else { mars_free_mref(mref); @@ -678,25 +678,29 @@ int handler_thread(void *data) static int server_get_info(struct server_output *output, struct mars_info *info) { struct server_input *input = output->brick->inputs[0]; + return GENERIC_INPUT_CALL(input, mars_get_info, info); } static int server_ref_get(struct server_output *output, struct mref_object *mref) { struct server_input *input = output->brick->inputs[0]; + return GENERIC_INPUT_CALL(input, mref_get, mref); } static void server_ref_put(struct server_output *output, struct mref_object *mref) { struct server_input *input = output->brick->inputs[0]; - GENERIC_INPUT_CALL(input, mref_put, mref); + + GENERIC_INPUT_CALL_VOID(input, mref_put, mref); } static void server_ref_io(struct server_output *output, struct mref_object *mref) { struct server_input *input = output->brick->inputs[0]; - GENERIC_INPUT_CALL(input, mref_io, mref); + + GENERIC_INPUT_CALL_VOID(input, mref_io, mref); } static int server_switch(struct server_brick *brick) diff --git a/kernel/mars_trans_logger.c b/kernel/mars_trans_logger.c index 8ce1cf1e..66b807ce 100644 --- a/kernel/mars_trans_logger.c +++ b/kernel/mars_trans_logger.c @@ -970,7 +970,7 @@ restart: input = brick->inputs[TL_INPUT_READ]; CHECK_PTR(input, err); - GENERIC_INPUT_CALL(input, mref_put, mref); + GENERIC_INPUT_CALL_VOID(input, mref_put, mref); err: ; } @@ -1089,7 +1089,7 @@ void trans_logger_ref_io(struct trans_logger_output *output, struct mref_object input = output->brick->inputs[TL_INPUT_READ]; - GENERIC_INPUT_CALL(input, mref_io, mref); + GENERIC_INPUT_CALL_VOID(input, mref_io, mref); return; err: MARS_FAT("cannot handle IO\n"); @@ -1465,12 +1465,12 @@ void _fire_one(struct list_head *tmp, bool do_update) sub_input = sub_mref_a->my_input; #ifdef DO_WRITEBACK - GENERIC_INPUT_CALL(sub_input, mref_io, sub_mref); + GENERIC_INPUT_CALL_VOID(sub_input, mref_io, sub_mref); #else SIMPLE_CALLBACK(sub_mref, 0); #endif if (do_update) { // CHECK: shouldn't we do this always? - GENERIC_INPUT_CALL(sub_input, mref_put, sub_mref); + GENERIC_INPUT_CALL_VOID(sub_input, mref_put, sub_mref); } } @@ -2140,7 +2140,7 @@ bool phase3_startio(struct writeback_info *wb) sub_mref = sub_mref_a->object; sub_input = sub_mref_a->my_input; - GENERIC_INPUT_CALL(sub_input, mref_put, sub_mref); + GENERIC_INPUT_CALL_VOID(sub_input, mref_put, sub_mref); } update_writeback_info(wb); @@ -3038,7 +3038,7 @@ int replay_data(struct trans_logger_brick *brick, loff_t pos, void *buf, int len SETUP_CALLBACK(mref, replay_endio, mref_a); mref_a->my_brick = brick; - GENERIC_INPUT_CALL(input, mref_io, mref); + GENERIC_INPUT_CALL_VOID(input, mref_io, mref); if (unlikely(mref->ref_len <= 0)) { status = -EINVAL; @@ -3050,7 +3050,7 @@ int replay_data(struct trans_logger_brick *brick, loff_t pos, void *buf, int len buf += mref->ref_len; len -= mref->ref_len; - GENERIC_INPUT_CALL(input, mref_put, mref); + GENERIC_INPUT_CALL_VOID(input, mref_put, mref); } #endif status = 0; diff --git a/kernel/mars_usebuf.c b/kernel/mars_usebuf.c index 335157f1..da5a1d4e 100644 --- a/kernel/mars_usebuf.c +++ b/kernel/mars_usebuf.c @@ -100,7 +100,7 @@ static void _usebuf_endio(struct generic_callback *cb) sub_mref->ref_flags |= MREF_WRITE; _usebuf_copy(mref, sub_mref, 1); mref->ref_flags |= MREF_UPTODATE; - GENERIC_INPUT_CALL(mref_a->input, mref_io, sub_mref); + GENERIC_INPUT_CALL_VOID(mref_a->input, mref_io, sub_mref); return; #endif } @@ -219,7 +219,7 @@ static void usebuf_ref_put(struct usebuf_output *output, struct mref_object *mre if (!_mref_put(mref)) return; - GENERIC_INPUT_CALL(input, mref_put, sub_mref); + GENERIC_INPUT_CALL_VOID(input, mref_put, sub_mref); usebuf_free_mref(mref); } @@ -291,7 +291,7 @@ static void usebuf_ref_io(struct usebuf_output *output, struct mref_object *mref _usebuf_endio(sub_mref->ref_cb); return; #endif - GENERIC_INPUT_CALL(input, mref_io, sub_mref); + GENERIC_INPUT_CALL_VOID(input, mref_io, sub_mref); return;