Timer leaks on pcntl_fork in ZTS build

[bwoebi]

Description

The following code:

<?php
pcntl_fork();

Resulted in this output (in child process):

=================================================================
==58097==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0xffffa37f943b in malloc (/usr/lib/aarch64-linux-gnu/libasan.so.5+0xcf43b)
    #1 0xffffa3622aaf in timer_create ../sysdeps/unix/sysv/linux/timer_create.c:59
    #2 0xaaaacd663a6b in zend_max_execution_timer_init /usr/local/src/php/Zend/zend_max_execution_timer.c:46
    #3 0xaaaacd2913c7 in init_executor /usr/local/src/php/Zend/zend_execute_API.c:202
    #4 0xaaaacd2e3497 in zend_activate /usr/local/src/php/Zend/zend.c:1257
    #5 0xaaaacd0b09c3 in php_request_startup /usr/local/src/php/main/main.c:1758
    #6 0xaaaacd7b92db in do_cli /usr/local/src/php/sapi/cli/php_cli.c:934
    #7 0xaaaacd7bbd57 in main /usr/local/src/php/sapi/cli/php_cli.c:1340
    #8 0xffffa08c7da3 in __libc_start_main ../csu/libc-start.c:308
    #9 0xaaaacbea50f3  (/opt/php/debug-zts-asan/bin/php+0x5b50f3)

There's an explicit comment that this shouldn't be freed after a fork:

void zend_max_execution_timer_shutdown(void) /* {{{ */
{
	/* Don't try to delete a timer created before a call to fork() */
	if (EG(pid) != getpid()) {
		return;
	}

So paging @dunglas who wrote that comment.

PHP Version

PHP 8.3

Operating System

No response

[dunglas]

This is likely a false positive to ignore or fix in the sanitizer:

The child does not inherit timers from its parent (setitimer(2), alarm(2), timer_create(2)).

https://www.man7.org/linux/man-pages/man2/fork.2.html

[bwoebi]

@dunglas No, it isn't. The docs say it doesn't inherit timer_create(2), which is the syscall, i.e. it's not inherited from the kernel space perspective.
However glibc, i.e. timer_create(3) (https://linux.die.net/man/3/timer_create), does allocate some memory, which needs to be freed in the child.

[dunglas]

@bwoebi it's a false positive: https://sourceware.org/bugzilla/show_bug.cgi?id=29705

[bwoebi]

@dunglas This is not related - this linked bug is a false positive of within a thread, where the timer is indeed deleted.

In my case, the timer is not freed at all from the perspective of the child. That memory is actually alive after a fork.

[bwoebi]

But it's a bug in glibc, which has been fixed in glibc 2.33: bminor/glibc@7a887dd, only SIGEV_THREAD is actually supposed to be allocating memory.

[bwoebi]

@dunglas I'd like to ask the question though: would it do any harm to unconditionally call timer_delete in the child?

[dunglas]

IIRC this was causing segfaults.

[arnaud-lb]

It is surprising that the glibc does not free timer resources after a fork since the timers are not inherited by child processes.

Currently it appears to be safe to delete a timer created in the parent process (before and after the 2.33 fix), but I don't think we should do this unless we can get a confirmation from glibc maintainers that the current behavior is expected.

If it is, deleting the timer in zend_max_execution_timer_init() if there is already a timer would be safe and fix the issue: https://github.com/php/php-src/pull/12324/files. EG(pid) being non-zero implies that EG(max_execution_timer_timer) is an initialized and non-deleted timer.

Edit: deleting a timer in the child process actually fails with EINVAL because INLINE_SYSCALL_CALL (timer_delete, ktimerid) fails in timer_delete().