Patch to disarm timers after an exec syscall

Patch to disarm timers after an exec syscall

[Gernot Payer]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

Hi all,

while running the openposix testsuite I saw testcase timer_create/9-1.c 
failing. This testcase tests whether timers are disarmed when a process calls 
exec, as described in e.g. 
http://www.opengroup.org/onlinepubs/009695399/functions/timer_create.html.

The attached one-liner patch (+ one line comment) fixes this issue. I did the 
diff against 2.6.12.1, but the fix is pretty much the same for every other 
2.6.x kernel I had a look at.

I don't think this patch breaks anything, as relying on this (undocumented) 
behaviour would imho be bad style.

So tell me what you think, and if you have some pointers to interesting 
discussions about Linux and POSIX compliance then I would like to read that 
as well.

mfg
Gernot

[-- Attachment #2: delete-old-itimers-in-do_execve-linux-2.6.12.1.patch --]
[-- Type: text/x-diff, Size: 333 bytes --]

--- linux-2.6.12.1-orig/fs/exec.c	2005-06-29 14:29:31.069738264 +0200
+++ linux-2.6.12.1/fs/exec.c	2005-06-29 14:34:46.034856288 +0200
@@ -1200,6 +1200,10 @@
 		acct_update_integrals(current);
 		update_mem_hiwater(current);
 		kfree(bprm);
+
+		/* delete old itimers */
+		exit_itimers(current->signal);
+		
 		return retval;
 	}
 

Re: Patch to disarm timers after an exec syscall

[Andrew Morton]

Gernot Payer <gpayer@suse.de> wrote:
>
> while running the openposix testsuite I saw testcase timer_create/9-1.c 
>  failing. This testcase tests whether timers are disarmed when a process calls 
>  exec, as described in e.g. 
>  http://www.opengroup.org/onlinepubs/009695399/functions/timer_create.html.
> 
>  The attached one-liner patch (+ one line comment) fixes this issue. I did the 
>  diff against 2.6.12.1, but the fix is pretty much the same for every other 
>  2.6.x kernel I had a look at.
> 
>  I don't think this patch breaks anything, as relying on this (undocumented) 
>  behaviour would imho be bad style.
> 
>  So tell me what you think, and if you have some pointers to interesting 
>  discussions about Linux and POSIX compliance then I would like to read that 
>  as well.
> 
>  --- linux-2.6.12.1-orig/fs/exec.c	2005-06-29 14:29:31.069738264 +0200
>  +++ linux-2.6.12.1/fs/exec.c	2005-06-29 14:34:46.034856288 +0200
>  @@ -1200,6 +1200,10 @@
>   		acct_update_integrals(current);
>   		update_mem_hiwater(current);
>   		kfree(bprm);
>  +
>  +		/* delete old itimers */
>  +		exit_itimers(current->signal);
>  +		
>   		return retval;
>   	}

Ouch.  What does 2.4.x do?

It wouldn't surprise me if fixing this breaks _something_ out there.  We
might have to remain non-POSIX-compliant for the rest of time.

Re: Patch to disarm timers after an exec syscall

[Chris Wright]

* Gernot Payer (gpayer@suse.de) wrote:
> Hi all,
> 
> while running the openposix testsuite I saw testcase timer_create/9-1.c 
> failing. This testcase tests whether timers are disarmed when a process calls 
> exec, as described in e.g. 
> http://www.opengroup.org/onlinepubs/009695399/functions/timer_create.html.

> 
> The attached one-liner patch (+ one line comment) fixes this issue. I did the 
> diff against 2.6.12.1, but the fix is pretty much the same for every other 
> 2.6.x kernel I had a look at.

No, this can't do.  It conflicts with the other bit of requirements.
Specifically:
http://www.opengroup.org/onlinepubs/009695399/functions/exec.html

As you mention:

  [TMR] Per-process timers created by the calling process
  shall be deleted before replacing the current process image with the new
  process image.

But also:

  The new process shall inherit at least the following attributes from the
  calling process image:
  <snip>
  o [XSI] Interval timers

And this kills the latter.

thanks,
-chris

Re: Patch to disarm timers after an exec syscall

[Chris Wright]

* Andrew Morton (akpm@osdl.org) wrote:
> Ouch.  What does 2.4.x do?

Does 2.4 even support realtime timers?

> It wouldn't surprise me if fixing this breaks _something_ out there.  We
> might have to remain non-POSIX-compliant for the rest of time.

But I agree, the fix is wrong as it at least breaks compliance with
itimers.

thanks,
-chris

Re: Patch to disarm timers after an exec syscall

[Roland McGrath]

In the current code, de_thread already calls exit_itimers to address
exactly this issue.  Can someone please send me the test case that
demonstrates this is not working right?


Thanks,
Roland

Re: Patch to disarm timers after an exec syscall

[Gernot Payer]

On Wednesday 29 June 2005 20:27, Chris Wright wrote:
> No, this can't do.  It conflicts with the other bit of requirements.
> Specifically:
> http://www.opengroup.org/onlinepubs/009695399/functions/exec.html
>
> As you mention:
>
>   [TMR] Per-process timers created by the calling process
>   shall be deleted before replacing the current process image with the new
>   process image.
>
> But also:
>
>   The new process shall inherit at least the following attributes from the
>   calling process image:
>   <snip>
>   o [XSI] Interval timers
>
> And this kills the latter.

Ah, thanks for pointing that out. Unfortunately this fact isn't made clear in 
the timer_create page and the exec page isn't very understandable for 
non-native speakers and non-lawyers. ;-)

But after reading the parts you mentioned (expecially the [XSI] and [TMR] 
acronyms), I have to agree with you, the patch is wrong and so is the test 
case I mentioned.

However, poking around in the kernel was fun anyway. ;-)

> thanks,
> -chris

mfg
Gernot

Re: Patch to disarm timers after an exec syscall

[Gernot Payer]

On Wednesday 29 June 2005 20:50, Roland McGrath wrote:
> In the current code, de_thread already calls exit_itimers to address
> exactly this issue.  Can someone please send me the test case that
> demonstrates this is not working right?

This test case can be found here:

http://cvs.sourceforge.net/viewcvs.py/posixtest/posixtestsuite/conformance/interfaces/timer_create/9-1.c?view=markup

But as already mentioned in this thread, this test case is wrong.

> Thanks,
> Roland

mfg
Gernot