Re: [PATCH] fix broken 2.4.x rt_sigprocmask error handling

[Erik Andersen]

On Mon Jan 05, 2004 at 11:23:02AM -0200, Marcelo Tosatti wrote:
> Hi Erik,
> 
> For how long the behaviour has been this? It is broken, but its brokenness
> is harmless.

Not sure really.  It is correct in 2.2.x, and 2.6.x, but is
wrong in all 2.4.x kernels starting with 2.4.0, but I havn't
bothered finding exactly which kernel broke things.

> Maybe some app rely on this behaviour? (I understand its wrong, but
> still).
> 
> Is there any major distribution which includes this fix?

I highly doubt anything relies on this.  Glibc checks for this
case before the syscall is made.  I found the bug while running
some the ltp testsuite on uClibc.  I have since added a check to
workaround the bug. but I would prefer to have it fixed properly
in the kernel.

> Anyway, isnt it easier to move the "if (error)" up like this?
> 
> --- signal.c.orig       2004-01-05 11:13:17.000000000 -0200
> +++ signal.c    2004-01-05 11:14:09.000000000 -0200
> @@ -888,11 +888,12 @@
>                         break;
>                 }
> 
> +               if (error)
> +                       goto out;
> +
>                 current->blocked = new_set;
>                 recalc_sigpending(current);
>                 spin_unlock_irq(&current->sigmask_lock);
> -               if (error)
> -                       goto out;
>                 if (oset)
>                         goto set_old;
>         } else if (oset) {

Only if you plan to ignore the locked spinlock.  :-)
The 2.2.x code does this:

	-current->blocked = new_set;
	+if (!error)
	    current->blocked = new_set;

I condiered that, but I thought it better to make the code 
more closely match 2.6, which looked a bit nicer.

 -Erik

--
Erik B. Andersen             http://codepoet-consulting.com/
--This message was written using 73% post-consumer electrons--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/