linux-kernel - [PATCH 3/9] sh: Split breakpoint validation into "check" and "commit"

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]
Date:   Sun,  6 May 2018 21:19:49 +0200
From:   Frederic Weisbecker <frederic@...nel.org>
To:     LKML <linux-kernel@...r.kernel.org>
Cc:     Frederic Weisbecker <frederic@...nel.org>,
        Jiri Olsa <jolsa@...hat.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Chris Zankel <chris@...kel.net>,
        Paul Mackerras <paulus@...ba.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Will Deacon <will.deacon@....com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Rich Felker <dalias@...c.org>, Ingo Molnar <mingo@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Max Filippov <jcmvbkbc@...il.com>
Subject: [PATCH 3/9] sh: Split breakpoint validation into "check" and "commit"

The breakpoint code mixes up attribute check and commit into a single
code entity. Therefore the validation may return an error due to
incorrect atributes while still leaving halfway modified architecture
breakpoint struct.

Prepare fox fixing this misdesign and separate both logics.

Signed-off-by: Frederic Weisbecker <frederic@...nel.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Andy Lutomirski <luto@...nel.org>
Cc: Yoshinori Sato <ysato@...rs.sourceforge.jp>
Cc: Rich Felker <dalias@...c.org>
Cc: Ingo Molnar <mingo@...nel.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Will Deacon <will.deacon@....com>
Cc: Mark Rutland <mark.rutland@....com>
Cc: Max Filippov <jcmvbkbc@...il.com>
Cc: Chris Zankel <chris@...kel.net>
Cc: Catalin Marinas <catalin.marinas@....com>
Cc: Benjamin Herrenschmidt <benh@...nel.crashing.org>
Cc: Paul Mackerras <paulus@...ba.org>
Cc: Michael Ellerman <mpe@...erman.id.au>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
Cc: Jiri Olsa <jolsa@...hat.com>
Cc: Namhyung Kim <namhyung@...nel.org>
---
 arch/sh/kernel/hw_breakpoint.c | 89 ++++++++++++++++++++++++------------------
 1 file changed, 50 insertions(+), 39 deletions(-)

diff --git a/arch/sh/kernel/hw_breakpoint.c b/arch/sh/kernel/hw_breakpoint.c
index 286c3ab..c89152f 100644
--- a/arch/sh/kernel/hw_breakpoint.c
+++ b/arch/sh/kernel/hw_breakpoint.c
@@ -174,14 +174,53 @@ int arch_bp_generic_fields(int sh_len, int sh_type,
 	return 0;
 }
 
-static int arch_build_bp_info(struct perf_event *bp)
+static int hw_breakpoint_arch_check(struct perf_event *bp,
+				    const struct perf_event_attr *attr)
+{
+	unsigned int align;
+
+	/* Check type */
+	switch (attr->bp_type) {
+	case HW_BREAKPOINT_W:
+	case HW_BREAKPOINT_R:
+	case HW_BREAKPOINT_W | HW_BREAKPOINT_R:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Check len */
+	switch (attr->bp_len) {
+	case HW_BREAKPOINT_LEN_1:
+	case HW_BREAKPOINT_LEN_2:
+	case HW_BREAKPOINT_LEN_4:
+	case HW_BREAKPOINT_LEN_8:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	align = attr->bp_len - 1;
+
+	/*
+	 * Check that the low-order bits of the address are appropriate
+	 * for the alignment implied by len.
+	 */
+	if (attr->bp_addr & align)
+		return -EINVAL;
+
+	return 0;
+}
+
+static void hw_breakpoint_arch_commit(struct perf_event *bp)
 {
 	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+	struct perf_event_attr *attr = &bp->attr;
 
-	info->address = bp->attr.bp_addr;
+	info->address = attr->bp_addr;
 
 	/* Len */
-	switch (bp->attr.bp_len) {
+	switch (attr->bp_len) {
 	case HW_BREAKPOINT_LEN_1:
 		info->len = SH_BREAKPOINT_LEN_1;
 		break;
@@ -195,11 +234,11 @@ static int arch_build_bp_info(struct perf_event *bp)
 		info->len = SH_BREAKPOINT_LEN_8;
 		break;
 	default:
-		return -EINVAL;
+		WARN_ON_ONCE(1);
 	}
 
 	/* Type */
-	switch (bp->attr.bp_type) {
+	switch (attr->bp_type) {
 	case HW_BREAKPOINT_R:
 		info->type = SH_BREAKPOINT_READ;
 		break;
@@ -210,10 +249,8 @@ static int arch_build_bp_info(struct perf_event *bp)
 		info->type = SH_BREAKPOINT_RW;
 		break;
 	default:
-		return -EINVAL;
+		WARN_ON_ONCE(1);
 	}
-
-	return 0;
 }
 
 /*
@@ -221,39 +258,13 @@ static int arch_build_bp_info(struct perf_event *bp)
  */
 int arch_validate_hwbkpt_settings(struct perf_event *bp)
 {
-	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
-	unsigned int align;
-	int ret;
+	int err;
 
-	ret = arch_build_bp_info(bp);
-	if (ret)
-		return ret;
+	err = hw_breakpoint_arch_check(bp, &bp->attr);
+	if (err)
+		return err;
 
-	ret = -EINVAL;
-
-	switch (info->len) {
-	case SH_BREAKPOINT_LEN_1:
-		align = 0;
-		break;
-	case SH_BREAKPOINT_LEN_2:
-		align = 1;
-		break;
-	case SH_BREAKPOINT_LEN_4:
-		align = 3;
-		break;
-	case SH_BREAKPOINT_LEN_8:
-		align = 7;
-		break;
-	default:
-		return ret;
-	}
-
-	/*
-	 * Check that the low-order bits of the address are appropriate
-	 * for the alignment implied by len.
-	 */
-	if (info->address & align)
-		return -EINVAL;
+	hw_breakpoint_arch_commit(bp);
 
 	return 0;
 }
-- 
2.7.4