]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
coredump: fix PIDFD_INFO_COREDUMP ioctl check
authorLaura Brehm <laurajfbrehm@gmail.com>
Thu, 3 Jul 2025 12:02:44 +0000 (14:02 +0200)
committerChristian Brauner <brauner@kernel.org>
Mon, 7 Jul 2025 11:26:03 +0000 (13:26 +0200)
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 <laurabrehm@hey.com>
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 <brauner@kernel.org>
fs/pidfs.c

index 69919be1c9d8918b60670c937cee5255dd6a57f8..4625e097e3a0c0516ce50645f9cbbd8abd67684f 100644 (file)
@@ -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);