d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use the MI macro TRAPF_USERMODE() instead of open-coded checks for

Browse Source 
SEL_UPL and sometimes PSL_VM.  This is just a style change on amd64,
but on i386 it fixes 1 unimportant place where the PSL_VM check was
missing and starts fixing 1 important place where the PSL_VM check
had a logic error.

Fix logic errors in treating vm86 bioscall mode as kernel mode.  The
main place checked all the necessary flags, but put the necessary
parentheses for the PSL_VM and PCB_VM86CALL checks in the wrong
place.  The broken case is only reached if a vm86 bioscall uses a
%cs which is nonzero mod 4, but that is unusual -- most bios calls
start with %cs = 0xc000 or 0xf000 and rarely change it.  Another
place was missing the check for PCB_VM86CALL, but was only reachable
if there are bugs virtualizing PSL_I.

Add a macro TF_HAS_STACKREGS() and use this instead of converting
open-coded checks of SEL_UPL, etc. to TRAPF_USERMODE() when we only
care about whether the frame has stack registers.  This fixes 3
places in my recent fix for register variables in vm86 mode where I
messed up the PSL_VM check and cleans up other places.
This commit is contained in:
bde 2016-09-14 12:57:40 +00:00
parent 386ddae584
commit d58cd5baa4
4 changed files with 30 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
sys/amd64/amd64/trap.c
View File

@ -236,7 +236,7 @@ trap(struct trapframe *frame)
* interrupts disabled until they are accidentally
* enabled later.
*/
if (ISPL(frame->tf_cs) == SEL_UPL)
if (TRAPF_USERMODE(frame))
uprintf(
"pid %ld (%s): trap %d with interrupts disabled\n",
(long)curproc->p_pid, curthread->td_name, type);
@ -260,7 +260,7 @@ trap(struct trapframe *frame)
code = frame->tf_err;
if (ISPL(frame->tf_cs) == SEL_UPL) {
if (TRAPF_USERMODE(frame)) {
/* user trap */
td->td_pticks = 0;
@ -787,7 +787,7 @@ trap_fatal(frame, eva)
else
msg = "UNKNOWN";
printf("\n\nFatal trap %d: %s while in %s mode\n", type, msg,
ISPL(frame->tf_cs) == SEL_UPL ? "user" : "kernel");
TRAPF_USERMODE(frame) ? "user" : "kernel");
#ifdef SMP
/* two separate prints in case of a trap on an unmapped page */
printf("cpuid = %d; ", PCPU_GET(cpuid));
@ -804,7 +804,7 @@ trap_fatal(frame, eva)
}
printf("instruction pointer = 0x%lx:0x%lx\n",
frame->tf_cs & 0xffff, frame->tf_rip);
if (ISPL(frame->tf_cs) == SEL_UPL) {
if (TF_HAS_STACKREGS(frame)) {
ss = frame->tf_ss & 0xffff;
esp = frame->tf_rsp;
} else {
@ -934,7 +934,7 @@ amd64_syscall(struct thread *td, int traced)
ksiginfo_t ksi;
#ifdef DIAGNOSTIC
if (ISPL(td->td_frame->tf_cs) != SEL_UPL) {
if (!TRAPF_USERMODE(frame)) {
panic("syscall");
/* NOT REACHED */
}

13
sys/i386/i386/db_trace.c
View File

@ -82,8 +82,7 @@ struct db_variable *db_eregs = db_regs + nitems(db_regs);
static __inline int
get_esp(struct trapframe *tf)
{
return ((ISPL(tf->tf_cs) || kdb_frame->tf_eflags & PSL_VM) ?
tf->tf_esp : (intptr_t)&tf->tf_esp);
return (TF_HAS_STACKREGS(tf) ? tf->tf_esp : (intptr_t)&tf->tf_esp);
}
static int
@ -147,7 +146,7 @@ db_esp(struct db_variable *vp, db_expr_t *valuep, int op)
if (op == DB_VAR_GET)
*valuep = get_esp(kdb_frame);
else if (ISPL(kdb_frame->tf_cs))
else if (TF_HAS_STACKREGS(kdb_frame))
kdb_frame->tf_esp = *valuep;
return (1);
}
@ -180,9 +179,9 @@ db_ss(struct db_variable *vp, db_expr_t *valuep, int op)
return (0);
if (op == DB_VAR_GET)
*valuep = (ISPL(kdb_frame->tf_cs) ||
kdb_frame->tf_eflags & PSL_VM) ? kdb_frame->tf_ss : rss();
else if (ISPL(kdb_frame->tf_cs) || kdb_frame->tf_eflags & PSL_VM)
*valuep = TF_HAS_STACKREGS(kdb_frame) ? kdb_frame->tf_ss :
rss();
else if (TF_HAS_STACKREGS(kdb_frame))
kdb_frame->tf_ss = *valuep;
return (1);
}
@ -439,7 +438,7 @@ db_backtrace(struct thread *td, struct trapframe *tf, struct i386_frame *frame,
* Find where the trap frame actually ends.
* It won't contain tf_esp or tf_ss unless crossing rings.
*/
if (ISPL(kdb_frame->tf_cs))
if (TF_HAS_STACKREGS(kdb_frame))
instr = (int)(kdb_frame + 1);
else
instr = (int)&kdb_frame->tf_esp;

12
sys/i386/i386/trap.c
View File

@ -267,7 +267,8 @@ trap(struct trapframe *frame)
* interrupts disabled until they are accidentally
* enabled later.
*/
if (ISPL(frame->tf_cs) == SEL_UPL || (frame->tf_eflags & PSL_VM))
if (TRAPF_USERMODE(frame) &&
(curpcb->pcb_flags & PCB_VM86CALL) == 0)
uprintf(
"pid %ld (%s): trap %d with interrupts disabled\n",
(long)curproc->p_pid, curthread->td_name, type);
@ -307,9 +308,7 @@ trap(struct trapframe *frame)
enable_intr();
}
if ((ISPL(frame->tf_cs) == SEL_UPL) ||
((frame->tf_eflags & PSL_VM) &&
!(curpcb->pcb_flags & PCB_VM86CALL))) {
if (TRAPF_USERMODE(frame) && (curpcb->pcb_flags & PCB_VM86CALL) == 0) {
/* user trap */
td->td_pticks = 0;
@ -963,7 +962,7 @@ trap_fatal(frame, eva)
}
printf("instruction pointer = 0x%x:0x%x\n",
frame->tf_cs & 0xffff, frame->tf_eip);
if ((ISPL(frame->tf_cs) == SEL_UPL) || (frame->tf_eflags & PSL_VM)) {
if (TF_HAS_STACKREGS(frame)) {
ss = frame->tf_ss & 0xffff;
esp = frame->tf_esp;
} else {
@ -1117,7 +1116,8 @@ syscall(struct trapframe *frame)
ksiginfo_t ksi;
#ifdef DIAGNOSTIC
if (ISPL(frame->tf_cs) != SEL_UPL) {
if (!(TRAPF_USERMODE(frame) &&
(curpcb->pcb_flags & PCB_VM86CALL) == 0)) {
panic("syscall");
/* NOT REACHED */
}

16
sys/x86/include/frame.h
View File

@ -64,7 +64,7 @@ struct trapframe {
int tf_eip;
int tf_cs;
int tf_eflags;
/* below only when crossing rings (e.g. user to kernel) */
/* below only when crossing rings (user to kernel) */
int tf_esp;
int tf_ss;
};
@ -89,10 +89,10 @@ struct trapframe_vm86 {
int tf_eip;
int tf_cs;
int tf_eflags;
/* below only when crossing rings (e.g. user to kernel) */
/* below only when crossing rings (user (including vm86) to kernel) */
int tf_esp;
int tf_ss;
/* below only when switching out of VM86 mode */
/* below only when crossing from vm86 mode to kernel */
int tf_vm86_es;
int tf_vm86_ds;
int tf_vm86_fs;
@ -136,6 +136,7 @@ struct trapframe {
register_t tf_rip;
register_t tf_cs;
register_t tf_rflags;
/* below only when crossing rings (user to kernel) */
register_t tf_rsp;
register_t tf_ss;
};
@ -145,4 +146,13 @@ struct trapframe {
#define TF_HASFPXSTATE 0x4
#endif /* __amd64__ */
/*
* This alias for the MI TRAPF_USERMODE() should be used when we don't
* care about user mode itself, but need to know if a frame has stack
* registers. The difference is only logical, but on i386 the logic
* for using TRAPF_USERMODE() is complicated by sometimes treating vm86
* bioscall mode (which is a special ring 3 user mode) as kernel mode.
*/
#define TF_HAS_STACKREGS(tf) TRAPF_USERMODE(tf)
#endif /* _MACHINE_FRAME_H_ */