Hi,
The attached patch fixes the signal handler processing bug
described in the last mail. You can use the procedure of that mail to
verify that the bug is really fixed.
Greetings,
Juan
PREVIOUS OPERATION AND BUGS
1. Checking of signals was done at two places: after a task switch and
at the end of a system call, before returning to user space. There is
the possibility of duplicating the work of manipulating the user stack
in a call to schedule() during a system call and again at the end of
the system call.
2. At the end of interrupts, a call to schedule is done before
returning to user space, but checking of signals will be done only
after a task switch, thus missing perfectly valid opportunities to
deliver signals.
3. The user stack layout is different during a system call than during
an interrupt. The function that modifies the user stack to call a
signal handler ("arch_setup_sighandler_stack") assumes the user stack
layout to be as during a system call.
4. However, installing a signal handler after an interrupt did not
(immediately) crash the task because the manipulation of the user
stack did not modify the return address, and the modification to the
user stack pointer was ignored. As result, the signal handler is not
called.
NEW OPERATION
1. Checking of signals is done at the end of a system call, before
returning to user space. Also at the end of any interrupt, if the
interrupt routine will return to user space. It is removed from the
function schedule. Now, checking of signals is done only at the
moments when it makes sense.
2. Function "arch_setup_sighandler_stack" accounts for the difference
in the user stack layout at different times.
3. User stack pointer is reloaded from the task structure before
returning to user space from an interrupt. Now, delivery of signals
always work.
OTHER CHANGES
1. A small optimization to reduce code size was done to function
"run_init_process".
2. Despite the above optimization, there is an increase in code size
of 32 bytes.
The Image builded without errors. The kernel was tested with QEMU and
dioscuri emulators. Also in a PPro pc booting from floppy.
diff -Nurb elks.orig/arch/i86/kernel/irqtab.c elks/arch/i86/kernel/irqtab.c
--- elks.orig/arch/i86/kernel/irqtab.c 2012-06-20 13:45:28.000000000 -0500
+++ elks/arch/i86/kernel/irqtab.c 2012-06-26 14:14:08.000000000 -0500
@@ -19,19 +19,17 @@
#define SEG_IRQ_DATA es
#define stashed_ds [0]
- /* _our_ds [18] bios16.c */
#else
#define SEG_IRQ_DATA cs
- #define stashed_ds cseg_stashed_ds
#ifndef S_SPLINT_S
#asm
- .globl cseg_stashed_ds
+ .globl stashed_ds
.even
-cseg_stashed_ds:
+stashed_ds:
.word 0
#endasm
@@ -39,6 +37,8 @@
#endif
+extern void sig_check(void);
+
void irqtab_init(void)
{
#ifndef S_SPLINT_S
@@ -413,20 +413,22 @@
mov ss,bx ! /* Set SS: right */
mov bx,_current
cmp dx,TASK_USER_SS[bx] ! entry ss = current->t_regs.ss?
- je utask ! Switch to kernel
-!
-! Bios etc - switch to interrupt stack
-!
- mov sp,#_intstack
- j ktask
+ jne btask ! Switch to interrupt stack
!
! User task. Extract kernel SP. (BX already holds current)
! At this point, the kernel stack is empty. Thus, we can load
! the kernel stack pointer without accesing memory
!
-utask:
+ mov TASK_USER_SP[bx],sp
lea sp,TASK_KSTKTOP[bx] ! switch to kernel stack ptr
+ lea bp,TASK_USER_SP[bx]
inc ch ! Switch allowable
+ j updct
+!
+! Bios etc - switch to interrupt stack
+!
+btask:
+ mov sp,#_intstack
!
! In ktask state we have a suitable stack. It might be
! better to use the intstack..
@@ -437,8 +439,8 @@
! leave them in stashed_ss/sp as we could re-enter the
! routine on a reschedule.
! */
- push bp ! push entry SP
push dx ! push entry SS
+ push bp ! push entry SP
!
! The registers are now stored. Remember where
!
@@ -446,6 +448,7 @@
!
! Update intr_count
!
+updct:
inc _intr_count
!
! We are on a suitable stack and ch says whether
@@ -513,40 +516,40 @@
! mov bx,_need_resched ! Schedule needed
! cmp bx,#0 !
! je nosched ! No
- call _schedule ! Task switch
!
! This path will return directly to user space
!
- pop ax ! stacked SS
- pop cx ! stacked SP
-#ifdef CONFIG_ADVANCED_MM
+ call _schedule ! Task switch
mov bx,_current
- mov ax, TASK_USER_SS[bx] ! user ds
- mov bp, sp
-; mov 12[bp], ax ! change the es in the stack
-; mov 14[bp], ax ! change the ds in the stack
-#endif
+ mov 8[bx],#1
+ call _sig_check ! Check signals
!
! At this point, the kernel stack is empty. Thus, there is no
! need to save the kernel stack pointer.
!
+ mov bx,_current
+ mov sp,TASK_USER_SP[bx]
+#ifndef CONFIG_ADVANCED_MM
+ mov ss,TASK_USER_SS[bx]
+#else
+ mov ax, TASK_USER_SS[bx] ! user ds
+ mov bp, sp
+ mov ss, ax
+ mov 12[bp], ax ! change the es in the stack
+ mov 14[bp], ax ! change the ds in the stack
+#endif
j noschedpop
-
-nosched:
!
! Now we have to rescue our stack pointer/segment.
!
- pop ax ! SS
+nosched:
pop cx ! SP
-!
-! Switch stacks to the interrupting stack
-!
-noschedpop:
- mov ss,ax
+ pop ss ! SS
mov sp,cx
!
! Restore registers and return
!
+noschedpop:
pop bp
pop di
pop si
diff -Nurb elks.orig/arch/i86/kernel/process.c elks/arch/i86/kernel/process.c
--- elks.orig/arch/i86/kernel/process.c 2012-06-20 13:45:28.000000000 -0500
+++ elks/arch/i86/kernel/process.c 2012-06-26 14:16:09.000000000 -0500
@@ -50,8 +50,6 @@
#else
-#define stashed_ds cseg_stashed_ds
-
#ifndef S_SPLINT_S
#asm
@@ -63,7 +61,7 @@
* linker doesnt store them in block.
*/
- .extern cseg_stashed_ds
+ .extern stashed_ds
/* and now code */
@@ -79,11 +77,8 @@
void sig_check(void)
{
- register __ptask currentp = current;
-
- if (currentp->signal)
- (void) do_signal();
- currentp->signal = 0;
+ if (current->signal)
+ do_signal();
}
#ifndef S_SPLINT_S
@@ -189,6 +184,8 @@
pop ax
call _syscall
push ax
+ mov bx,_current
+ mov 8[bx],#0
call _sig_check
pop ax
pop bx
@@ -310,14 +307,7 @@
*/
{
#asm
- cli
- mov bx, _current
- mov sp, TASK_USER_SP[bx] ! user stack offset
- mov ax, TASK_USER_SS[bx] ! user stack segment
- mov ss, ax
- mov ds, ax
- mov es, ax
- iret ! reloads flags = >reenables interrupts
+ br _ret_from_syscall
#endasm
}
#endif
@@ -411,16 +401,23 @@
void arch_setup_sighandler_stack(register struct task_struct *t,
__sighandler_t addr,unsigned signr)
{
- debug4("Stack %x was %x %x %x\n", addr, get_ustack(t,0), get_ustack(t,2),
- get_ustack(t,4));
- put_ustack(t, 2, (int) get_ustack(t,0));
- put_ustack(t, 0, (int) get_ustack(t,4));
- put_ustack(t, 4, (int) signr);
- put_ustack(t, -2, (int)t->t_regs.cs);
- put_ustack(t, -4, (int) addr);
+ register char *i;
+
+ i = 0;
+ if(t->t_regs.bx != 0) {
+ for(; (int)i < 18; i += 2)
+ put_ustack(t, (int)i-4, (int)get_ustack(t,(int)i));
+ }
+ debug4("Stack %x was %x %x %x\n", addr, get_ustack(t,(int)i), get_ustack(t,(int)i+2),
+ get_ustack(t,(int)i+4));
+ put_ustack(t, (int)i-4, (int)addr);
+ put_ustack(t, (int)i-2, (int)get_ustack(t,(int)i+2));
+ put_ustack(t, (int)i+2, (int)get_ustack(t,(int)i));
+ put_ustack(t, (int)i, (int)get_ustack(t,(int)i+4));
+ put_ustack(t, (int)i+4, (int)signr);
t->t_regs.sp -= 4;
- debug5("Stack is %x %x %x %x %x\n", get_ustack(t,0), get_ustack(t,2),
- get_ustack(t,4),get_ustack(t,6), get_ustack(t,8));
+ debug5("Stack is %x %x %x %x %x\n", get_ustack(t,(int)i), get_ustack(t,(int)i+2),
+ get_ustack(t,(int)i+4),get_ustack(t,(int)i+6), get_ustack(t,(int)i+8));
}
/*
diff -Nurb elks.orig/arch/i86/kernel/signal.c elks/arch/i86/kernel/signal.c
--- elks.orig/arch/i86/kernel/signal.c 2012-05-11 13:26:27.000000000 -0500
+++ elks/arch/i86/kernel/signal.c 2012-06-25 13:06:36.000000000 -0500
@@ -24,10 +24,9 @@
register struct sigaction *sa;
unsigned signr;
- while (currentp->signal) {
+ while ((currentp->signal &= ((1 << NSIG) - 1))) {
signr = find_first_non_zero_bit(¤tp->signal, NSIG);
- if (signr == NSIG)
- panic("No signal set!\n");
+ clear_bit(signr, ¤tp->signal);
debug2("Process %d has signal %d.\n", currentp->pid, signr);
sa = ¤tp->sig.action[signr];
@@ -81,6 +80,7 @@
arch_setup_sighandler_stack(current, sa->sa_handler, signr);
debug1("Stack at %x\n", current->t_regs.sp);
sa->sa_handler = SIG_DFL;
+ currentp->signal = 0;
return 1;
}
diff -Nurb elks.orig/arch/i86/sibo/irqtab.c elks/arch/i86/sibo/irqtab.c
--- elks.orig/arch/i86/sibo/irqtab.c 2012-06-20 13:45:28.000000000 -0500
+++ elks/arch/i86/sibo/irqtab.c 2012-06-26 14:19:40.000000000 -0500
@@ -19,19 +19,17 @@
#define SEG_IRQ_DATA es
#define stashed_ds [0]
- /* _our_ds [18] bios16.c */
#else
#define SEG_IRQ_DATA cs
- #define stashed_ds cseg_stashed_ds
#ifndef S_SPLINT_S
#asm
- .globl cseg_stashed_ds
+ .globl stashed_ds
.even
-cseg_stashed_ds:
+stashed_ds:
.word 0
#endasm
@@ -39,6 +37,8 @@
#endif
+extern void sig_check(void);
+
void irqtab_init(void)
{
#ifndef S_SPLINT_S
@@ -412,20 +412,22 @@
mov ss,bx ! /* Set SS: right */
mov bx,_current
cmp dx,TASK_USER_SS[bx] ! entry ss = current->t_regs.ss?
- je utask ! Switch to kernel
-!
-! Bios etc - switch to interrupt stack
-!
- mov sp,#_intstack
- j ktask
+ jne btask ! Switch to interrupt stack
!
! User task. Extract kernel SP. (BX already holds current)
! At this point, the kernel stack is empty. Thus, we can load
! the kernel stack pointer without accesing memory
!
-utask:
+ mov TASK_USER_SP[bx],sp
lea sp,TASK_KSTKTOP[bx] ! switch to kernel stack ptr
+ lea bp,TASK_USER_SP[bx]
inc ch ! Switch allowable
+ j updct
+!
+! Bios etc - switch to interrupt stack
+!
+btask:
+ mov sp,#_intstack
!
! In ktask state we have a suitable stack. It might be
! better to use the intstack..
@@ -436,8 +438,8 @@
! leave them in stashed_ss/sp as we could re-enter the
! routine on a reschedule.
! */
- push bp ! push entry SP
push dx ! push entry SS
+ push bp ! push entry SP
!
! The registers are now stored. Remember where
!
@@ -445,6 +447,7 @@
!
! Update intr_count
!
+updct:
inc _intr_count
!
! We are on a suitable stack and ch says whether
@@ -483,40 +486,40 @@
! mov bx,_need_resched ! Schedule needed
! cmp bx,#0 !
! je nosched ! No
- call _schedule ! Task switch
!
! This path will return directly to user space
!
- pop ax ! stacked SS
- pop cx ! stacked SP
-#ifdef CONFIG_ADVANCED_MM
+ call _schedule ! Task switch
mov bx,_current
- mov ax, TASK_USER_SS[bx] ! user ds
- mov bp, sp
-; mov 12[bp], ax ! change the es in the stack
-; mov 14[bp], ax ! change the ds in the stack
-#endif
+ mov 8[bx],#1
+ call _sig_check ! Check signals
!
! At this point, the kernel stack is empty. Thus, there in no
! need to save the kernel stack pointer.
!
+ mov bx,_current
+ mov sp,TASK_USER_SP[bx]
+#ifndef CONFIG_ADVANCED_MM
+ mov ss,TASK_USER_SS[bx]
+#else
+ mov ax, TASK_USER_SS[bx] ! user ds
+ mov bp, sp
+ mov ss, ax
+ mov 12[bp], ax ! change the es in the stack
+ mov 14[bp], ax ! change the ds in the stack
+#endif
j noschedpop
-
-nosched:
!
! Now we have to rescue our stack pointer/segment.
!
- pop ax ! SS
+nosched:
pop cx ! SP
-!
-! Switch stacks to the interrupting stack
-!
-noschedpop:
- mov ss,ax
+ pop ss ! SS
mov sp,cx
!
! Restore registers and return
!
+noschedpop:
pop bp
pop di
pop si
diff -Nurb elks.orig/kernel/sched.c elks/kernel/sched.c
--- elks.orig/kernel/sched.c 2012-06-20 13:45:28.000000000 -0500
+++ elks/kernel/sched.c 2012-06-25 13:06:36.000000000 -0500
@@ -24,8 +24,6 @@
extern int intr_count;
-extern int do_signal(void);
-
static void run_timer_list();
void add_to_runqueue(register struct task_struct *p)
@@ -150,11 +148,6 @@
tswitch(); /* Won't return for a new task */
- if(current->signal){
- do_signal();
- current->signal = 0;
- }
-
if (timeout) {
del_timer(&timer);
}
