* Re: Register saving and signal handling
2005-01-25 20:16 Register saving and signal handling Robert Szeleney
@ 2005-01-25 21:26 ` linux-os
0 siblings, 0 replies; 2+ messages in thread
From: linux-os @ 2005-01-25 21:26 UTC (permalink / raw)
To: Robert Szeleney; +Cc: Linux kernel
[-- Attachment #1: Type: TEXT/PLAIN, Size: 3752 bytes --]
On Tue, 25 Jan 2005, Robert Szeleney wrote:
> Hi!
>
> This is my first time posting to the Linux kernel mailing-list, and I hope
> someone can help me or at least explain following to me.
>
> When a task gets interrupted by a signal, the do_signal() function is called.
> Now, when the signal is not handled by the task and the interrupted function
> returned -EINTR, the syscall gets restarted by modifying the user mode EIP to
> point to the int 0x80 again.
>
> When the task leaves the do_signal function, it will pop the saved registers
> and return to the user mode and immediately do the syscall again.
>
> But now to the actual question:
>
> Let's make a new "test" system call function. Let's call it: sys_test
>
> asmlinkage int sys_test(int para1, int para2)
> {
> para1++;
> para2++;
>
> kill( current->pid, SIGCHLD);
> return -EINTR;
> }
>
> When compiling this function, GCC increments para1 and para2 by one. para1
> and para2 are stored on the kernel stack. The system call entry assembler
> function pushed the registers containing this values from the usermode on the
> stack.
> But GCC actually modified the values on the stack here, the "live" data.
> (which will be poped later again, right before returning to user mode)
>
> After returning from this function, the system call wrapper checks for a
> signal and calls do_signal. The do_signal call will restart the system call
> by modifying the user mode EIP. Then, the system call wrapper will pop the
> saved registers from the stack. But here I see this problem. The already
> modified values for para1 and para2 will be popped. When the system call is
> then start again, para1 and para2 don't have to original value.
>
> One can say, why are you modifying para1 and para2 then? Yes, this is
> correct, but after compiling a few more test sources, I got following
> problem:
>
> asmlinkage int sys_test(int iSize)
> {
> printk("Size is: %d\n", iSize * sizeof(any_structure));
> }
>
> When compiling this, GCC produces following assembler:
>
> ....
> sall $4, 8(%ebp)
> ....
>
But this is correct. The caller should pass a COPY of the parameter.
The called procedure can do anything it wants to this copy. Check
to see what asmlinkage is #defined to be ou your system. It should
be __attribute__((regparam(0))). If it got changed, all bets are
off with any interface code. Everything needs to match.
> which actually modifies the content of the stack holding the iSize again. It
> is very difficult to keep track of such implicit stack argument
> modifications.
> Thus, when a signal is waiting and the syscall is restarted, iSize contains a
> different value already.
>
> So, does anyone else have such a problem? Is there any compiler flag missing?
> Is this possible at all?
>
> Thank you very much!
> Robert!
>
> Btw, please CC to mf204005@liwest.at too.
Yes.
For instance:
void funct(int param)
{
param++;
}
int main()
{
int foo = 0;
printf("%d\n", foo);
}
That code should pass a COPY of foo to funct(). Procedure
funct() should be able to do anything it wants with it
and, since it's a copy, main() should still print 0.
However, there is some kernel code that passes the
actual value, not a copy.
linux-2.6.9/arch/i386/kernel/semaphore.c is an example.
This has some hand-tweaked assembly that violates the
de-facto 'C' standard. There may be other such kernel
code. I submitted a patch, but it was rejected. Just
for kicks, I attached the patch so you can see what
problem(s) may still exist.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.10 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by Dictator Bush.
98.36% of all statistics are fiction.
[-- Attachment #2: Type: TEXT/PLAIN, Size: 1741 bytes --]
--- linux-2.6.9/arch/i386/kernel/semaphore.c.orig 2004-10-29 13:00:17.961579368 -0400
+++ linux-2.6.9/arch/i386/kernel/semaphore.c 2004-10-29 13:03:35.046617888 -0400
@@ -198,9 +198,11 @@
#endif
"pushl %eax\n\t"
"pushl %edx\n\t"
- "pushl %ecx\n\t"
+ "pushl %ecx\n\t" // Register to save
+ "pushl %ecx\n\t" // Passed parameter
"call __down\n\t"
- "popl %ecx\n\t"
+ "leal 0x04(%esp), %esp\t\n" // Bypass corrupted parameter
+ "popl %ecx\n\t" // Restore original
"popl %edx\n\t"
"popl %eax\n\t"
#if defined(CONFIG_FRAME_POINTER)
@@ -220,9 +222,11 @@
"movl %esp,%ebp\n\t"
#endif
"pushl %edx\n\t"
- "pushl %ecx\n\t"
+ "pushl %ecx\n\t" // Save register
+ "pushl %ecx\n\t" // Passed parameter
"call __down_interruptible\n\t"
- "popl %ecx\n\t"
+ "leal 0x04(%esp), %esp\n\t" // Bypass corrupted parameter
+ "popl %ecx\n\t" // Restore register
"popl %edx\n\t"
#if defined(CONFIG_FRAME_POINTER)
"movl %ebp,%esp\n\t"
@@ -241,9 +245,11 @@
"movl %esp,%ebp\n\t"
#endif
"pushl %edx\n\t"
- "pushl %ecx\n\t"
+ "pushl %ecx\n\t" // Save register
+ "pushl %ecx\n\t" // Passed parameter
"call __down_trylock\n\t"
- "popl %ecx\n\t"
+ "leal 0x04(%esp), %esp\n\t" // Bypass corrupted parameter
+ "popl %ecx\n\t" // Restore register
"popl %edx\n\t"
#if defined(CONFIG_FRAME_POINTER)
"movl %ebp,%esp\n\t"
@@ -259,9 +265,11 @@
"__up_wakeup:\n\t"
"pushl %eax\n\t"
"pushl %edx\n\t"
- "pushl %ecx\n\t"
+ "pushl %ecx\n\t" // Save register
+ "pushl %ecx\n\t" // Passed parameter
"call __up\n\t"
- "popl %ecx\n\t"
+ "leal 0x04(%esp), %esp\n\t" // Bypass corrupted parameter
+ "popl %ecx\n\t" // Restore register
"popl %edx\n\t"
"popl %eax\n\t"
"ret"
^ permalink raw reply [flat|nested] 2+ messages in thread