From: Laura Brehm Date: Thu, 3 Jul 2025 12:02:44 +0000 (+0200) Subject: coredump: fix PIDFD_INFO_COREDUMP ioctl check X-Git-Tag: v6.16-rc7~19^2~7 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=830a9e37cfb203aa0f73cd947eda89eda89cc48c;p=thirdparty%2Flinux.git coredump: fix PIDFD_INFO_COREDUMP ioctl check In Commit 1d8db6fd698de1f73b1a7d72aea578fdd18d9a87 ("pidfs, coredump: add PIDFD_INFO_COREDUMP"), the following code was added: if (mask & PIDFD_INFO_COREDUMP) { kinfo.mask |= PIDFD_INFO_COREDUMP; kinfo.coredump_mask = READ_ONCE(pidfs_i(inode)->__pei.coredump_mask); } [...] if (!(kinfo.mask & PIDFD_INFO_COREDUMP)) { task_lock(task); if (task->mm) kinfo.coredump_mask = pidfs_coredump_mask(task->mm->flags); task_unlock(task); } The second bit in particular looks off to me - the condition in essence checks whether PIDFD_INFO_COREDUMP was **not** requested, and if so fetches the coredump_mask in kinfo, since it's checking !(kinfo.mask & PIDFD_INFO_COREDUMP), which is unconditionally set in the earlier hunk. I'm tempted to assume the idea in the second hunk was to calculate the coredump mask if one was requested but fetched in the first hunk, in which case the check should be if ((kinfo.mask & PIDFD_INFO_COREDUMP) && !(kinfo.coredump_mask)) which might be more legibly written as if ((mask & PIDFD_INFO_COREDUMP) && !(kinfo.coredump_mask)) This could also instead be achieved by changing the first hunk to be: if (mask & PIDFD_INFO_COREDUMP) { kinfo.coredump_mask = READ_ONCE(pidfs_i(inode)->__pei.coredump_mask); if (kinfo.coredump_mask) kinfo.mask |= PIDFD_INFO_COREDUMP; } and the second hunk to: if ((mask & PIDFD_INFO_COREDUMP) && !(kinfo.mask & PIDFD_INFO_COREDUMP)) { task_lock(task); if (task->mm) { kinfo.coredump_mask = pidfs_coredump_mask(task->mm->flags); kinfo.mask |= PIDFD_INFO_COREDUMP; } task_unlock(task); } However, when looking at this, the supposition that the second hunk means to cover cases where the coredump info was requested but the first hunk failed to get it starts getting doubtful, so apologies if I'm completely off-base. This patch addresses the issue by fixing the check in the second hunk. Signed-off-by: Laura Brehm Link: https://lore.kernel.org/20250703120244.96908-3-laurabrehm@hey.com Cc: brauner@kernel.org Cc: linux-fsdevel@vger.kernel.org Signed-off-by: Christian Brauner --- diff --git a/fs/pidfs.c b/fs/pidfs.c index 69919be1c9d8..4625e097e3a0 100644 --- a/fs/pidfs.c +++ b/fs/pidfs.c @@ -319,7 +319,7 @@ static long pidfd_info(struct file *file, unsigned int cmd, unsigned long arg) if (!c) return -ESRCH; - if (!(kinfo.mask & PIDFD_INFO_COREDUMP)) { + if ((kinfo.mask & PIDFD_INFO_COREDUMP) && !(kinfo.coredump_mask)) { task_lock(task); if (task->mm) kinfo.coredump_mask = pidfs_coredump_mask(task->mm->flags);