From patchwork Tue Feb 6 19:23:57 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tycho Andersen X-Patchwork-Id: 197624 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:168b:b0:106:860b:bbdd with SMTP id ma11csp1766204dyb; Tue, 6 Feb 2024 11:25:01 -0800 (PST) X-Google-Smtp-Source: AGHT+IE9sxcJLf0h36u9w0JKrG08esb64xh3SZIIw+l5lPuiFJFZXlMA8Fkku+XBRRob++efiP8u X-Received: by 2002:a17:906:4a8e:b0:a35:8596:cdc7 with SMTP id x14-20020a1709064a8e00b00a358596cdc7mr2718582eju.31.1707247501672; Tue, 06 Feb 2024 11:25:01 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1707247501; cv=pass; d=google.com; s=arc-20160816; b=GPmNigLcV6ktVPru1/poEaRGcC02OQBhFEvdY7wbtuMoUUrDIzK0J64ZqsB1u8Ejz0 jjtKe1icUU/JvT1K2XO5Ix+7/gBlYKPfqU9Izo43fooQwpRXfuU7BL1kWSw3weK9cG1a uY4oIrB8vp5dUc9RZv5VdP3iFXzdcM8GvucEkjU1SF0C/wyJVNoNg6JfFu1qSqBEM9kc SDM9YkiaQhiZSar7yHe/ne280K86zsBoJIlM95dgQQ7zi4iL+FKtEUbRHP0mMLF9Jnhe UEX9V8vv5G0qhpYK3S0MaoMlZ3jzvjCI3O3xazI/yi0Nzj0z1CmtOtHjsCsP/g5SAGSp 42Dw== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:feedback-id:dkim-signature:dkim-signature; bh=y8GyuwLWAhh7c8OATUhVeiRBYCHx0aeUuHH+DE1GXN4=; fh=9jTptbUf0OC329bkt/1IKS1tF1H+JZ2bLQrYrOwVt9s=; b=uYfGcw52p45SmZwH4ZOvmbNwg/PrQB9niFrG0r/8Ub16ieZddjUkk+AI4j507LMKNk hU0TghrnUWGBrt3patrjtD1RYJncgtJu45rLQKJ9mbzrghaLWIXKQEWT5IqQUjhkFkaw jI59jDCNxxYofQnxFU9eRh4V4RrPXr/oeuda2CxtdtucgxIJBOLopAJeQQOG4oXKArT3 8sHs86uS9k56X8f4DF1BSPE3BhdhV0xUl+R+uraozicYc94+plkwrQQ/QPvQ7FwoWNZw lwzRYhz+yxlXe/S5yiFrMPYKK0KIwhAU1Mq1Dd1Fae097L1YZKZIui+hJGskqBU+SLo2 6Spg==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@tycho.pizza header.s=fm2 header.b=O5WuoKc9; dkim=pass header.i=@messagingengine.com header.s=fm3 header.b=GTcAcPhB; arc=pass (i=1 spf=pass spfdomain=tycho.pizza dkim=pass dkdomain=tycho.pizza dkim=pass dkdomain=messagingengine.com); spf=pass (google.com: domain of linux-kernel+bounces-55553-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-55553-ouuuleilei=gmail.com@vger.kernel.org" X-Forwarded-Encrypted: i=1; AJvYcCUUBIflrYTKlJSh8fWTbmj+y2hUTuH600/wLH8A5wTe7gRhJslo5t13FO0IhxEAPszOwZR6Ihp1dvG3iQWRgl3/83rQAA== Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [2604:1380:4601:e00::3]) by mx.google.com with ESMTPS id s26-20020a170906169a00b00a3828d12f41si991959ejd.865.2024.02.06.11.25.01 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 06 Feb 2024 11:25:01 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-55553-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3; Authentication-Results: mx.google.com; dkim=pass header.i=@tycho.pizza header.s=fm2 header.b=O5WuoKc9; dkim=pass header.i=@messagingengine.com header.s=fm3 header.b=GTcAcPhB; arc=pass (i=1 spf=pass spfdomain=tycho.pizza dkim=pass dkdomain=tycho.pizza dkim=pass dkdomain=messagingengine.com); spf=pass (google.com: domain of linux-kernel+bounces-55553-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-55553-ouuuleilei=gmail.com@vger.kernel.org" Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 1F5E21F258FE for ; Tue, 6 Feb 2024 19:25:01 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id D619F14AB3; Tue, 6 Feb 2024 19:24:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=tycho.pizza header.i=@tycho.pizza header.b="O5WuoKc9"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="GTcAcPhB" Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 390FA14266; Tue, 6 Feb 2024 19:24:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=66.111.4.27 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707247482; cv=none; b=sVNegaHmOVGPRUZqzMtp08GIdA0l6YE6pmXOmyxkQmNlXwexW1U95RvRv0u3XfCzMF5OXjUnphK3QKlYE7nOl4OBXLxfcgwAR3fmSRiDdpTDL9L8Mh5E1/cC9c/0YvyYwb17k2WlDZwMKauGBXJSOoAzBnwcvftYdML4AQP2mz8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707247482; c=relaxed/simple; bh=M4fAkBlp0hjdilUomg63kXcVDzF13rsTVmWNtJq0Q7U=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=ICzHGPCc3rTPiEL88VmrmX+VXTfynR2K0+Pi9c0G8gTjuPg8v9tEAcrc+EQcP9fm9lpC1v+FFeb4v5Vg/ccCA1qy7eJ5P1C/BB1v7Y3Zf/fvLBIBzIFVf7b7iWvLDEKJ54iIUVF0bgZ8TjSTdtVPrLiYVmT6onIWd9vwkSdyDBk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tycho.pizza; spf=pass smtp.mailfrom=tycho.pizza; dkim=pass (2048-bit key) header.d=tycho.pizza header.i=@tycho.pizza header.b=O5WuoKc9; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=GTcAcPhB; arc=none smtp.client-ip=66.111.4.27 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=tycho.pizza Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=tycho.pizza Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 25D3C5C00E3; Tue, 6 Feb 2024 14:24:39 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Tue, 06 Feb 2024 14:24:39 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tycho.pizza; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:message-id:mime-version:reply-to:subject:subject:to :to; s=fm2; t=1707247479; x=1707333879; bh=y8GyuwLWAhh7c8OATUhVe iRBYCHx0aeUuHH+DE1GXN4=; b=O5WuoKc9V99yeRlIx55bptAuuOCazO9h6ocfA u9kuP14uwpPaiTfvBh1N/2JdgNACu97fe9yMW4VFpJ2WU1QlvvRFc7VOuW3d5nvX fG9WELXT6HuOPfDuGMapcjBz/IHm4uaTR9CTGVbG7ncX+EAHD2Ay/iuaLnhykFHj Sj0ayjBEtpz/DNAl3zs+LH6gcg7gS5CXm2qPc1QId4OsWJL0pK0TqaVT4vhFKIsE QIlC3lvanE+La5roPn4jTgd4ffnllZJDNHsqFDF2RCnWu57ulpJdKxqMI0RggKgH vXFLPbuDY9Yw/Jjw9IQX1ujbWlaOQdbwH75fG8TWdj618wKtw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:message-id:mime-version:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; t=1707247479; x=1707333879; bh=y8GyuwLWAhh7c8OATUhVeiRBYCHx 0aeUuHH+DE1GXN4=; b=GTcAcPhBZv+wG8/2b/GgauapVAxz13km7GlUpXkh6Ppa iKXy31ylxSM/l0jYi5S8qp31DveyIdtsjx234I2IZtbM2HyYWetR9zS2LMzw16uV 0o9ErPn8x+HrtkVvGpaxFnP/gc5S17fP0rQ7aw7d4U3qBYJWDvM9F04450uULk2c pvrmiKzIx3ezlPjqfXcqqLyJKhu3tUNXWln18p1hgAabpDXPHCMi09aocJ/lQQhq HbT604dsI9Gpwo158kdWGolnPitakhGcZ0FCgHPQswlMtvooSsg7wtC3yRND3SrI T5mnNbz8adW40XDc/xexPbDkiTbNRg4ZxrGLPtMgJA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvledrtddtgdelgecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkffoggfgsedtkeertdertddtnecuhfhrohhmpefvhigthhhoucet nhguvghrshgvnhcuoehthigthhhosehthigthhhordhpihiiiigrqeenucggtffrrghtth gvrhhnpeehfeefheelfedtgfejgeehleeifedvgffhueduueehheeuhffhhfethfeivdeg geenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehthi gthhhosehthigthhhordhpihiiiigr X-ME-Proxy: Feedback-ID: i21f147d5:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 6 Feb 2024 14:24:37 -0500 (EST) From: Tycho Andersen To: Christian Brauner Cc: Oleg Nesterov , "Eric W . Biederman" , linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, Tycho Andersen , Tycho Andersen Subject: [PATCH v2] pidfd: getfd should always report ESRCH if a task is exiting Date: Tue, 6 Feb 2024 12:23:57 -0700 Message-Id: <20240206192357.81942-1-tycho@tycho.pizza> X-Mailer: git-send-email 2.34.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1790168614076916516 X-GMAIL-MSGID: 1790178756334808013 From: Tycho Andersen We can get EBADF from __pidfd_fget() if a task is currently exiting, which might be confusing. Let's check PF_EXITING, and just report ESRCH if so. I chose PF_EXITING, because it is set in exit_signals(), which is called before exit_files(). Since ->exit_status is mostly set after exit_files() in exit_notify(), using that still leaves a window open for the race. Signed-off-by: Tycho Andersen v2: fix a race in the check by putting the check after __pidfd_fget() (thanks Oleg) Signed-off-by: Tycho Andersen Signed-off-by: Christian Brauner Reviewed-by: Oleg Nesterov --- kernel/pid.c | 17 +++++++++- .../selftests/pidfd/pidfd_getfd_test.c | 31 ++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) base-commit: 082d11c164aef02e51bcd9c7cbf1554a8e42d9b5 diff --git a/kernel/pid.c b/kernel/pid.c index de0bf2f8d18b..a8cd6296ed6d 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -693,8 +693,23 @@ static int pidfd_getfd(struct pid *pid, int fd) file = __pidfd_fget(task, fd); put_task_struct(task); - if (IS_ERR(file)) + if (IS_ERR(file)) { + /* + * It is possible that the target thread is exiting; it can be + * either: + * 1. before exit_signals(), which gives a real fd + * 2. before exit_files() takes the task_lock() gives a real fd + * 3. after exit_files() releases task_lock(), ->files is NULL; + * this has PF_EXITING, since it was set in exit_signals(), + * __pidfd_fget() returns EBADF. + * In case 3 we get EBADF, but that really means ESRCH, since + * the task is currently exiting and has freed its files + * struct, so we fix it up. + */ + if (task->flags & PF_EXITING && PTR_ERR(file) == -EBADF) + return -ESRCH; return PTR_ERR(file); + } ret = receive_fd(file, NULL, O_CLOEXEC); fput(file); diff --git a/tools/testing/selftests/pidfd/pidfd_getfd_test.c b/tools/testing/selftests/pidfd/pidfd_getfd_test.c index 0930e2411dfb..cd51d547b751 100644 --- a/tools/testing/selftests/pidfd/pidfd_getfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_getfd_test.c @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -129,6 +130,7 @@ FIXTURE(child) * When it is closed, the child will exit. */ int sk; + bool ignore_child_result; }; FIXTURE_SETUP(child) @@ -165,10 +167,14 @@ FIXTURE_SETUP(child) FIXTURE_TEARDOWN(child) { + int ret; + EXPECT_EQ(0, close(self->pidfd)); EXPECT_EQ(0, close(self->sk)); - EXPECT_EQ(0, wait_for_pid(self->pid)); + ret = wait_for_pid(self->pid); + if (!self->ignore_child_result) + EXPECT_EQ(0, ret); } TEST_F(child, disable_ptrace) @@ -235,6 +241,29 @@ TEST(flags_set) EXPECT_EQ(errno, EINVAL); } +TEST_F(child, no_strange_EBADF) +{ + struct pollfd fds; + + self->ignore_child_result = true; + + fds.fd = self->pidfd; + fds.events = POLLIN; + + ASSERT_EQ(kill(self->pid, SIGKILL), 0); + ASSERT_EQ(poll(&fds, 1, 5000), 1); + + /* + * It used to be that pidfd_getfd() could race with the exiting thread + * between exit_files() and release_task(), and get a non-null task + * with a NULL files struct, and you'd get EBADF, which was slightly + * confusing. + */ + errno = 0; + EXPECT_EQ(sys_pidfd_getfd(self->pidfd, self->remote_fd, 0), -1); + EXPECT_EQ(errno, ESRCH); +} + #if __NR_pidfd_getfd == -1 int main(void) {