From patchwork Tue Oct 15 14:13:31 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Brauner X-Patchwork-Id: 11190441 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 89E6B14DB for ; Tue, 15 Oct 2019 14:13:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 723C720650 for ; Tue, 15 Oct 2019 14:13:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732563AbfJOONn (ORCPT ); Tue, 15 Oct 2019 10:13:43 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:39662 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732547AbfJOONm (ORCPT ); Tue, 15 Oct 2019 10:13:42 -0400 Received: from [213.220.153.21] (helo=localhost.localdomain) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1iKNZy-0004YA-L2; Tue, 15 Oct 2019 14:13:38 +0000 From: Christian Brauner To: linux-kernel@vger.kernel.org Cc: Shuah Khan , Andrew Morton , "Peter Zijlstra (Intel)" , Ingo Molnar , Michal Hocko , Thomas Gleixner , Elena Reshetova , Christian Kellner , Roman Gushchin , Andrea Arcangeli , Al Viro , Aleksa Sarai , "Dmitry V. Levin" , linux-kselftest@vger.kernel.org, Christian Brauner , Jann Horn , Oleg Nesterov , linux-api@vger.kernel.org Subject: [PATCH 1/2] pidfd: verify task is alive when printing fdinfo Date: Tue, 15 Oct 2019 16:13:31 +0200 Message-Id: <20191015141332.4055-1-christian.brauner@ubuntu.com> X-Mailer: git-send-email 2.23.0 MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Currently, when a task is dead we still print the pid it used to use in the fdinfo files of its pidfds. This doesn't make much sense since the pid may have already been reused. So verify that the task is still alive. If the task is not alive anymore, we will print -1. This allows us to differentiate between a task not being present in a given pid namespace - in which case we already print 0 - and a task having been reaped. Note that this uses PIDTYPE_PID for the check. Technically, we could've checked PIDTYPE_TGID since pidfds currently only refer to thread-group leaders but if they won't anymore in the future then this check becomes problematic without it being immediately obvious to non-experts imho. If a thread is created via clone(CLONE_THREAD) than struct pid has a single non-empty list pid->tasks[PIDTYPE_PID] and this pid can't be used as a PIDTYPE_TGID meaning pid->tasks[PIDTYPE_TGID] will return NULL even though the thread-group leader might still be very much alive. We could be more complicated and do something like: bool alive = false; rcu_read_lock(); struct task_struct *tsk = pid_task(pid, PIDTYPE_PID); if (tsk && task_tgid(tsk)) alive = true; rcu_read_unlock(); but it's really not worth it. We already have created a pidfd and we thus know it refers to a thread-group leader. Checking PIDTYPE_PID is fine and is easier to maintain should we ever allow pidfds to refer to threads. Cc: Jann Horn Cc: Christian Kellner Cc: Oleg Nesterov Cc: linux-api@vger.kernel.org Signed-off-by: Christian Brauner --- kernel/fork.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/kernel/fork.c b/kernel/fork.c index 782986962d47..a67944a5e542 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1695,6 +1695,18 @@ static int pidfd_release(struct inode *inode, struct file *file) } #ifdef CONFIG_PROC_FS +static inline bool task_alive(struct pid *pid) +{ + bool alive = true; + + rcu_read_lock(); + if (!pid_task(pid, PIDTYPE_PID)) + alive = false; + rcu_read_unlock(); + + return alive; +} + /** * pidfd_show_fdinfo - print information about a pidfd * @m: proc fdinfo file @@ -1732,15 +1744,20 @@ static int pidfd_release(struct inode *inode, struct file *file) */ static void pidfd_show_fdinfo(struct seq_file *m, struct file *f) { - struct pid_namespace *ns = proc_pid_ns(file_inode(m->file)); struct pid *pid = f->private_data; - pid_t nr = pid_nr_ns(pid, ns); + struct pid_namespace *ns; + pid_t nr = -1; + + if (likely(task_alive(pid))) { + ns = proc_pid_ns(file_inode(m->file)); + nr = pid_nr_ns(pid, ns); + } - seq_put_decimal_ull(m, "Pid:\t", nr); + seq_put_decimal_ll(m, "Pid:\t", nr); #ifdef CONFIG_PID_NS - seq_put_decimal_ull(m, "\nNSpid:\t", nr); - if (nr) { + seq_put_decimal_ll(m, "\nNSpid:\t", nr); + if (nr > 0) { int i; /* If nr is non-zero it means that 'pid' is valid and that @@ -1749,7 +1766,7 @@ static void pidfd_show_fdinfo(struct seq_file *m, struct file *f) * Start at one below the already printed level. */ for (i = ns->level + 1; i <= pid->level; i++) - seq_put_decimal_ull(m, "\t", pid->numbers[i].nr); + seq_put_decimal_ll(m, "\t", pid->numbers[i].nr); } #endif seq_putc(m, '\n'); From patchwork Tue Oct 15 14:13:32 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Brauner X-Patchwork-Id: 11190443 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 198E214DB for ; Tue, 15 Oct 2019 14:13:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0082C2054F for ; Tue, 15 Oct 2019 14:13:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732500AbfJOONx (ORCPT ); Tue, 15 Oct 2019 10:13:53 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:39661 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732546AbfJOONl (ORCPT ); Tue, 15 Oct 2019 10:13:41 -0400 Received: from [213.220.153.21] (helo=localhost.localdomain) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1iKNZz-0004YA-8W; Tue, 15 Oct 2019 14:13:39 +0000 From: Christian Brauner To: linux-kernel@vger.kernel.org Cc: Shuah Khan , Andrew Morton , "Peter Zijlstra (Intel)" , Ingo Molnar , Michal Hocko , Thomas Gleixner , Elena Reshetova , Christian Kellner , Roman Gushchin , Andrea Arcangeli , Al Viro , Aleksa Sarai , "Dmitry V. Levin" , linux-kselftest@vger.kernel.org, Christian Brauner Subject: [PATCH 2/2] test: verify fdinfo for pidfd of reaped process Date: Tue, 15 Oct 2019 16:13:32 +0200 Message-Id: <20191015141332.4055-2-christian.brauner@ubuntu.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191015141332.4055-1-christian.brauner@ubuntu.com> References: <20191015141332.4055-1-christian.brauner@ubuntu.com> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Test that the fdinfo field of a pidfd referring to a dead process correctly shows Pid: -1 and NSpid: -1. Cc: Christian Kellner Signed-off-by: Christian Brauner Reviewed-by: Christian Kellner --- .../selftests/pidfd/pidfd_fdinfo_test.c | 59 ++++++++++++++----- 1 file changed, 45 insertions(+), 14 deletions(-) diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c index 3721be994abd..22558524f71c 100644 --- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c +++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c @@ -113,11 +113,15 @@ static struct child clone_newns(int (*fn)(void *), void *args, return ret; } +static inline void child_close(struct child *child) +{ + close(child->fd); +} + static inline int child_join(struct child *child, struct error *err) { int r; - (void)close(child->fd); r = wait_for_pid(child->pid); if (r < 0) error_set(err, PIDFD_ERROR, "waitpid failed (ret %d, errno %d)", @@ -128,6 +132,12 @@ static inline int child_join(struct child *child, struct error *err) return r; } +static inline int child_join_close(struct child *child, struct error *err) +{ + child_close(child); + return child_join(child, err); +} + static inline void trim_newline(char *str) { char *pos = strrchr(str, '\n'); @@ -136,8 +146,8 @@ static inline void trim_newline(char *str) *pos = '\0'; } -static int verify_fdinfo_nspid(int pidfd, struct error *err, - const char *expect, ...) +static int verify_fdinfo(int pidfd, struct error *err, const char *prefix, + size_t prefix_len, const char *expect, ...) { char buffer[512] = {0, }; char path[512] = {0, }; @@ -160,17 +170,20 @@ static int verify_fdinfo_nspid(int pidfd, struct error *err, pidfd); while (getline(&line, &n, f) != -1) { - if (strncmp(line, "NSpid:", 6)) + char *val; + + if (strncmp(line, prefix, prefix_len)) continue; found = 1; - r = strcmp(line + 6, buffer); + val = line + prefix_len; + r = strcmp(val, buffer); if (r != 0) { trim_newline(line); trim_newline(buffer); - error_set(err, PIDFD_FAIL, "NSpid: '%s' != '%s'", - line + 6, buffer); + error_set(err, PIDFD_FAIL, "%s '%s' != '%s'", + prefix, val, buffer); } break; } @@ -179,8 +192,8 @@ static int verify_fdinfo_nspid(int pidfd, struct error *err, fclose(f); if (found == 0) - return error_set(err, PIDFD_FAIL, "NSpid not found for fd %d", - pidfd); + return error_set(err, PIDFD_FAIL, "%s not found for fd %d", + prefix, pidfd); return PIDFD_PASS; } @@ -213,7 +226,7 @@ static int child_fdinfo_nspid_test(void *args) } pidfd = *(int *)args; - r = verify_fdinfo_nspid(pidfd, &err, "\t0\n"); + r = verify_fdinfo(pidfd, &err, "NSpid:", 6, "\t0\n"); if (r != PIDFD_PASS) ksft_print_msg("NSpid fdinfo check failed: %s\n", err.msg); @@ -242,24 +255,42 @@ static void test_pidfd_fdinfo_nspid(void) /* The children will have pid 1 in the new pid namespace, * so the line must be 'NSPid:\t\t1'. */ - verify_fdinfo_nspid(a.fd, &err, "\t%d\t%d\n", a.pid, 1); - verify_fdinfo_nspid(b.fd, &err, "\t%d\t%d\n", b.pid, 1); + verify_fdinfo(a.fd, &err, "NSpid:", 6, "\t%d\t%d\n", a.pid, 1); + verify_fdinfo(b.fd, &err, "NSpid:", 6, "\t%d\t%d\n", b.pid, 1); /* wait for the process, check the exit status and set * 'err' accordingly, if it is not already set. */ + child_join_close(&a, &err); + child_join_close(&b, &err); + + error_report(&err, test_name); +} + +static void test_pidfd_dead_fdinfo(void) +{ + struct child a; + struct error err = {0, }; + const char *test_name = "pidfd check fdinfo for dead process"; + + /* Create a new child in a new pid and mount namespace */ + a = clone_newns(child_fdinfo_nspid_test, NULL, &err); + error_check(&err, test_name); child_join(&a, &err); - child_join(&b, &err); + verify_fdinfo(a.fd, &err, "Pid:", 4, "\t-1\n"); + verify_fdinfo(a.fd, &err, "NSpid:", 6, "\t-1\n"); + child_close(&a); error_report(&err, test_name); } int main(int argc, char **argv) { ksft_print_header(); - ksft_set_plan(1); + ksft_set_plan(2); test_pidfd_fdinfo_nspid(); + test_pidfd_dead_fdinfo(); return ksft_exit_pass(); } From patchwork Wed Oct 16 15:36:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Brauner X-Patchwork-Id: 11193635 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id DE4CB1390 for ; Wed, 16 Oct 2019 15:37:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BEE32207FF for ; Wed, 16 Oct 2019 15:37:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2405144AbfJPPhT (ORCPT ); Wed, 16 Oct 2019 11:37:19 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:47479 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2404935AbfJPPhT (ORCPT ); Wed, 16 Oct 2019 11:37:19 -0400 Received: from [213.220.153.21] (helo=localhost.localdomain) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1iKlMU-00050w-4Z; Wed, 16 Oct 2019 15:37:18 +0000 From: Christian Brauner To: oleg@redhat.com, linux-kernel@vger.kernel.org Cc: aarcange@redhat.com, akpm@linux-foundation.org, christian@kellner.me, cyphar@cyphar.com, elena.reshetova@intel.com, guro@fb.com, jannh@google.com, ldv@altlinux.org, linux-api@vger.kernel.org, linux-kselftest@vger.kernel.org, mhocko@suse.com, mingo@kernel.org, peterz@infradead.org, shuah@kernel.org, tglx@linutronix.de, viro@zeniv.linux.org.uk, Christian Brauner Subject: [PATCH v2 4/5] exit: use task_alive() in do_wait() Date: Wed, 16 Oct 2019 17:36:05 +0200 Message-Id: <20191016153606.2326-4-christian.brauner@ubuntu.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191016153606.2326-1-christian.brauner@ubuntu.com> References: <20191015141332.4055-1-christian.brauner@ubuntu.com> <20191016153606.2326-1-christian.brauner@ubuntu.com> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Replace hlist_empty() with the new task_alive() helper which is more idiomatic, easier to grep for, and unifies how callers perform this check. Cc: Oleg Nesterov Signed-off-by: Christian Brauner --- /* v1 */ patch not present /* v2 */ patch introduced --- kernel/exit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/exit.c b/kernel/exit.c index a46a50d67002..2bb0cbe94bda 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -1457,7 +1457,7 @@ static long do_wait(struct wait_opts *wo) */ wo->notask_error = -ECHILD; if ((wo->wo_type < PIDTYPE_MAX) && - (!wo->wo_pid || hlist_empty(&wo->wo_pid->tasks[wo->wo_type]))) + (!wo->wo_pid || !task_alive(wo->wo_pid, wo->wo_type))) goto notask; set_current_state(TASK_INTERRUPTIBLE); From patchwork Wed Oct 16 15:36:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christian Brauner X-Patchwork-Id: 11193637 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6E0C81390 for ; Wed, 16 Oct 2019 15:37:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 59C9521A49 for ; Wed, 16 Oct 2019 15:37:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2405279AbfJPPhV (ORCPT ); Wed, 16 Oct 2019 11:37:21 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:47481 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2405140AbfJPPhU (ORCPT ); Wed, 16 Oct 2019 11:37:20 -0400 Received: from [213.220.153.21] (helo=localhost.localdomain) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1iKlMU-00050w-QB; Wed, 16 Oct 2019 15:37:18 +0000 From: Christian Brauner To: oleg@redhat.com, linux-kernel@vger.kernel.org Cc: aarcange@redhat.com, akpm@linux-foundation.org, christian@kellner.me, cyphar@cyphar.com, elena.reshetova@intel.com, guro@fb.com, jannh@google.com, ldv@altlinux.org, linux-api@vger.kernel.org, linux-kselftest@vger.kernel.org, mhocko@suse.com, mingo@kernel.org, peterz@infradead.org, shuah@kernel.org, tglx@linutronix.de, viro@zeniv.linux.org.uk, Christian Brauner Subject: [PATCH v2 5/5] pid: use task_alive() in pidfd_open() Date: Wed, 16 Oct 2019 17:36:06 +0200 Message-Id: <20191016153606.2326-5-christian.brauner@ubuntu.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191016153606.2326-1-christian.brauner@ubuntu.com> References: <20191015141332.4055-1-christian.brauner@ubuntu.com> <20191016153606.2326-1-christian.brauner@ubuntu.com> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Use the new task_alive() helper in pidfd_open(). This simplifies the code and avoids taking rcu_read_{lock,unlock}() and leads to overall nicer code. Cc: Oleg Nesterov Signed-off-by: Christian Brauner --- /* v1 */ patch not present /* v2 */ patch introduced --- kernel/pid.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/kernel/pid.c b/kernel/pid.c index 70ad4a9f728c..1f425b6c4c47 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -497,7 +497,7 @@ static int pidfd_create(struct pid *pid) */ SYSCALL_DEFINE2(pidfd_open, pid_t, pid, unsigned int, flags) { - int fd, ret; + int fd; struct pid *p; if (flags) @@ -510,13 +510,11 @@ SYSCALL_DEFINE2(pidfd_open, pid_t, pid, unsigned int, flags) if (!p) return -ESRCH; - ret = 0; - rcu_read_lock(); - if (!pid_task(p, PIDTYPE_TGID)) - ret = -EINVAL; - rcu_read_unlock(); + if (task_alive(p, PIDTYPE_TGID)) + fd = pidfd_create(p); + else + fd = -EINVAL; - fd = ret ?: pidfd_create(p); put_pid(p); return fd; }