linux-kernel - [PATCH] x86: skip paravirt patching when appropriate

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: <20070818222821.GX3672@sequoia.sous-sol.org>
Date:	Sat, 18 Aug 2007 15:28:21 -0700
From:	Chris Wright <chrisw@...s-sol.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Chris Wright <chrisw@...s-sol.org>, Andi Kleen <ak@...e.de>,
	Jeremy Fitzhardinge <jeremy@...p.org>, patches@...-64.org,
	linux-kernel@...r.kernel.org,
	Rusty Russell <rusty@...tcorp.com.au>,
	Zach Amsden <zach@...are.com>
Subject: [PATCH] x86: skip paravirt patching when appropriate

* Chris Wright (chrisw@...s-sol.org) wrote:
> Now that I understand the problem, I do have a very simple (slightly
> overkill) fix for paravirt patching.  This can be cleaned up to avoid
> the copies when they aren't needed, but that will take a little more
> auditing of the various patchers.  If you still prefer a revert I've
> got one handy, and we can re-visit this all post .23.

This one avoids the patching when necessary, but needs some validation on
VMI (and Zach's not avail today).  I'll resend when I know it's working
for all paravirt patchers.

thanks,
-chris
--

Subject: [PATCH] x86: skip paravirt patching when appropriate
From: Chris Wright <chrisw@...s-sol.org>

commit d34fda4a84c18402640a1a2342d6e6d9829e6db7 was a little overkill
in the case where a paravirt patcher chooses to leave patch site
unpatched.  Instead of copying original instructions to temp buffer
then back to patch site, simply skip patching those sites altogether.

Cc: Jeremy Fitzhardinge <jeremy@...p.org>
Cc: Rusty Russell <rusty@...tcorp.com.au>
Cc: Zach Amsden <zach@...are.com>
Signed-off-by: Chris Wright <chrisw@...s-sol.org>
---
 arch/i386/kernel/alternative.c |    4 ++--
 arch/i386/kernel/paravirt.c    |   10 +++++-----
 arch/i386/kernel/vmi.c         |    4 ++--
 include/asm-i386/paravirt.h    |    3 +++
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/i386/kernel/alternative.c b/arch/i386/kernel/alternative.c
index 9f4ac8b..b81d87e 100644
--- a/arch/i386/kernel/alternative.c
+++ b/arch/i386/kernel/alternative.c
@@ -366,10 +366,10 @@ void apply_paravirt(struct paravirt_patch_site *start,
 		unsigned int used;
 
 		BUG_ON(p->len > MAX_PATCH_LEN);
-		/* prep the buffer with the original instructions */
-		memcpy(insnbuf, p->instr, p->len);
 		used = paravirt_ops.patch(p->instrtype, p->clobbers, insnbuf,
 					  (unsigned long)p->instr, p->len);
+		if (used == PV_NO_PATCH)
+			continue;
 
 		BUG_ON(used > p->len);
 
diff --git a/arch/i386/kernel/paravirt.c b/arch/i386/kernel/paravirt.c
index 739cfb2..a36ce34 100644
--- a/arch/i386/kernel/paravirt.c
+++ b/arch/i386/kernel/paravirt.c
@@ -122,7 +122,7 @@ unsigned paravirt_patch_nop(void)
 
 unsigned paravirt_patch_ignore(unsigned len)
 {
-	return len;
+	return PV_NO_PATCH;
 }
 
 struct branch {
@@ -139,9 +139,9 @@ unsigned paravirt_patch_call(void *insnbuf,
 	unsigned long delta = (unsigned long)target - (addr+5);
 
 	if (tgt_clobbers & ~site_clobbers)
-		return len;	/* target would clobber too much for this site */
+		return PV_NO_PATCH;	/* target would clobber too much for this site */
 	if (len < 5)
-		return len;	/* call too long for patch site */
+		return PV_NO_PATCH;	/* call too long for patch site */
 
 	b->opcode = 0xe8; /* call */
 	b->delta = delta;
@@ -157,7 +157,7 @@ unsigned paravirt_patch_jmp(const void *target, void *insnbuf,
 	unsigned long delta = (unsigned long)target - (addr+5);
 
 	if (len < 5)
-		return len;	/* call too long for patch site */
+		return PV_NO_PATCH;	/* call too long for patch site */
 
 	b->opcode = 0xe9;	/* jmp */
 	b->delta = delta;
@@ -196,7 +196,7 @@ unsigned paravirt_patch_insns(void *insnbuf, unsigned len,
 	unsigned insn_len = end - start;
 
 	if (insn_len > len || start == NULL)
-		insn_len = len;
+		insn_len = PV_NO_PATCH;
 	else
 		memcpy(insnbuf, start, insn_len);
 
diff --git a/arch/i386/kernel/vmi.c b/arch/i386/kernel/vmi.c
index 18673e0..27ae004 100644
--- a/arch/i386/kernel/vmi.c
+++ b/arch/i386/kernel/vmi.c
@@ -118,7 +118,7 @@ static unsigned patch_internal(int call, unsigned len, void *insnbuf,
 
 		case VMI_RELOCATION_NONE:
 			/* leave native code in place */
-			break;
+			return PV_NO_PATCH;
 
 		default:
 			BUG();
@@ -153,7 +153,7 @@ static unsigned vmi_patch(u8 type, u16 clobbers, void *insns,
 		default:
 			break;
 	}
-	return len;
+	return PV_NO_PATCH;
 }
 
 /* CPUID has non-C semantics, and paravirt-ops API doesn't match hardware ISA */
diff --git a/include/asm-i386/paravirt.h b/include/asm-i386/paravirt.h
index 9fa3fa9..b26794f 100644
--- a/include/asm-i386/paravirt.h
+++ b/include/asm-i386/paravirt.h
@@ -252,6 +252,9 @@ extern struct paravirt_ops paravirt_ops;
 #define paravirt_alt(insn_string)					\
 	_paravirt_alt(insn_string, "%c[paravirt_typenum]", "%c[paravirt_clobber]")
 
+enum {
+	PV_NO_PATCH = -1
+};
 unsigned paravirt_patch_nop(void);
 unsigned paravirt_patch_ignore(unsigned len);
 unsigned paravirt_patch_call(void *insnbuf,
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/