linuxsh-dev

Syscall restarted with the clobbered 5th argument

[Kaz K. <kk...@rr...>]

Hi,

I'm working on the latest GNU libc with the 2.6.22-rc4 kernel and
I've found a few kernel problems.  This is the first one.

We use R0 as the 5th argument of syscall.  When the syscall restarts
after signal handling, we should restore the old value of R0.
The attached patch does it.  Without this patch, I've experienced random
failures in the situation which signals are issued frequently.

Regards,
	kaz

Signed-off-by: Kaz Kojima <kk...@rr...>

--- GIT/linux-2.6/arch/sh/kernel/signal.c	2007-06-09 07:33:58.000000000 +0900
+++ linux-2.6.22-rc4/arch/sh/kernel/signal.c	2007-06-15 10:56:43.000000000 +0900
@@ -481,7 +481,7 @@ give_sigsegv:
 
 static int
 handle_signal(unsigned long sig, struct k_sigaction *ka, siginfo_t *info,
-	      sigset_t *oldset, struct pt_regs *regs)
+	      sigset_t *oldset, struct pt_regs *regs, unsigned int save_r0)
 {
 	int ret;
 
@@ -500,6 +500,7 @@ handle_signal(unsigned long sig, struct 
 				}
 			/* fallthrough */
 			case -ERESTARTNOINTR:
+				regs->regs[0] = save_r0;
 				regs->pc -= instruction_size(
 						ctrl_inw(regs->pc - 4));
 				break;
@@ -583,7 +584,8 @@ static void do_signal(struct pt_regs *re
 	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
 	if (signr > 0) {
 		/* Whee!  Actually deliver the signal.  */
-		if (handle_signal(signr, &ka, &info, oldset, regs) == 0) {
+		if (handle_signal(signr, &ka, &info, oldset, regs, save_r0)
+		    == 0) {
 			/* a signal was successfully delivered; the saved
 			 * sigmask will have been stored in the signal frame,
 			 * and will be restored by sigreturn, so we can simply

Re: Syscall restarted with the clobbered 5th argument

[Paul M. <le...@li...>]

On Sun, Jun 17, 2007 at 07:20:15AM +0900, Kaz Kojima wrote:
> I'm working on the latest GNU libc with the 2.6.22-rc4 kernel and
> I've found a few kernel problems.  This is the first one.
> 
> We use R0 as the 5th argument of syscall.  When the syscall restarts
> after signal handling, we should restore the old value of R0.
> The attached patch does it.  Without this patch, I've experienced random
> failures in the situation which signals are issued frequently.
> 
Applied, thanks. I'll add this to the 2.6.22 queue.

Re: Syscall restarted with the clobbered 5th argument

[Carl S. <sha...@gm...>]

Hi Kaz,

I've just fixed the same problem (in exactly the same way) in our
2.6.17 ST kernel!  It causes a failure in the glibc test tst-eintr1.

I found another minor problem as well - we are missing a test for
-ERESTART_RESTARTBLOCK in handle_signal().

i.e.

switch (regs->regs[0]) {
                        case -ERESTARTNOHAND:
                                regs->regs[0] = -EINTR;
                                break;

should be

switch (regs->regs[0]) {
                        case -ERESTART_RESTARTBLOCK:
                        case -ERESTARTNOHAND:
                                regs->regs[0] = -EINTR;
                                break;

(Apologies for lack of patch but it is only a one line change and I
don't have a copy of the latest git kernel handy).

This fixes the LTP test nanosleep03 - the current kernel causes
-ERESTART_RESTARTBLOCK to reach user space rather than the correct
-EINTR.

Slightly off-topic for the kernel mailing list, this work came about
because we are experiencing occasional problems with SH4 thread
unwinding after a pthread_cancel() and the ensuing signal.  I'll email
you more information off-list as well as some other SH glibc fixes we
have.

Carl

On 6/16/07, Kaz Kojima <kk...@rr...> wrote:
> Hi,
>
> I'm working on the latest GNU libc with the 2.6.22-rc4 kernel and
> I've found a few kernel problems.  This is the first one.
>
> We use R0 as the 5th argument of syscall.  When the syscall restarts
> after signal handling, we should restore the old value of R0.
> The attached patch does it.  Without this patch, I've experienced random
> failures in the situation which signals are issued frequently.
>
> Regards,
>         kaz
>
> Signed-off-by: Kaz Kojima <kk...@rr...>
>
> --- GIT/linux-2.6/arch/sh/kernel/signal.c       2007-06-09 07:33:58.000000000 +0900
> +++ linux-2.6.22-rc4/arch/sh/kernel/signal.c    2007-06-15 10:56:43.000000000 +0900
> @@ -481,7 +481,7 @@ give_sigsegv:
>
>  static int
>  handle_signal(unsigned long sig, struct k_sigaction *ka, siginfo_t *info,
> -             sigset_t *oldset, struct pt_regs *regs)
> +             sigset_t *oldset, struct pt_regs *regs, unsigned int save_r0)
>  {
>         int ret;
>
> @@ -500,6 +500,7 @@ handle_signal(unsigned long sig, struct
>                                 }
>                         /* fallthrough */
>                         case -ERESTARTNOINTR:
> +                               regs->regs[0] = save_r0;
>                                 regs->pc -= instruction_size(
>                                                 ctrl_inw(regs->pc - 4));
>                                 break;
> @@ -583,7 +584,8 @@ static void do_signal(struct pt_regs *re
>         signr = get_signal_to_deliver(&info, &ka, regs, NULL);
>         if (signr > 0) {
>                 /* Whee!  Actually deliver the signal.  */
> -               if (handle_signal(signr, &ka, &info, oldset, regs) == 0) {
> +               if (handle_signal(signr, &ka, &info, oldset, regs, save_r0)
> +                   == 0) {
>                         /* a signal was successfully delivered; the saved
>                          * sigmask will have been stored in the signal frame,
>                          * and will be restored by sigreturn, so we can simply
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by DB2 Express
> Download DB2 Express C - the FREE version of DB2 express and take
> control of your XML. No limits. Just data. Click to get it now.
> http://sourceforge.net/powerbar/db2/
> _______________________________________________
> linuxsh-dev mailing list
> lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linuxsh-dev
>

Re: Syscall restarted with the clobbered 5th argument

[Paul M. <le...@li...>]

On Mon, Jun 18, 2007 at 12:00:39PM +0100, Carl Shaw wrote:
> I found another minor problem as well - we are missing a test for
> -ERESTART_RESTARTBLOCK in handle_signal().
> 
> This fixes the LTP test nanosleep03 - the current kernel causes
> -ERESTART_RESTARTBLOCK to reach user space rather than the correct
> -EINTR.

Hmm, wonder how we missed that. Looks like sh64 needs it too. I'll queue
up a fix for 2.6.22, thanks Carl.