Re: [PATCH] [5/12] x86_64: Make patching more robust, fix paravirt issue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Linus Torvalds ([email protected]) wrote:
> On Sat, 18 Aug 2007, Chris Wright wrote:
> > > 
> > > Check the latest git head. Does it still break?
> > 
> > Yeah, this is the latest git.  The broken commit is Rusty's patch which,
> > after Linus reverted the write-protected remap changes, is no longer
> > necessary.  AFAICT patching is writing garbage into the insn stream.
> > I suspect it's copying an uninitialized temp buffer.
> 
> Can you send me the revert patch that is verified to work?

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.

thanks,
-chris
--

Subject: [PATCH] x86: properly initialize temp insn buffer for paravirt patching 
From: Chris Wright <[email protected]>

With commit ab144f5ec64c42218a555ec1dbde6b60cf2982d6 the patching code
now collects the complete new instruction stream into a temp buffer
before finally patching in the new insns.  In some cases the paravirt
patchers will choose to leave the patch site unpatched (length mismatch,
clobbers mismatch, etc).  This causes the new patching code to copy an
uninitialized temp buffer, i.e. garbage, to the callsite.  Simply make
sure to always initialize the buffer with the original instruction stream.
A better fix is to audit all the patchers and return proper length so that
apply_paravirt() can skip copies when we leave the patch site untouched.

Signed-off-by: Chris Wright <[email protected]>
---
 arch/i386/kernel/alternative.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/i386/kernel/alternative.c b/arch/i386/kernel/alternative.c
index 1b66d5c..9f4ac8b 100644
--- a/arch/i386/kernel/alternative.c
+++ b/arch/i386/kernel/alternative.c
@@ -366,6 +366,8 @@ 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);
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux