Re: CVSup segfaults identified/solved [PATCH]
Luoqi Chen wrote: > > o restoring %gs is now handled in the proper sigreturn. > > Restoration of %gs should not be in the kernel because it comes from > user application and maybe invalid, if you restore it inside the kernel > it could be fatal to the whole system, and on the other hand just a core > dump if done in the trampoline code which is still in user mode. Hmmm... What if the application passes a (possibly handcrafted) sigcontext to an explicit call to sigreturn. %gs should be restored in that case too, right? Isn't it therefore better to have %gs in the trapframe? -- Marcel Moolenaarmailto:[EMAIL PROTECTED] SCC Internetworking & Databases http://www.scc.nl/ The FreeBSD projectmailto:[EMAIL PROTECTED] To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: CVSup segfaults identified/solved [PATCH]
> Luoqi Chen wrote: > > > > o restoring %gs is now handled in the proper sigreturn. > > > > Restoration of %gs should not be in the kernel because it comes from > > user application and maybe invalid, if you restore it inside the kernel > > it could be fatal to the whole system, and on the other hand just a core > > dump if done in the trampoline code which is still in user mode. > > Hmmm... What if the application passes a (possibly handcrafted) > sigcontext to an explicit call to sigreturn. %gs should be restored in > that case too, right? > > Isn't it therefore better to have %gs in the trapframe? > > -- > Marcel Moolenaarmailto:[EMAIL PROTECTED] > SCC Internetworking & Databases http://www.scc.nl/ > The FreeBSD projectmailto:[EMAIL PROTECTED] > The only place sigreturn is called explicitly is to enter VM86 mode, and in that case, %gs is restored inside kernel as part of vm86 trapframe. In fact, you may choose to restore %gs in the kernel, but you have to be prepared to take a fault on the load_gs operation and to recover from the fault properly. The reason why %gs is not in the trapframe is that trapframe is used at all entrances to the kernel (syscall/trap/intr), if %gs is included, then we need to save and restore %gs for each syscall/trap/intr, that's about 40 clock cycles wasted each time. -lq To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: CVSup segfaults identified/solved [PATCH]
> Hi, > > It seems that the trampoline code got too long and resulted in the > coredumps people reported. The following patch solves that. it basicly > works as follows: > > o Simplify the trampoline code so that it doesn't have to distinguish >between an old- and new sigframe and also restoring %gs in both > cases. > o Which sigreturn to use is now determined by the process flag that >is used to determine which sendsig is to be used (symmetry) > o restoring %gs is now handled in the proper sigreturn. > > I'll commit this if noone objects. > > -- > Marcel Moolenaarmailto:[EMAIL PROTECTED] > SCC Internetworking & Databases http://www.scc.nl/ > The FreeBSD projectmailto:[EMAIL PROTECTED] Another comment, in mcontext_t structure it's best we list all registers individually like in sigcontext instead of using the struct trapframe. We might want to change trapframe in the future, but ucontext is part of the binary API and we don't want it to be affected. -lq To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: CVSup segfaults identified/solved [PATCH]
> Hi, > > It seems that the trampoline code got too long and resulted in the > coredumps people reported. The following patch solves that. it basicly > works as follows: > > o Simplify the trampoline code so that it doesn't have to distinguish >between an old- and new sigframe and also restoring %gs in both > cases. > o Which sigreturn to use is now determined by the process flag that >is used to determine which sendsig is to be used (symmetry) > o restoring %gs is now handled in the proper sigreturn. > > I'll commit this if noone objects. > > -- > Marcel Moolenaarmailto:[EMAIL PROTECTED] > SCC Internetworking & Databases http://www.scc.nl/ > The FreeBSD projectmailto:[EMAIL PROTECTED] Restoration of %gs should not be in the kernel because it comes from user application and maybe invalid, if you restore it inside the kernel it could be fatal to the whole system, and on the other hand just a core dump if done in the trampoline code which is still in user mode. -lq To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
CVSup segfaults identified/solved [PATCH]
Hi,
It seems that the trampoline code got too long and resulted in the
coredumps people reported. The following patch solves that. it basicly
works as follows:
o Simplify the trampoline code so that it doesn't have to distinguish
between an old- and new sigframe and also restoring %gs in both
cases.
o Which sigreturn to use is now determined by the process flag that
is used to determine which sendsig is to be used (symmetry)
o restoring %gs is now handled in the proper sigreturn.
I'll commit this if noone objects.
--
Marcel Moolenaarmailto:[EMAIL PROTECTED]
SCC Internetworking & Databases http://www.scc.nl/
The FreeBSD projectmailto:[EMAIL PROTECTED]
Index: i386/i386/genassym.c
===
RCS file: /home/ncvs/src/sys/i386/i386/genassym.c,v
retrieving revision 1.80
diff -u -r1.80 genassym.c
--- genassym.c 1999/10/04 19:33:51 1.80
+++ genassym.c 1999/10/05 12:02:18
@@ -148,7 +148,6 @@
printf("#define\tTF_EFLAGS %#x\n", OS(trapframe, tf_eflags));
printf("#define\tSIGF_HANDLER %#x\n", OS(sigframe, sf_ahu.sf_handler));
- printf("#define\tSIGF_SIGRET %#x\n", OS(sigframe, sf_sigreturn));
printf("#define\tSIGF_SC %#x\n", OS(osigframe, sf_siginfo.si_sc));
printf("#define\tSIGF_UC %#x\n", OS(sigframe, sf_uc));
Index: i386/i386/locore.s
===
RCS file: /home/ncvs/src/sys/i386/i386/locore.s,v
retrieving revision 1.128
diff -u -r1.128 locore.s
--- locore.s1999/09/29 15:06:19 1.128
+++ locore.s1999/10/05 11:54:06
@@ -415,37 +415,14 @@
*/
NON_GPROF_ENTRY(sigcode)
callSIGF_HANDLER(%esp) /* call signal handler */
- movlSIGF_SIGRET(%esp),%eax /* Get sigreturn cookie */
- cmpl$0x0ABCDEF0,%eax/* New one? */
- jne 3f
-/* New signalling code */
- lea SIGF_UC(%esp),%eax /* get ucontext */
+ lea SIGF_UC(%esp),%eax /* get ucontext_t */
pushl %eax
- testl $PSL_VM,UC_EFLAGS(%eax)
- jne 1f
- movlUC_GS(%eax),%gs /* restore %gs */
-1:
movl$SYS_sigreturn,%eax
pushl %eax/* junk to fake return addr. */
int $0x80 /* enter kernel with args */
- /* on stack */
-2:
- jmp 2b
-/* Old signalling code */
-3:
- lea SIGF_SC(%esp),%eax /* get sigcontext */
- pushl %eax
- testl $PSL_VM,SC_PS(%eax)
- jne 4f
- movlSC_GS(%eax),%gs /* restore %gs */
-4:
- movl$SYS_osigreturn,%eax
- pushl %eax/* junk to fake return addr. */
- int $0x80 /* enter kernel with args */
/* on stack */
-5:
- jmp 5b
-
+1:
+ jmp 1b
ALIGN_TEXT
_esigcode:
Index: i386/i386/machdep.c
===
RCS file: /home/ncvs/src/sys/i386/i386/machdep.c,v
retrieving revision 1.366
diff -u -r1.366 machdep.c
--- machdep.c 1999/10/04 19:33:51 1.366
+++ machdep.c 1999/10/05 12:47:23
@@ -725,8 +725,6 @@
tf->tf_eflags &= ~(PSL_VM|PSL_NT|PSL_T|PSL_VIF|PSL_VIP);
}
- sf.sf_sigreturn = 0x0ABCDEF0;
-
/*
* Copy the sigframe out to the user's stack.
*/
@@ -789,6 +787,8 @@
struct trapframe_vm86 *tf = (struct trapframe_vm86 *)regs;
struct vm86_kernel *vm86;
+ load_gs(scp->sc_gs);
+
/*
* if pcb_ext == 0 or vm86_inited == 0, the user hasn't
* set up the vm86 area, and we can't enter vm86 mode.
@@ -888,6 +888,10 @@
ucontext_t *ucp;
int cs, eflags;
+ if ((p->p_flag & P_NEWSIGSET) == 0) {
+ return osigreturn(p, (struct osigreturn_args *)uap);
+ }
+
regs = p->p_md.md_regs;
ucp = uap->sigcntxp;
eflags = ucp->uc_mcontext.mc_tf.tf_eflags;
@@ -898,6 +902,8 @@
if (eflags & PSL_VM) {
struct trapframe_vm86 *tf = (struct trapframe_vm86 *)regs;
struct vm86_kernel *vm86;
+
+ load_gs(ucp->uc_mcontext.mc_gs);
/*
* if pcb_ext == 0 or vm86_inited == 0, the user hasn't
Index: i386/include/sigframe.h
===
RCS file: /home/ncvs/src/sys/i386/include/sigframe.h,v
retrieving revision 1.2
diff -u -r1.2 sigframe.h
--- sigframe.h 1999/10/03 12:55:58 1.2
+++ sigframe.h 1999/10/05 11:53:53
@@ -86,9 +86,8 @@
__siginfohandler_t *sf_action;
__sighandler_t *sf_handler;
