lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20200414103618.12657-7-alexandre.chartre@oracle.com>
Date:   Tue, 14 Apr 2020 12:36:15 +0200
From:   Alexandre Chartre <alexandre.chartre@...cle.com>
To:     x86@...nel.org
Cc:     linux-kernel@...r.kernel.org, jpoimboe@...hat.com,
        peterz@...radead.org, jthierry@...hat.com, tglx@...utronix.de,
        alexandre.chartre@...cle.com
Subject: [PATCH V3 6/9] objtool: Report inconsistent stack changes in alternative

To allow a valid stack unwinding, an alternative should have code
where the same stack changes happens at the same places as in the
original code. Add a check in objtool to validate that stack changes
in alternative are effectively consitent with the original code.

Signed-off-by: Alexandre Chartre <alexandre.chartre@...cle.com>
---
 tools/objtool/check.c | 155 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 155 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 0574ce8e232d..9488a9c7be24 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2724,6 +2724,156 @@ static int validate_reachable_instructions(struct objtool_file *file)
 	return 0;
 }
 
+static int validate_alternative_state(struct objtool_file *file,
+				      struct instruction *first,
+				      struct instruction *prev,
+				      struct instruction *current,
+				      bool include_current)
+{
+	struct instruction *alt_insn, *alt_first;
+	unsigned long roff_prev, roff_curr, roff;
+	int count, err = 0, err_alt, alt_num;
+	struct alternative *alt;
+	const char *err_str;
+
+	/*
+	 * Check that instructions in alternatives between the corresponding
+	 * previous and current instructions, have the same stack state.
+	 */
+
+	/* use relative offset to find corresponding alt instructions */
+	roff_prev = prev->offset - first->offset;
+	roff_curr = current->offset - first->offset;
+	alt_num = 0;
+
+	list_for_each_entry(alt, &first->alts, list) {
+		if (!alt->insn->alt_group)
+			continue;
+
+		alt_num++;
+		alt_first = alt->insn;
+		count = 0;
+		err_alt = 0;
+
+		for (alt_insn = alt_first;
+		     alt_insn && alt_insn->alt_group == alt_first->alt_group;
+		     alt_insn = next_insn_same_sec(file, alt_insn)) {
+
+			roff = alt_insn->offset - alt_first->offset;
+			if (roff < roff_prev)
+				continue;
+
+			if (roff > roff_curr ||
+			    (roff == roff_curr && !include_current))
+				break;
+
+			count++;
+			/*
+			 * The first instruction we check must be aligned with
+			 * the corresponding "prev" instruction because stack
+			 * change should happen at the same offset.
+			 */
+			if (count == 1 && roff != roff_prev) {
+				err_str = "misaligned alternative state change";
+				err_alt++;
+			}
+
+			if (!err_alt && memcmp(&prev->state, &alt_insn->state,
+					       sizeof(struct insn_state))) {
+				err_str = "invalid alternative state";
+				err_alt++;
+			}
+
+			/*
+			 * On error, report the error and stop checking
+			 * this alternative but continue checking other
+			 * alternatives.
+			 */
+			if (err_alt) {
+				if (!err) {
+					WARN_FUNC("error in alternative",
+						  first->sec, first->offset);
+				}
+				WARN_FUNC("in alternative %d",
+					  alt_first->sec, alt_first->offset,
+					  alt_num);
+				WARN_FUNC("%s", alt_insn->sec, alt_insn->offset,
+					  err_str);
+
+				err += err_alt;
+				break;
+			}
+		}
+	}
+
+	return err;
+}
+
+static int validate_alternative(struct objtool_file *file)
+{
+	struct instruction *first, *prev, *next, *insn;
+	bool in_alternative;
+	int err;
+
+	err = 0;
+	first = prev = NULL;
+	in_alternative = false;
+	for_each_insn(file, insn) {
+		if (insn->ignore || !insn->alt_group || insn->ignore_alts)
+			continue;
+
+		if (!in_alternative) {
+			if (list_empty(&insn->alts))
+				continue;
+
+			/*
+			 * Setup variables to start the processing of a
+			 * new alternative.
+			 */
+			first = insn;
+			prev = insn;
+			err = 0;
+			in_alternative = true;
+
+		} else if (!err && memcmp(&prev->state, &insn->state,
+					  sizeof(struct insn_state))) {
+			/*
+			 * The stack state has changed and no error was
+			 * reported for this alternative. Check that the
+			 * stack state is the same in all alternatives
+			 * between the last check and up to this instruction.
+			 *
+			 * Once we have an error, stop checking the
+			 * alternative because once the stack state is
+			 * inconsistent, it will likely be inconsistent
+			 * for other instructions as well.
+			 */
+			err = validate_alternative_state(file, first,
+							 prev, insn, false);
+			prev = insn;
+		}
+
+		/*
+		 * If the next instruction is in the same alternative then
+		 * continue with processing this alternative. Otherwise
+		 * that's the end of this alternative so check there is no
+		 * stack state changes in remaining instructions (if no
+		 * error was reported yet).
+		 */
+		next = list_next_entry(insn, list);
+		if (next && !next->ignore &&
+		    next->alt_group == first->alt_group)
+			continue;
+
+		if (!err)
+			err = validate_alternative_state(file, first,
+							 prev, insn, true);
+		in_alternative = false;
+	}
+
+	return 0;
+}
+
 static struct objtool_file file;
 
 int check(const char *_objname, bool orc)
@@ -2769,6 +2919,11 @@ int check(const char *_objname, bool orc)
 		goto out;
 	warnings += ret;
 
+	ret = validate_alternative(&file);
+	if (ret < 0)
+		goto out;
+	warnings += ret;
+
 	if (!warnings) {
 		ret = validate_reachable_instructions(&file);
 		if (ret < 0)
-- 
2.18.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ