From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 68C9BD1F9C6 for ; Tue, 15 Oct 2024 19:40:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AF5846B007B; Tue, 15 Oct 2024 15:40:58 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A7E976B0082; Tue, 15 Oct 2024 15:40:58 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8F8ED6B0083; Tue, 15 Oct 2024 15:40:58 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 6DD606B007B for ; Tue, 15 Oct 2024 15:40:58 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id BA4001A181B for ; Tue, 15 Oct 2024 19:40:41 +0000 (UTC) X-FDA: 82676854386.12.6A4E2FD Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) by imf23.hostedemail.com (Postfix) with ESMTP id 539ED140015 for ; Tue, 15 Oct 2024 19:40:51 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=OB7qCqMu; spf=pass (imf23.hostedemail.com: domain of surenb@google.com designates 209.85.214.174 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1729021113; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=ceSrgwFmVnFYDMnDeioPXKsc+zj1mu3wINeJBmSiYkI=; b=J+eeANI6oxEwwQGjYJ+G88b5MsOx9wKQgAZ7n4R7qBFAIG8K9UulMf/cfp4qAT3zso6Vfa rnbaIWFoVM7jCZWldeX36x0Bj2reK/5UtyqOm5QcAGMaLcF6fMGoCW4vTStGNB9TJWhCnm regldUy2OHaXdSewsd3FfqfGHTIP8N4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1729021113; a=rsa-sha256; cv=none; b=7K37i3oFPmR8VM5zIf+A+nAeyd+QawFNnDJZjqxt8ns1duxvGKGzHV/hHFbGjBkYjXJC93 gT6fc9Ddu46YGQgSCS5pm9GJG0empPkwpDaCHXYIEtALkhAeDE5bs/W156D/m8EmxVx2+5 L5NP2GqeQHzW/7S9S6NWJZSktupuK5Q= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=OB7qCqMu; spf=pass (imf23.hostedemail.com: domain of surenb@google.com designates 209.85.214.174 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-pl1-f174.google.com with SMTP id d9443c01a7336-20ca03687fdso602625ad.0 for ; Tue, 15 Oct 2024 12:40:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1729021255; x=1729626055; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=ceSrgwFmVnFYDMnDeioPXKsc+zj1mu3wINeJBmSiYkI=; b=OB7qCqMuxlK2UOSozR+IhPTQqApbZ8Va2xdpu1Jwf5bUzDT8MKJmXT3XKaQxNRF1hW UXdQ7vECCqlbzPKVDRtXSsgqzdiGJugMIwx16rRzwMVvqKp3N1k+XjNwLH3p/g4vDsuO 5mTLHhcY6n9UfRlVqUuZAqRUw/VY79dNaTEweVdd+41MUxOp7uWZ2Wq87LsYTbZpbF2P NIIO6dYgdKMErUSlsnBTxOeNVl+ZVSyW5Cs6FBju8ExsWPtQJZPeLLkmgeyOV1RrHPbq cOkFH9ANly0DE//Ldi/AeCKpEehMJjQ8nMeEfPR4cypawxv8ksaxfeDwhMHe9BbvkTg5 UoWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1729021255; x=1729626055; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=ceSrgwFmVnFYDMnDeioPXKsc+zj1mu3wINeJBmSiYkI=; b=CWDo/R2o6cUVXudZNNVwkoVpTxJ0GWItDN7cXb8BrFXSdsqP1tBAvCu0aPMvMnAJ89 st7ymYfeChEk9DTAeOLc8pYRxsXtKbAJForAzKH5elk8jne/4JnX4KlxEd1fS88Ay6AO O4ptsu7LAbhGWDtwTK2Lp9gPyQ2pIyTadadLucTawqFdUYE6+Dm6LszEJpU0e/DHI2Po 61gB8HCAHYcs/niD+025xandnqMsI9hLJKrFziYdB4uW2GGn/BbnyFS3tkeur+cmzVC2 N1AL6/HPemMCpkRNGQ5gAISuH09aH4A7JrX/cb05p94q37N8YvwZCRNuW5BhzD/5yspU z/AA== X-Forwarded-Encrypted: i=1; AJvYcCV4acxSlHE1Z4Pl+QWj5UfqeaDD6/ywNb2AFHNMtmI4NTOX+pKmc86slCZWBKBFVHg2lAhIpMUUyw==@kvack.org X-Gm-Message-State: AOJu0YyWN3WFksPwxZjjdXJx0FMwTjs5OjxtSMiOAVgQjsNPc5cLX3VP /Tlgv5IDHRXCnkPqlberamVLHM5mhjT/8aYykvbxLFgikd9jRDY6vpIx351h/PfQ8yfUR6hi4W3 13mxMynOn6FoMj5x0z8X3OzJXc1/w3Hn09nLX X-Google-Smtp-Source: AGHT+IHxCAq4eZcP1OzyDqLt5O4MIrDNock6f5an6pNRsDIMwufvJRat1zbWgpWXbP0gwK1J8tBn2vAWiTLIW7GCC6g= X-Received: by 2002:a17:902:f552:b0:202:3a49:ace7 with SMTP id d9443c01a7336-20d2d05cba5mr517545ad.20.1729021254327; Tue, 15 Oct 2024 12:40:54 -0700 (PDT) MIME-Version: 1.0 References: <8e7edaf2f648fb01a71def749f17f76c0502dee1.1728643714.git.lorenzo.stoakes@oracle.com> In-Reply-To: <8e7edaf2f648fb01a71def749f17f76c0502dee1.1728643714.git.lorenzo.stoakes@oracle.com> From: Suren Baghdasaryan Date: Tue, 15 Oct 2024 12:40:41 -0700 Message-ID: Subject: Re: [PATCH v2 1/3] pidfd: extend pidfd_get_pid() and de-duplicate pid lookup To: Lorenzo Stoakes Cc: Christian Brauner , Shuah Khan , "Liam R . Howlett" , Vlastimil Babka , pedro.falcato@gmail.com, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 539ED140015 X-Stat-Signature: re6zigogrcrkeuo68dx9owkgpe1dep7h X-Rspamd-Server: rspam09 X-Rspam-User: X-HE-Tag: 1729021251-695632 X-HE-Meta: U2FsdGVkX18n08v501VzpQOOTHjKS0emh29KCBXBTaN69/LYny+hhMU1MCNIAml2F7SpK3WFUv9AVh6jwvILRzEUWgP9gnOS4DOdKNAO4a+DSAEdnsi5hWGd29leu+1FJ2Pebv0ePGV6KJNAhf2Maq+cPxrqM2ypjxKM42tUjEEiiQXd0CF2vjT+WWVhp6DesDO156IAEkBA6hDDwlJOr2oErXZfNIl2Cq8Jvr6XMG2BJ3+FVfSHqc5idtV07YWliZVD5qoreAJXuhRcgOlZRc9mGr+/s8WV8/PzzICC/fKi8GqyYzsaAMWB6Au8BpWEvUDb7SKDxgCbpAMBHYaKa6bJNrJGldVW8lePZ1GI6gtMPalr2vG6u57bCe0DKXynrHJhbfqkBFQqCB/eqPXY75lmj239ClomDO4ieJGL6OSb+/HFhMpTnaWFDU9Y31hcfXDzBaJoU4Tw9PfifQ+FAkQyl/qlUB+C+v2fZ5DZPy3XAdFLOKPkLy7E3UKiT/CrapIVOTnYL1lDLU+dPfAuL1bk9zQ/H83SJHMqBP/vFgvPU1CcAW4drBG3h8NrUQiGEGpWE9QFr1f7b/6/pAsFQuDuQzIgzaYtZZnTjALto+HKdh7akNj9uPuv9x0XpQqWMCRZHVhoynGtH+7M8Lw2hs3X+z+XyvaAiwtKrV2PNH6N8FiI4W8R5VT6d0eP3UQjzgXvPZewhBTCgPAeKEtY3HpEHTNyjFz3pld6oK6UfChTx3uxOR8IgdovOHE8oHq7MQWhUJHEbCMhgWTGGYY0hrm3Sj5RMANdLIxeSGonz3MEAUFU/6Qdy7hZYMxC4X62dnRfe77yVxjWiKHYP8CPhygebx4TyHAHqog9PHv9pHd6SUihdcq8cDwUoXlypeGxhU3bymnXS2e9LAwWIqyvdJ1SF2VZeYUYgA6J82qGMLKym7j7UHJCz8RToHyjxKiBpfvBIhpX/wdMh7tdCzV TCUFMAXu L3kW9kxX8/zPycoXo+yDVVU/aPTl/bvG44U6fWqW8gngarb2K8Y4xD16pg/taNAI0mF9gX05QavNfQin3befZo+bnxcE1PtclAXEA0YCm6r8tb8oGSHPCGGUu1+vcxIzlGEkyeTk5VrLhbHYnYfcJBA0lCs1F7iMRveiF3Stw4db2TichDYEFyQX5V+Stnk4Gd7Mb3H+6wC/zpCpcCA7iI+IQ0jDg0PKJs7HI0UCdBy2ERHvZf6wJpeBEvuyPq+VegwTax4tf2hq8DGYdTtlt+K4djuEWc23F6Q/nNZwXQAl6FicTzoDUOPzBfddw9an969BBWmW+PJiwuWIM9nnTIBrXpw== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Fri, Oct 11, 2024 at 4:06=E2=80=AFAM Lorenzo Stoakes wrote: > > The means by which a pid is determined from a pidfd is duplicated, with > some callers holding a reference to the (pid)fd, and others explicitly > pinning the pid. > > Introduce __pidfd_get_pid() which abstracts both approaches and provide > optional output parameters for file->f_flags and the fd (the latter of > which, if provided, prevents the function from decrementing the fd's > refernce count). > > Additionally, allow the ability to open a pidfd by opening a /proc/ > directory, utilised by the pidfd_send_signal() system call, providing a > pidfd_get_pid_proc() helper function to do so. > > Doing this allows us to eliminate open-coded pidfd pid lookup and to > consistently handle this in one place. > > This lays the groundwork for a subsequent patch which adds a new sentinel > pidfd to explicitly reference the current process (i.e. thread group > leader) without the need for a pidfd. > > Signed-off-by: Lorenzo Stoakes > --- > include/linux/pid.h | 42 +++++++++++++++++++++++++++++++- > kernel/pid.c | 58 ++++++++++++++++++++++++++++++--------------- > kernel/signal.c | 22 ++++------------- > 3 files changed, 84 insertions(+), 38 deletions(-) > > diff --git a/include/linux/pid.h b/include/linux/pid.h > index a3aad9b4074c..68b02eab7509 100644 > --- a/include/linux/pid.h > +++ b/include/linux/pid.h > @@ -2,6 +2,7 @@ > #ifndef _LINUX_PID_H > #define _LINUX_PID_H > > +#include > #include > #include > #include > @@ -72,8 +73,47 @@ extern struct pid init_struct_pid; > > struct file; > > + > +/** > + * __pidfd_get_pid() - Retrieve a pid associated with the specified pidf= d. > + * > + * @pidfd: The pidfd whose pid we want, or the fd of a /proc/ = file if > + * @alloc_proc is also set. > + * @pin_pid: If set, then the reference counter of the returned pid i= s > + * incremented. If not set, then @fd should be provided to = pin the > + * pidfd. > + * @allow_proc: If set, then an fd of a /proc/ file can be passed i= nstead > + * of a pidfd, and this will be used to determine the pid. > + * @flags: Output variable, if non-NULL, then the file->f_flags of = the > + * pidfd will be set here. > + * @fd: Output variable, if non-NULL, then the pidfd reference w= ill > + * remain elevated and the caller will need to decrement it > + * themselves. > + * > + * Returns: If successful, the pid associated with the pidfd, otherwise = an > + * error. > + */ > +struct pid *__pidfd_get_pid(unsigned int pidfd, bool pin_pid, > + bool allow_proc, unsigned int *flags, > + struct fd *fd); > + > +static inline struct pid *pidfd_get_pid(unsigned int pidfd, unsigned int= *flags) > +{ > + return __pidfd_get_pid(pidfd, /* pin_pid =3D */ true, > + /* allow_proc =3D */ false, > + flags, /* fd =3D */ NULL); > +} > + > +static inline struct pid *pidfd_to_pid_proc(unsigned int pidfd, > + unsigned int *flags, > + struct fd *fd) > +{ > + return __pidfd_get_pid(pidfd, /* pin_pid =3D */ false, > + /* allow_proc =3D */ true, > + flags, fd); > +} > + > struct pid *pidfd_pid(const struct file *file); > -struct pid *pidfd_get_pid(unsigned int fd, unsigned int *flags); > struct task_struct *pidfd_get_task(int pidfd, unsigned int *flags); > int pidfd_prepare(struct pid *pid, unsigned int flags, struct file **ret= ); > void do_notify_pidfd(struct task_struct *task); > diff --git a/kernel/pid.c b/kernel/pid.c > index 2715afb77eab..25cc1c36a1b1 100644 > --- a/kernel/pid.c > +++ b/kernel/pid.c > @@ -36,6 +36,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -534,22 +535,46 @@ struct pid *find_ge_pid(int nr, struct pid_namespac= e *ns) > } > EXPORT_SYMBOL_GPL(find_ge_pid); > > -struct pid *pidfd_get_pid(unsigned int fd, unsigned int *flags) > +struct pid *__pidfd_get_pid(unsigned int pidfd, bool pin_pid, > + bool allow_proc, unsigned int *flags, > + struct fd *fd) > { > - struct fd f; > + struct file *file; > struct pid *pid; > + struct fd f =3D fdget(pidfd); > > - f =3D fdget(fd); > - if (!fd_file(f)) > + file =3D fd_file(f); > + if (!file) > return ERR_PTR(-EBADF); > > - pid =3D pidfd_pid(fd_file(f)); > - if (!IS_ERR(pid)) { > - get_pid(pid); > - *flags =3D fd_file(f)->f_flags; > + pid =3D pidfd_pid(file); > + /* If we allow opening a pidfd via /proc/, do so. */ > + if (IS_ERR(pid) && allow_proc) > + pid =3D tgid_pidfd_to_pid(file); > + > + if (IS_ERR(pid)) { > + fdput(f); > + return pid; > } > > - fdput(f); > + if (pin_pid) > + get_pid(pid); > + else > + WARN_ON_ONCE(!fd); /* Nothing to keep pid/pidfd around? *= / > + > + if (flags) > + *flags =3D file->f_flags; > + > + /* > + * If the user provides an fd output then it will handle decremen= ting > + * its reference counter. > + */ > + if (fd) > + *fd =3D f; > + else > + /* Otherwise we release it. */ > + fdput(f); > + > return pid; > } There is an EXPORT_SYMBOL_GPL(pidfd_get_pid) right after this line. It should also be changed to EXPORT_SYMBOL_GPL(__pidfd_get_pid), otherwise __pidfd_get_pid() will not be exported. A module calling pidfd_get_pid() now inlined in the header file will try to call __pidfd_get_pid() and will have trouble resolving this symbol. > > @@ -747,23 +772,18 @@ SYSCALL_DEFINE3(pidfd_getfd, int, pidfd, int, fd, > unsigned int, flags) > { > struct pid *pid; > - struct fd f; > int ret; > > /* flags is currently unused - make sure it's unset */ > if (flags) > return -EINVAL; > > - f =3D fdget(pidfd); > - if (!fd_file(f)) > - return -EBADF; > - > - pid =3D pidfd_pid(fd_file(f)); > + pid =3D pidfd_get_pid(pidfd, NULL); > if (IS_ERR(pid)) > - ret =3D PTR_ERR(pid); > - else > - ret =3D pidfd_getfd(pid, fd); > + return PTR_ERR(pid); > > - fdput(f); > + ret =3D pidfd_getfd(pid, fd); > + > + put_pid(pid); > return ret; > } > diff --git a/kernel/signal.c b/kernel/signal.c > index 4344860ffcac..868bfa674c62 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -3875,17 +3875,6 @@ static int copy_siginfo_from_user_any(kernel_sigin= fo_t *kinfo, > return copy_siginfo_from_user(kinfo, info); > } > > -static struct pid *pidfd_to_pid(const struct file *file) > -{ > - struct pid *pid; > - > - pid =3D pidfd_pid(file); > - if (!IS_ERR(pid)) > - return pid; > - > - return tgid_pidfd_to_pid(file); > -} > - > #define PIDFD_SEND_SIGNAL_FLAGS \ > (PIDFD_SIGNAL_THREAD | PIDFD_SIGNAL_THREAD_GROUP | \ > PIDFD_SIGNAL_PROCESS_GROUP) > @@ -3908,10 +3897,11 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, in= t, sig, > siginfo_t __user *, info, unsigned int, flags) > { > int ret; > - struct fd f; > struct pid *pid; > kernel_siginfo_t kinfo; > enum pid_type type; > + unsigned int f_flags; > + struct fd f; > > /* Enforce flags be set to 0 until we add an extension. */ > if (flags & ~PIDFD_SEND_SIGNAL_FLAGS) > @@ -3921,12 +3911,8 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int= , sig, > if (hweight32(flags & PIDFD_SEND_SIGNAL_FLAGS) > 1) > return -EINVAL; > > - f =3D fdget(pidfd); > - if (!fd_file(f)) > - return -EBADF; > - > /* Is this a pidfd? */ > - pid =3D pidfd_to_pid(fd_file(f)); > + pid =3D pidfd_to_pid_proc(pidfd, &f_flags, &f); > if (IS_ERR(pid)) { > ret =3D PTR_ERR(pid); > goto err; > @@ -3939,7 +3925,7 @@ SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int,= sig, > switch (flags) { > case 0: > /* Infer scope from the type of pidfd. */ > - if (fd_file(f)->f_flags & PIDFD_THREAD) > + if (f_flags & PIDFD_THREAD) > type =3D PIDTYPE_PID; > else > type =3D PIDTYPE_TGID; > -- > 2.46.2 >