On 19/02/2016 12:11, Aurelien Jarno wrote:
> On 2015-08-29 17:49, Valdis Kletnieks wrote:
>> Compiler warning:
>>
>> CC [M] arch/x86/kvm/emulate.o
>> arch/x86/kvm/emulate.c: In function "__do_insn_fetch_bytes":
>> arch/x86/kvm/emulate.c:814:9: warning: "linear" may be used uninitialized in this function [-Wmaybe-uninitialized]
>>
>> GCC is smart enough to realize that the inlined __linearize may return before
>> setting the value of linear, but not smart enough to realize the same
>> X86EMU_CONTINUE blocks actual use of the value. However, the value of
>> 'linear' can only be set to one value, so hoisting the one line of code
>> upwards makes GCC happy with the code.
>>
>> Reported-by: Aruna Hewapathirane <aruna.hewapathirane@xxxxxxxxx>
>> Tested-by: Aruna Hewapathirane <aruna.hewapathirane@xxxxxxxxx>
>> Signed-off-by: Valdis Kletnieks <valdis.kletnieks@xxxxxx>
>>
>> --- a/arch/x86/kvm/emulate.c.dist 2015-08-11 14:10:05.366061993 -0400
>> +++ b/arch/x86/kvm/emulate.c 2015-08-29 13:43:13.014163958 -0400
>> @@ -650,6 +650,7 @@ static __always_inline int __linearize(s
>> u16 sel;
>>
>> la = seg_base(ctxt, addr.seg) + addr.ea;
>> + *linear = la;
>> *max_size = 0;
>> switch (mode) {
>> case X86EMUL_MODE_PROT64:
>> @@ -693,7 +694,6 @@ static __always_inline int __linearize(s
>> }
>> if (insn_aligned(ctxt, size) && ((la & (size - 1)) != 0))
>> return emulate_gp(ctxt, 0);
>> - *linear = la;
>> return X86EMUL_CONTINUE;
>> bad:
>> if (addr.seg == VCPU_SREG_SS)
>>
>
> Unfortunately this patch broke GNU/Hurd when running under KVM. It fails
> to boot almost immediately. I haven't debug it more, but it looks like
> *linear should not always be written. This can easily be reproduced by
> trying to boot Debian Installer from this ISO:
>
> http://ftp.debian-ports.org/debian-cd/hurd-i386/debian-hurd-2015/debian-hurd-2015-i386-CD-1.iso
The bug is that la can be changed by the "la &= (u32)-1" line.
So the fix could be like:
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 1505587d06e9..b9b09fec173b 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -650,10 +650,10 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt,
u16 sel;
la = seg_base(ctxt, addr.seg) + addr.ea;
- *linear = la;
*max_size = 0;
switch (mode) {
case X86EMUL_MODE_PROT64:
+ *linear = la;
if (is_noncanonical_address(la))
goto bad;
@@ -662,6 +662,7 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt,
goto bad;
break;
default:
+ *linear = la = (u32)la;
usable = ctxt->ops->get_segment(ctxt, &sel, &desc, NULL,
addr.seg);
if (!usable)
@@ -689,7 +690,6 @@ static __always_inline int __linearize(struct x86_emulate_ctxt *ctxt,
if (size > *max_size)
goto bad;
}
- la &= (u32)-1;
break;
}
if (insn_aligned(ctxt, size) && ((la & (size - 1)) != 0))
Can you test it?
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
