Messages in this thread Patch in this message | | | Date | Sun, 10 Mar 2019 14:10:46 +0100 | From | Peter Zijlstra <> | Subject | Re: [PATCH 18/20] objtool: Add UACCESS validation |
| |
On Fri, Mar 08, 2019 at 03:02:09PM -0600, Josh Poimboeuf wrote:
> These gotos make my head spin. Again I would much prefer a small amount > of code duplication over this.
something like so then?
--- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -1888,6 +1888,23 @@ static inline const char *insn_dest_name return "{dynamic}"; } +static int validate_call(struct instruction *insn, struct insn_state *state) +{ + if (state->uaccess && !func_uaccess_safe(insn->call_dest)) { + WARN_FUNC("call to %s() with UACCESS enabled", + insn->sec, insn->offset, insn_dest_name(insn)); + return 1; + } + + if (state->df) { + WARN_FUNC("call to %s() with DF set", + insn->sec, insn->offset, insn_dest_name(insn)); + return 1; + } + + return 0; +} + /* * Follow the branch starting at the given instruction, and recursively follow * any other branches (jumps). Meanwhile, track the frame pointer state at @@ -2036,25 +2053,9 @@ static int validate_branch(struct objtoo case INSN_CALL: case INSN_CALL_DYNAMIC: -do_call: - if (state.uaccess && !func_uaccess_safe(insn->call_dest)) { - WARN_FUNC("call to %s() with UACCESS enabled", - sec, insn->offset, insn_dest_name(insn)); - return 1; - } - - if (state.df) { - WARN_FUNC("call to %s() with DF set", - sec, insn->offset, insn_dest_name(insn)); - return 1; - } - - if (insn->type == INSN_JUMP_UNCONDITIONAL || - insn->type == INSN_JUMP_DYNAMIC) - return 0; - - if (insn->type == INSN_JUMP_CONDITIONAL) - break; + ret = validate_call(insn, &state); + if (ret) + return ret; if (insn->type == INSN_CALL) { if (is_fentry_call(insn)) @@ -2077,13 +2078,15 @@ static int validate_branch(struct objtoo case INSN_JUMP_CONDITIONAL: case INSN_JUMP_UNCONDITIONAL: if (func && !insn->jump_dest) { -do_sibling_call: + /* sibling call */ if (has_modified_stack_frame(&state)) { WARN_FUNC("sibling call from callable instruction with modified stack frame", sec, insn->offset); return 1; } - goto do_call; + ret = validate_call(insn, &state); + if (ret) + return ret; } else if (insn->jump_dest && (!func || !insn->jump_dest->func || @@ -2104,8 +2107,17 @@ static int validate_branch(struct objtoo break; case INSN_JUMP_DYNAMIC: - if (func && list_empty(&insn->alts)) - goto do_sibling_call; + if (func && list_empty(&insn->alts)) { + /* sibling call */ + if (has_modified_stack_frame(&state)) { + WARN_FUNC("sibling call from callable instruction with modified stack frame", + sec, insn->offset); + return 1; + } + ret = validate_call(insn, &state); + if (ret) + return ret; + } return 0;
| |