patch for a crash in x86 emulator
Hi,
I've been running XFree86 4.3.0.1 on a machine with a particularly
weird videocard and I've got the VESA driver craching. If anyone
wonders, the BIOS data in the log is:
(II) VESA(0): VESA BIOS detected
(II) VESA(0): VESA VBE Version 3.0
(II) VESA(0): VESA VBE Total Mem: 1024 kB
(II) VESA(0): VESA VBE OEM: Intel(R) 810, Intel(R) 815 Chipset Video BIOS
(II) VESA(0): VESA VBE OEM Software Rev: 37.16
(II) VESA(0): VESA VBE OEM Vendor: Intel Corporation
(II) VESA(0): VESA VBE OEM Product: Intel(R) 810, Intel(R) 815 Chipset
(II) VESA(0): VESA VBE OEM Product Rev: Hardware Version 0.0
The machine is a retail box with embedded LCD touch-screen.
A little investigation has shown that it crashes in
x86emuOp_mov_word_SR_RM() (in programs/Xserver/hw/xfree86/int10/ops.c
which is symlinked from extras/x86emu/src/x86emu) on the underlined line:
case 3: /* register to register */
destreg = decode_rm_seg_register(rh);
DECODE_PRINTF(",");
srcreg = DECODE_RM_WORD_REGISTER(rl);
DECODE_PRINTF("\n");
TRACE_AND_STEP();
*destreg = *srcreg;
^^
break;
This happens because destreg is 0, because it's returned that way
from decode_rm_seg_register(). rh is 4, and that's the value that
decode_rm_seg_register() in decode.c (also linked from extras) does
not understand. I've looked it up in the manual and actually
the value 4 is for FS and value 5 is for GS. So here is the
patch (also attached, to avoid corruption by the mailer):
-- cut here ---
*** decode.c.0 Thu Dec 4 15:42:51 2003
--- decode.cThu Dec 4 17:15:29 2003
***
*** 699,705
--- 699,709
DECODE_PRINTF("DS");
return &M.x86.R_DS;
case 4:
+ DECODE_PRINTF("FS");
+ return &M.x86.R_FS;
case 5:
+ DECODE_PRINTF("GS");
+ return &M.x86.R_GS;
case 6:
case 7:
DECODE_PRINTF("ILLEGAL SEGREG");
-- cut here ---
This patch made the X server to work with this card.
Could someone please check it in? I've looked in CVS and the most
recent version still has this bug in it.
There is also a more general question: in case when an instruction opcode
can't be decoded, many routines in decode.c rely on just calling
HALT_SYS() for error handling. However HALT_SYS() expands to
X86EMU_halt_sys() which does nothing but sets a flag. The decode
routines return 0 instead of all kinds of pointers to their caller
which would immediately try to reference that pointer and crash.
That means that any misformatted routine met in BIOS will make the
X server to crash. I think that it's not good. I think that in case
of a bad opcode an error message has to be printed into the log,
so that the user would have some clue, and then the decoders must
return not NULL pointer but a valid pointer to some trash value,
so that their caller would be able to reference it without crashing
before the interpreter has a chance to halt.
If I make a patch to fix it as described, would anyone be willing to
review and commit it? (BTW, any pointers to how to print an error
message properly from the bowels of the i86 interpreters would be
appreciated too).
-SB
decode.df.gz
Description: GNU Zip compressed data
Re: patch for a crash in x86 emulator
On Thu, 04 Dec 2003 20:35:02 -0500, Sergey Babkin wrote: > >I've been running XFree86 4.3.0.1 on a machine with a particularly >weird videocard and I've got the VESA driver craching. >... >A little investigation has shown that it crashes in >x86emuOp_mov_word_SR_RM() >... >This happens because destreg is 0, because it's returned that way >from decode_rm_seg_register(). rh is 4, and that's the value that >decode_rm_seg_register() in decode.c (also linked from extras) does >not understand. I've looked it up in the manual and actually >the value 4 is for FS and value 5 is for GS. So, the conclusion here is that the Intel 815 BIOS uses FS and GS? That is both surprising and disturbing. The BIOS runs in real/v86 mode, and is not generally allowed to make any assumptions about the addressing in the machine. XFree86 doesn't map in any low-memory sections other than the segments at 0, A, B, and C. Given that, it is hard to imagine a scenario where DS and ES are not completely sufficient to do the job. Do you know what call is being made at the time of the crash? -- - Tim Roberts, [EMAIL PROTECTED] Providenza & Boekelheide, Inc. ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: patch for a crash in x86 emulator
On Thu, Dec 04, 2003 at 05:57:44PM -0800, Tim Roberts wrote: >On Thu, 04 Dec 2003 20:35:02 -0500, Sergey Babkin wrote: >> >>I've been running XFree86 4.3.0.1 on a machine with a particularly >>weird videocard and I've got the VESA driver craching. >>... >>A little investigation has shown that it crashes in >>x86emuOp_mov_word_SR_RM() >>... >>This happens because destreg is 0, because it's returned that way >>from decode_rm_seg_register(). rh is 4, and that's the value that >>decode_rm_seg_register() in decode.c (also linked from extras) does >>not understand. I've looked it up in the manual and actually >>the value 4 is for FS and value 5 is for GS. > >So, the conclusion here is that the Intel 815 BIOS uses FS and GS? That >is both surprising and disturbing. > >The BIOS runs in real/v86 mode, and is not generally allowed to make any >assumptions about the addressing in the machine. XFree86 doesn't map in >any low-memory sections other than the segments at 0, A, B, >and C. Given that, it is hard to imagine a scenario where DS and ES >are not completely sufficient to do the job. I have an Intel 810 with a BIOS that uses some 32-bit addressing modes that I hadn't seen used in other BIOSes. I put a bunch of updates for those into our emulator about a year ago. I haven't come across them using FS or GS though, but I'm not as surprised as I once would have been. David -- David Dawes developer/release engineer The XFree86 Project www.XFree86.org/~dawes ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: patch for a crash in x86 emulator
On Thu, Dec 04, 2003 at 08:35:02PM -0500, Sergey Babkin wrote:
>Hi,
>
>I've been running XFree86 4.3.0.1 on a machine with a particularly
>weird videocard and I've got the VESA driver craching. If anyone
>wonders, the BIOS data in the log is:
>
>(II) VESA(0): VESA BIOS detected
>(II) VESA(0): VESA VBE Version 3.0
>(II) VESA(0): VESA VBE Total Mem: 1024 kB
>(II) VESA(0): VESA VBE OEM: Intel(R) 810, Intel(R) 815 Chipset Video BIOS
>(II) VESA(0): VESA VBE OEM Software Rev: 37.16
>(II) VESA(0): VESA VBE OEM Vendor: Intel Corporation
>(II) VESA(0): VESA VBE OEM Product: Intel(R) 810, Intel(R) 815 Chipset
>(II) VESA(0): VESA VBE OEM Product Rev: Hardware Version 0.0
>
>The machine is a retail box with embedded LCD touch-screen.
>
>A little investigation has shown that it crashes in
>x86emuOp_mov_word_SR_RM() (in programs/Xserver/hw/xfree86/int10/ops.c
>which is symlinked from extras/x86emu/src/x86emu) on the underlined line:
>
>case 3: /* register to register */
>destreg = decode_rm_seg_register(rh);
>DECODE_PRINTF(",");
>srcreg = DECODE_RM_WORD_REGISTER(rl);
>DECODE_PRINTF("\n");
>TRACE_AND_STEP();
>*destreg = *srcreg;
> ^^
> break;
>
>This happens because destreg is 0, because it's returned that way
>from decode_rm_seg_register(). rh is 4, and that's the value that
>decode_rm_seg_register() in decode.c (also linked from extras) does
>not understand. I've looked it up in the manual and actually
>the value 4 is for FS and value 5 is for GS. So here is the
>patch (also attached, to avoid corruption by the mailer):
>
>-- cut here ---
>*** decode.c.0 Thu Dec 4 15:42:51 2003
>--- decode.cThu Dec 4 17:15:29 2003
>***
>*** 699,705
>--- 699,709
>DECODE_PRINTF("DS");
>return &M.x86.R_DS;
> case 4:
>+ DECODE_PRINTF("FS");
>+ return &M.x86.R_FS;
> case 5:
>+ DECODE_PRINTF("GS");
>+ return &M.x86.R_GS;
> case 6:
> case 7:
>DECODE_PRINTF("ILLEGAL SEGREG");
>-- cut here ---
>
>This patch made the X server to work with this card.
>Could someone please check it in? I've looked in CVS and the most
>recent version still has this bug in it.
I'll commit this now -- thanks.
>There is also a more general question: in case when an instruction opcode
>can't be decoded, many routines in decode.c rely on just calling
>HALT_SYS() for error handling. However HALT_SYS() expands to
>X86EMU_halt_sys() which does nothing but sets a flag. The decode
>routines return 0 instead of all kinds of pointers to their caller
>which would immediately try to reference that pointer and crash.
>That means that any misformatted routine met in BIOS will make the
>X server to crash. I think that it's not good. I think that in case
>of a bad opcode an error message has to be printed into the log,
>so that the user would have some clue, and then the decoders must
>return not NULL pointer but a valid pointer to some trash value,
>so that their caller would be able to reference it without crashing
>before the interpreter has a chance to halt.
Yes, that is a problem. The INTR_HALTED flag isn't tested until too
late. The potential problem of returning a trash value is that could
cause bad things too. Maybe the callers of functions that can return
NULL need to check for that, and abort the instruction? Then INTR_HALTED
will be noticed before moving on to the next instruction.
>If I make a patch to fix it as described, would anyone be willing to
>review and commit it? (BTW, any pointers to how to print an error
>message properly from the bowels of the i86 interpreters would be
>appreciated too).
Yes.
Use "printk". It should get supplied by whatever uses the emulator.
For the XFree86 server it provided as a function that calls VErrorF(),
the generic logging function.
David
--
David Dawes
developer/release engineer The XFree86 Project
www.XFree86.org/~dawes
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel
Re: patch for a crash in x86 emulator
Oops, somehow I've thought that I was subscribed to the list but it iurns that I was not - I guess I've stopped it before vacation and forgot to turn back on. So sorry about the delay. >>I've been running XFree86 4.3.0.1 on a machine with a particularly >>weird videocard and I've got the VESA driver craching. >>... >>A little investigation has shown that it crashes in >>x86emuOp_mov_word_SR_RM() >>... >>This happens because destreg is 0, because it's returned that way >>from decode_rm_seg_register(). rh is 4, and that's the value that >>decode_rm_seg_register() in decode.c (also linked from extras) does >>not understand. I've looked it up in the manual and actually >>the value 4 is for FS and value 5 is for GS. > >So, the conclusion here is that the Intel 815 BIOS uses FS and GS? That >is both surprising and disturbing. Maybe it's only some particular version. Because of 5 types of machines I have with o810-i815-i845 this is the only one that caused a crash. >The BIOS runs in real/v86 mode, and is not generally allowed to make any >assumptions about the addressing in the machine. XFree86 doesn't map in >any low-memory sections other than the segments at 0, A, B, >and C. Given that, it is hard to imagine a scenario where DS and ES >are not completely sufficient to do the job. Well, with 4 potential segments using 4 segment registers makes sense. If it's of any interest, the value that it tried to load into FS when crashed was 0x0100. BTW, the emulator supports FS/GS throughout most of it, just it was missing in the R/M decoding. -SB ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: patch for a crash in x86 emulator
> when crashed was 0x0100. BTW, the emulator supports FS/GS throughout > most of it, just it was missing in the R/M decoding. Oh, and the call made was setting the video mode. It crashed in a very unpleasant way too, leaving the screen in a dead state. I have more details written down at work, if you are interested. -SB ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: patch for a crash in x86 emulator
David Dawes wrote: >>return not NULL pointer but a valid pointer to some trash value, >>so that their caller would be able to reference it without crashing >>before the interpreter has a chance to halt. > >Yes, that is a problem. The INTR_HALTED flag isn't tested until too >late. The potential problem of returning a trash value is that could >cause bad things too. Maybe the callers of functions that can return >NULL need to check for that, and abort the instruction? Then >INTR_HALTED >will be noticed before moving on to the next instruction. Maybe a better way would be to check in the callers for INTR_HALTED and return immediately. This should handle nested calls relatively easily, with the same thing done in each function. Otherwise if there are nested calls the inner funtions will need to devise some special values to indicate the abort to their callers too. Thanks for the printk() pointer! -SB ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: patch for a crash in x86 emulator
On Sat, Dec 06, 2003 at 07:47:30PM -0500, Sergey Babkin wrote: >David Dawes wrote: > >>>return not NULL pointer but a valid pointer to some trash value, >>>so that their caller would be able to reference it without crashing >>>before the interpreter has a chance to halt. >> >>Yes, that is a problem. The INTR_HALTED flag isn't tested until too >>late. The potential problem of returning a trash value is that could >>cause bad things too. Maybe the callers of functions that can return >>NULL need to check for that, and abort the instruction? Then >INTR_HALTED >>will be noticed before moving on to the next instruction. > >Maybe a better way would be to check in the callers for INTR_HALTED >and return immediately. This should handle nested calls relatively >easily, with the same thing done in each function. Otherwise >if there are nested calls the inner funtions will need to devise >some special values to indicate the abort to their callers too. That's a good point. It would handle it nice and consistently. David -- David Dawes developer/release engineer The XFree86 Project www.XFree86.org/~dawes ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
Re: patch for a crash in x86 emulator
Sergey Babkin <[EMAIL PROTECTED]> wrote:
> -- cut here ---
> *** decode.c.0 Thu Dec 4 15:42:51 2003
> --- decode.cThu Dec 4 17:15:29 2003
> ***
> *** 699,705
> --- 699,709
> DECODE_PRINTF("DS");
> return &M.x86.R_DS;
> case 4:
> + DECODE_PRINTF("FS");
> + return &M.x86.R_FS;
> case 5:
> + DECODE_PRINTF("GS");
> + return &M.x86.R_GS;
> case 6:
> case 7:
> DECODE_PRINTF("ILLEGAL SEGREG");
> -- cut here ---
>
> This patch made the X server to work with this card.
Thanks for the update. I have patched the official x86emu sources hosted
in our Perforce server, and now is probably a good time for Egbert and I
to try and sync up our two versions again. Patches have probably been
made to the XFree86 version and over the last few months we made a bunch
of patches to our version as well.
Ebgert, do you want to work with me to get this done? Clearly I don't
have CVS commit access so I can't get stuff into XFree86, but I can work
on merging the XFree86 patches that are not in our tree (assuming we both
didn't patch the same bug ;-) so we can get back to a common version
again.
Regards,
---
Kendall Bennett
Chief Executive Officer
SciTech Software, Inc.
Phone: (530) 894 8400
http://www.scitechsoft.com
~ SciTech SNAP - The future of device driver technology! ~
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel
Re: patch for a crash in x86 emulator
On Mon, Dec 08, 2003 at 03:17:20PM -0800, Kendall Bennett wrote:
> Sergey Babkin <[EMAIL PROTECTED]> wrote:
>
> > -- cut here ---
> > *** decode.c.0 Thu Dec 4 15:42:51 2003
> > --- decode.cThu Dec 4 17:15:29 2003
> > ***
> > *** 699,705
> > --- 699,709
> > DECODE_PRINTF("DS");
> > return &M.x86.R_DS;
> > case 4:
> > + DECODE_PRINTF("FS");
> > + return &M.x86.R_FS;
> > case 5:
> > + DECODE_PRINTF("GS");
> > + return &M.x86.R_GS;
> > case 6:
> > case 7:
> > DECODE_PRINTF("ILLEGAL SEGREG");
> > -- cut here ---
> >
> > This patch made the X server to work with this card.
>
> Thanks for the update. I have patched the official x86emu sources hosted
> in our Perforce server, and now is probably a good time for Egbert and I
> to try and sync up our two versions again. Patches have probably been
> made to the XFree86 version and over the last few months we made a bunch
> of patches to our version as well.
>
> Ebgert, do you want to work with me to get this done? Clearly I don't
> have CVS commit access so I can't get stuff into XFree86, but I can work
> on merging the XFree86 patches that are not in our tree (assuming we both
> didn't patch the same bug ;-) so we can get back to a common version
> again.
Kendall,
Just do a diff and submit it to bugzilla. Then you don't have to rely on
Egbert being around and anyone of the XFree86 committers can take a look
at it.
Alan.
___
Devel mailing list
[EMAIL PROTECTED]
http://XFree86.Org/mailman/listinfo/devel
Re: patch for a crash in x86 emulator
Alan Hourihane <[EMAIL PROTECTED]> wrote: > Just do a diff and submit it to bugzilla. Then you don't have to > rely on Egbert being around and anyone of the XFree86 committers > can take a look at it. Ok, I will give that a shot. Can someone shoot me a copy of the latest sourece from CVS for just the x86 emulator? It will take a while for me to sync up to the entire tree. Regards, --- Kendall Bennett Chief Executive Officer SciTech Software, Inc. Phone: (530) 894 8400 http://www.scitechsoft.com ~ SciTech SNAP - The future of device driver technology! ~ ___ Devel mailing list [EMAIL PROTECTED] http://XFree86.Org/mailman/listinfo/devel
