* [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper
@ 2020-11-13 9:56 Bin Meng
2020-11-13 10:18 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Bin Meng @ 2020-11-13 9:56 UTC (permalink / raw)
To: Paolo Bonzini, Eduardo Habkost, Richard Henderson, qemu-devel; +Cc: Bin Meng
From: Bin Meng <bin.meng@windriver.com>
Per the SDM, when returning to outer privilege level, for segment
registers (ES, FS, GS, and DS) if the check fails, the segment
selector becomes null, but QEMU clears the base/limit/flags as well
as nullifying the segment selector, which should be a spec violation.
Real hardware seems to be compliant with the spec, at least on one
Coffee Lake board I tested.
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---
Changes in v2:
- clearing the DESC_P bit in the segment descriptor
target/i386/seg_helper.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c
index be88938..d539573 100644
--- a/target/i386/seg_helper.c
+++ b/target/i386/seg_helper.c
@@ -2108,7 +2108,10 @@ static inline void validate_seg(CPUX86State *env, int seg_reg, int cpl)
if (!(e2 & DESC_CS_MASK) || !(e2 & DESC_C_MASK)) {
/* data or non conforming code segment */
if (dpl < cpl) {
- cpu_x86_load_seg_cache(env, seg_reg, 0, 0, 0, 0);
+ cpu_x86_load_seg_cache(env, seg_reg, 0,
+ env->segs[seg_reg].base,
+ env->segs[seg_reg].limit,
+ env->segs[seg_reg].flags & ~DESC_P_MASK);
}
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper
2020-11-13 9:56 [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper Bin Meng
@ 2020-11-13 10:18 ` Paolo Bonzini
2020-11-13 10:23 ` Bin Meng
2020-11-16 12:48 ` Philippe Mathieu-Daudé
0 siblings, 2 replies; 9+ messages in thread
From: Paolo Bonzini @ 2020-11-13 10:18 UTC (permalink / raw)
To: Bin Meng, Eduardo Habkost, Richard Henderson, qemu-devel; +Cc: Bin Meng
On 13/11/20 10:56, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
>
> Per the SDM, when returning to outer privilege level, for segment
> registers (ES, FS, GS, and DS) if the check fails, the segment
> selector becomes null, but QEMU clears the base/limit/flags as well
> as nullifying the segment selector, which should be a spec violation.
>
> Real hardware seems to be compliant with the spec, at least on one
> Coffee Lake board I tested.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>
> ---
>
> Changes in v2:
> - clearing the DESC_P bit in the segment descriptor
>
> target/i386/seg_helper.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c
> index be88938..d539573 100644
> --- a/target/i386/seg_helper.c
> +++ b/target/i386/seg_helper.c
> @@ -2108,7 +2108,10 @@ static inline void validate_seg(CPUX86State *env, int seg_reg, int cpl)
> if (!(e2 & DESC_CS_MASK) || !(e2 & DESC_C_MASK)) {
> /* data or non conforming code segment */
> if (dpl < cpl) {
> - cpu_x86_load_seg_cache(env, seg_reg, 0, 0, 0, 0);
> + cpu_x86_load_seg_cache(env, seg_reg, 0,
> + env->segs[seg_reg].base,
> + env->segs[seg_reg].limit,
> + env->segs[seg_reg].flags & ~DESC_P_MASK);
> }
> }
> }
>
Queued, thanks. It would be nicer if the commit message explained how
the guest can notice the difference.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper
2020-11-13 10:18 ` Paolo Bonzini
@ 2020-11-13 10:23 ` Bin Meng
2020-11-13 10:39 ` Paolo Bonzini
2020-11-16 12:48 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 9+ messages in thread
From: Bin Meng @ 2020-11-13 10:23 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Bin Meng, Richard Henderson, Eduardo Habkost,
qemu-devel@nongnu.org Developers
Hi Paolo,
On Fri, Nov 13, 2020 at 6:18 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 13/11/20 10:56, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > Per the SDM, when returning to outer privilege level, for segment
> > registers (ES, FS, GS, and DS) if the check fails, the segment
> > selector becomes null, but QEMU clears the base/limit/flags as well
> > as nullifying the segment selector, which should be a spec violation.
> >
> > Real hardware seems to be compliant with the spec, at least on one
> > Coffee Lake board I tested.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >
> > ---
> >
> > Changes in v2:
> > - clearing the DESC_P bit in the segment descriptor
> >
> > target/i386/seg_helper.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c
> > index be88938..d539573 100644
> > --- a/target/i386/seg_helper.c
> > +++ b/target/i386/seg_helper.c
> > @@ -2108,7 +2108,10 @@ static inline void validate_seg(CPUX86State *env, int seg_reg, int cpl)
> > if (!(e2 & DESC_CS_MASK) || !(e2 & DESC_C_MASK)) {
> > /* data or non conforming code segment */
> > if (dpl < cpl) {
> > - cpu_x86_load_seg_cache(env, seg_reg, 0, 0, 0, 0);
> > + cpu_x86_load_seg_cache(env, seg_reg, 0,
> > + env->segs[seg_reg].base,
> > + env->segs[seg_reg].limit,
> > + env->segs[seg_reg].flags & ~DESC_P_MASK);
> > }
> > }
> > }
> >
>
> Queued, thanks.
Thanks!
> It would be nicer if the commit message explained how
> the guest can notice the difference.
The commit message says "Per the SDM" :) The actual failure case
involves a special code sequence that is exposed in VxWorks guest
testing. Linux does not expose this however.
Regards,
Bin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper
2020-11-13 10:23 ` Bin Meng
@ 2020-11-13 10:39 ` Paolo Bonzini
2020-11-17 10:08 ` Bin Meng
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2020-11-13 10:39 UTC (permalink / raw)
To: Bin Meng
Cc: Bin Meng, Richard Henderson, Eduardo Habkost,
qemu-devel@nongnu.org Developers
On 13/11/20 11:23, Bin Meng wrote:
>> It would be nicer if the commit message explained how
>> the guest can notice the difference.
>
> The commit message says "Per the SDM" :) The actual failure case
> involves a special code sequence that is exposed in VxWorks guest
> testing. Linux does not expose this however.
I see. Is there any chance you could write a testcase for
kvm-unit-tests? Or just explain how to write such a test, and then I
can write it myself; it's not clear to me how the guest can observe the
base and limit of a non-present segment.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper
2020-11-13 10:18 ` Paolo Bonzini
2020-11-13 10:23 ` Bin Meng
@ 2020-11-16 12:48 ` Philippe Mathieu-Daudé
2020-11-30 14:01 ` Bin Meng
1 sibling, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-16 12:48 UTC (permalink / raw)
To: Paolo Bonzini, Bin Meng, Eduardo Habkost, Richard Henderson, qemu-devel
Cc: Bin Meng
On 11/13/20 11:18 AM, Paolo Bonzini wrote:
> On 13/11/20 10:56, Bin Meng wrote:
...
>
> Queued, thanks. It would be nicer if the commit message explained how
> the guest can notice the difference.
Typo "segement" -> "segment" in subject.
>
> Paolo
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper
2020-11-13 10:39 ` Paolo Bonzini
@ 2020-11-17 10:08 ` Bin Meng
2020-11-17 11:06 ` Paolo Bonzini
0 siblings, 1 reply; 9+ messages in thread
From: Bin Meng @ 2020-11-17 10:08 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Bin Meng, Richard Henderson, Eduardo Habkost,
qemu-devel@nongnu.org Developers
Hi Paolo,
On Fri, Nov 13, 2020 at 6:39 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 13/11/20 11:23, Bin Meng wrote:
> >> It would be nicer if the commit message explained how
> >> the guest can notice the difference.
> >
> > The commit message says "Per the SDM" :) The actual failure case
> > involves a special code sequence that is exposed in VxWorks guest
> > testing. Linux does not expose this however.
>
> I see. Is there any chance you could write a testcase for
> kvm-unit-tests? Or just explain how to write such a test, and then I
> can write it myself; it's not clear to me how the guest can observe the
> base and limit of a non-present segment.
I am not familiar with kvm-unit-test. The original issue cannot be
reproduced with a KVM enabled QEMU as the codes-in-flaw is in the
emulation path.
Regards,
Bin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper
2020-11-17 10:08 ` Bin Meng
@ 2020-11-17 11:06 ` Paolo Bonzini
2020-11-24 6:09 ` Bin Meng
0 siblings, 1 reply; 9+ messages in thread
From: Paolo Bonzini @ 2020-11-17 11:06 UTC (permalink / raw)
To: Bin Meng
Cc: Bin Meng, Richard Henderson, Eduardo Habkost,
qemu-devel@nongnu.org Developers
On 17/11/20 11:08, Bin Meng wrote:
>> I see. Is there any chance you could write a testcase for
>> kvm-unit-tests? Or just explain how to write such a test, and then I
>> can write it myself; it's not clear to me how the guest can observe the
>> base and limit of a non-present segment.
>
> I am not familiar with kvm-unit-test. The original issue cannot be
> reproduced with a KVM enabled QEMU as the codes-in-flaw is in the
> emulation path.
kvm-unit-tests, despite the name, is a set generic tests for CPU
behavior; it works with other accelerators that QEMU supports including
the emulation path. You can find it at
https://gitlab.com/kvm-unit-tests/kvm-unit-tests.
If you explain in enough detail how VxWorks triggers the bug, I can take
care of writing the test.
Paolo
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper
2020-11-17 11:06 ` Paolo Bonzini
@ 2020-11-24 6:09 ` Bin Meng
0 siblings, 0 replies; 9+ messages in thread
From: Bin Meng @ 2020-11-24 6:09 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Bin Meng, Richard Henderson, Eduardo Habkost,
qemu-devel@nongnu.org Developers
Hi Paolo,
On Tue, Nov 17, 2020 at 7:06 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 17/11/20 11:08, Bin Meng wrote:
> >> I see. Is there any chance you could write a testcase for
> >> kvm-unit-tests? Or just explain how to write such a test, and then I
> >> can write it myself; it's not clear to me how the guest can observe the
> >> base and limit of a non-present segment.
> >
> > I am not familiar with kvm-unit-test. The original issue cannot be
> > reproduced with a KVM enabled QEMU as the codes-in-flaw is in the
> > emulation path.
>
> kvm-unit-tests, despite the name, is a set generic tests for CPU
> behavior; it works with other accelerators that QEMU supports including
> the emulation path. You can find it at
> https://gitlab.com/kvm-unit-tests/kvm-unit-tests.
I see. Thanks for the info.
> If you explain in enough detail how VxWorks triggers the bug, I can take
> care of writing the test.
I will try to create a test case using the kvm-unit-tests framework.
Regards,
Bin
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper
2020-11-16 12:48 ` Philippe Mathieu-Daudé
@ 2020-11-30 14:01 ` Bin Meng
0 siblings, 0 replies; 9+ messages in thread
From: Bin Meng @ 2020-11-30 14:01 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Bin Meng, Richard Henderson, Eduardo Habkost,
qemu-devel@nongnu.org Developers
On Mon, Nov 16, 2020 at 8:48 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 11/13/20 11:18 AM, Paolo Bonzini wrote:
> > On 13/11/20 10:56, Bin Meng wrote:
> ...
> >
> > Queued, thanks. It would be nicer if the commit message explained how
> > the guest can notice the difference.
>
> Typo "segement" -> "segment" in subject.
Thanks Philippe.
Paolo, please let me know if you need me to respin this patch, or you
can fix this when applying.
Regards,
Bin
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-11-30 14:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13 9:56 [PATCH v2] target/i386: seg_helper: Correct segement selector nullification in the RET/IRET helper Bin Meng
2020-11-13 10:18 ` Paolo Bonzini
2020-11-13 10:23 ` Bin Meng
2020-11-13 10:39 ` Paolo Bonzini
2020-11-17 10:08 ` Bin Meng
2020-11-17 11:06 ` Paolo Bonzini
2020-11-24 6:09 ` Bin Meng
2020-11-16 12:48 ` Philippe Mathieu-Daudé
2020-11-30 14:01 ` Bin Meng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).