On 24/04/2025 2:13 pm, Fabian Specht wrote:
>>> if ( !s->rex_prefix )
>>> {
>>> /* Convert 32-bit real/vm86 to 32-bit prot format. */
>>> unsigned int fip = fpstate.env.mode.real.fip_lo +
>>> (fpstate.env.mode.real.fip_hi << 16);
>>> unsigned int fdp = fpstate.env.mode.real.fdp_lo +
>>> (fpstate.env.mode.real.fdp_hi << 16);
>>> unsigned int fop = fpstate.env.mode.real.fop;
>>>
>>> fpstate.env.mode.prot.fip = fip & 0xf;
>>> fpstate.env.mode.prot.fcs = fip >> 4;
>>> fpstate.env.mode.prot.fop = fop;
>>> fpstate.env.mode.prot.fdp = fdp & 0xf;
>>> fpstate.env.mode.prot.fds = fdp >> 4;
>>> }
>> Several things. First, please always the UBSAN analysis from the crash.
> (XEN) UBSAN: Undefined behaviour in arch/x86/x86_emulate/blk.c:87:66
> (XEN) left shift of 65535 by 16 places cannot be represented in type 'int'
> (XEN) ----[ Xen-4.20.0 x86_64 debug=y ubsan=y Tainted: H ]----
Yes, it is a shift into the sign bit.
>
>> There are several different ways that shifts go wrong, and I suspect
>> this is a shift into a sign bit, which is notable given the unsigned
>> underlying type.
> Might be, but I am not entirely sure. Either way, it should be fixed
> through a simple cast to unsigned int I think.
I have a sneaking suspicion that this is sufficient:
diff --git a/xen/arch/x86/x86_emulate/private.h
b/xen/arch/x86/x86_emulate/private.h
index 30be59547032..9f3d6f0e5357 100644
--- a/xen/arch/x86/x86_emulate/private.h
+++ b/xen/arch/x86/x86_emulate/private.h
@@ -385,9 +385,9 @@ struct x87_env16 {
union {
struct {
uint16_t fip_lo;
- uint16_t fop:11, :1, fip_hi:4;
+ uint32_t fop:11, :1, fip_hi:4;
uint16_t fdp_lo;
- uint16_t :12, fdp_hi:4;
+ uint32_t :12, fdp_hi:4;
} real;
struct {
uint16_t fip;
The problem is that a uint16_t bitfield promotes into int. A base type
of uint32_t should cause the bitfield to promote into unsigned int directly.
>
>> Also, are you aware that the test isn't properly in Real Mode? It's in
>> so-called unreal mode (not actually a real mode, but a consequence of
>> how the segment registers work), which is relevant to how you manage to
>> re-enter the emulator for FLDENV.
> Yes I am aware. But the bug should be triggered regardless of the
> current mode, right?
Well, that depends. Some CPUs trucate %eip to %ip at this point. Xen
doesn't, which is why the XTF test doesn't crash in a different way.
~Andrew
