Il 10/10/2014 17:54, Radim Krčmář ha scritto:
>> >
>> > One exception is the case of conforming code segment. The SDM says: "Use a
>> > code-segment override prefix (CS) to read a readable... [it is] valid because
>> > the DPL of the code segment selected by the CS register is the same as the
>> > CPL." This is misleading since CS.DPL may be lower (numerically) than CPL, and
>> > CS would still be accessible. The emulator should avoid privilage level checks
>> > for data reads using CS.
> Ah, after stripping faulty presumptions, I'm not sure this change is
> enough ... shouldn't we also skip the check on conforming code segments?
>
> Method 2 is always valid because the privilege level of a conforming
> code segment is effectively the same as the CPL, regardless of its DPL.
Radim is right; we need to skip the check on conforming code segments
and, once we do that, checking addr.seg is not necessary anymore. That
is because, for a CS override on a nonconforming code segment, at the
time we fetch the instruction we know that cpl == desc.dpl. The less
restrictive data segment check (cpl <= desc.dpl) thus always passes.
Let's put together this check and the readability check, too, since
we are adding another "if (fetch)".
Can you guys think of a way to simplify the following untested patch?
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 03954f7900f5..9f3e33551db9 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -638,9 +638,6 @@ static int __linearize(struct x86_emulate_ctxt *ctxt,
if ((((ctxt->mode != X86EMUL_MODE_REAL) && (desc.type & 8))
|| !(desc.type & 2)) && write)
goto bad;
- /* unreadable code segment */
- if (!fetch && (desc.type & 8) && !(desc.type & 2))
- goto bad;
lim = desc_limit_scaled(&desc);
if ((ctxt->mode == X86EMUL_MODE_REAL) && !fetch &&
(ctxt->d & NoBigReal)) {
@@ -660,17 +657,40 @@ static int __linearize(struct x86_emulate_ctxt *ctxt,
goto bad;
}
cpl = ctxt->ops->cpl(ctxt);
- if (!(desc.type & 8)) {
- /* data segment */
+ if (fetch && (desc.type & 8)) {
+ if (!(desc.type & 4)) {
+ /* nonconforming code segment */
+ if (cpl != desc.dpl)
+ goto bad;
+ break;
+ } else {
+ /* conforming code segment */
+ if (cpl < desc.dpl)
+ goto bad;
+ break;
+ }
+ }
+
+ if (likely(!(desc.type & 8) || (desc.type & 6) == 2)) {
+ /*
+ * Data segment or readable, nonconforming code
+ * segment. The SDM mentions that access through
+ * a code-segment override prefix is always valid.
+ * This really only matters for conforming code
+ * segments (checked below, and always valid anyway):
+ * for nonconforming ones, cpl == desc.dpl was checked
+ * when fetching the instruction, meaning the following
+ * test will always pass too.
+ */
if (cpl > desc.dpl)
goto bad;
- } else if ((desc.type & 8) && !(desc.type & 4)) {
- /* nonconforming code segment */
- if (cpl != desc.dpl)
- goto bad;
- } else if ((desc.type & 8) && (desc.type & 4)) {
- /* conforming code segment */
- if (cpl < desc.dpl)
+ } else {
+ /*
+ * These are the (rare) cases that do not behave
+ * like data segments: nonreadable code segments (bad)
+ * and readable, conforming code segments (good).
+ */
+ if (!(desc.type & 2))
goto bad;
}
break;
--
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
