linux-kernel - Re: [PATCH 1/1] signal: Fix memory leak for PIDFD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aaadab31-661b-4939-b245-75675eda9b4b@lucifer.local>
Date: Mon, 18 Aug 2025 15:27:17 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: "Adrian Huang (Lenovo)" <adrianhuang0701@...il.com>
Cc: Christian Brauner <brauner@...nel.org>, linux-kernel@...r.kernel.org,
        ahuang12@...ovo.com
Subject: Re: [PATCH 1/1] signal: Fix memory leak for PIDFD_SELF* sentinels

On Mon, Aug 18, 2025 at 09:43:10PM +0800, Adrian Huang (Lenovo) wrote:
> Commit f08d0c3a7111 ("pidfd: add PIDFD_SELF* sentinels to refer to own
> thread/process") introduced a leak by acquiring a pid reference through
> get_task_pid(), which increments pid->count but never drops it with
> put_pid().
>
> As a result, kmemleak reports unreferenced pid objects after running
> tools/testing/selftests/pidfd/pidfd_test, for example:

OK am observing the same, good spot! Tried fix locally, it works, thanks :)

>
>   unreferenced object 0xff1100206757a940 (size 160):
>     comm "pidfd_test", pid 16965, jiffies 4294853028
>     hex dump (first 32 bytes):
>       01 00 00 00 00 00 00 00 00 00 00 00 fd 57 50 04  .............WP.
>       5e 44 00 00 00 00 00 00 18 de 34 17 01 00 11 ff  ^D........4.....
>     backtrace (crc cd8844d4):
>       kmem_cache_alloc_noprof+0x2f4/0x3f0
>       alloc_pid+0x54/0x3d0
>       copy_process+0xd58/0x1740
>       kernel_clone+0x99/0x3b0
>       __do_sys_clone3+0xbe/0x100
>       do_syscall_64+0x7b/0x2c0
>       entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Fix this by calling put_pid() after do_pidfd_send_signal() returns.
>
> Fixes: f08d0c3a7111 ("pidfd: add PIDFD_SELF* sentinels to refer to own thread/process")
> Signed-off-by: Adrian Huang (Lenovo) <adrianhuang0701@...il.com>

Needs a cc: Stable too I think.

LGTM and tested locally + confirmed memory leak + that this patch fixes it so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Tested-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>

> ---
>  kernel/signal.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index e2c928de7d2c..fe9190d84f28 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -4067,6 +4067,7 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig,
>  {
>  	struct pid *pid;
>  	enum pid_type type;
> +	int ret;
>
>  	/* Enforce flags be set to 0 until we add an extension. */
>  	if (flags & ~PIDFD_SEND_SIGNAL_FLAGS)
> @@ -4108,7 +4109,10 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig,
>  	}
>  	}
>
> -	return do_pidfd_send_signal(pid, sig, type, info, flags);
> +	ret = do_pidfd_send_signal(pid, sig, type, info, flags);
> +	put_pid(pid);

Yeah this seems to be a product of the default branch using CLASS(fd, ..) to
get/put the fd in scope but the PIDFF_SELF* stuff not.

> +
> +	return ret;
>  }
>
>  static int
> --
> 2.43.0
>