| Message ID | 20241113-pidfs_fh-v2-0-9a4d28155a37@e43.eu (mailing list archive) |
|---|---|
| Headers | show |
| Series | pidfs: implement file handle support | expand |
On Wed, Nov 13, 2024 at 7:01 PM Erin Shepherd <erin.shepherd@e43.eu> wrote: > > Since the introduction of pidfs, we have had 64-bit process identifiers > that will not be reused for the entire uptime of the system. This greatly > facilitates process tracking in userspace. > > There are two limitations at present: > > * These identifiers are currently only exposed to processes on 64-bit > systems. On 32-bit systems, inode space is also limited to 32 bits and > therefore is subject to the same reuse issues. > * There is no way to go from one of these unique identifiers to a pid or > pidfd. > > This patch implements fh_export and fh_to_dentry which enables userspace to > convert PIDs to and from PID file handles. A process can convert a pidfd into > a file handle using name_to_handle_at, store it (in memory, on disk, or > elsewhere) and then convert it back into a pidfd suing open_by_handle_at. > > To support us going from a file handle to a pidfd, we have to store a pid > inside the file handle. To ensure file handles are invariant and can move > between pid namespaces, we stash a pid from the initial namespace inside > the file handle. > > (There has been some discussion as to whether or not it is OK to include > the PID in the initial pid namespace, but so far there hasn't been any > conclusive reason given as to why this would be a bad idea) IIUC, this is already exposed as st_ino on a 64bit arch? If that is the case, then there is certainly no new info leak in this patch. > > Signed-off-by: Erin Shepherd <erin.shepherd@e43.eu> > --- > Changes in v2: > - Permit filesystems to opt out of CAP_DAC_READ_SEARCH > - Inline find_pid_ns/get_pid logic; remove unnecessary put_pid > - Squash fh_export & fh_to_dentry into one commit Not sure why you did that. It was pretty nice as separate commits if you ask me. Whatever. Thanks, Amir.
On 14/11/2024 08:02, Amir Goldstein wrote: > On Wed, Nov 13, 2024 at 7:01 PM Erin Shepherd <erin.shepherd@e43.eu> wrote: >> Since the introduction of pidfs, we have had 64-bit process identifiers >> that will not be reused for the entire uptime of the system. This greatly >> facilitates process tracking in userspace. >> >> There are two limitations at present: >> >> * These identifiers are currently only exposed to processes on 64-bit >> systems. On 32-bit systems, inode space is also limited to 32 bits and >> therefore is subject to the same reuse issues. >> * There is no way to go from one of these unique identifiers to a pid or >> pidfd. >> >> This patch implements fh_export and fh_to_dentry which enables userspace to >> convert PIDs to and from PID file handles. A process can convert a pidfd into >> a file handle using name_to_handle_at, store it (in memory, on disk, or >> elsewhere) and then convert it back into a pidfd suing open_by_handle_at. >> >> To support us going from a file handle to a pidfd, we have to store a pid >> inside the file handle. To ensure file handles are invariant and can move >> between pid namespaces, we stash a pid from the initial namespace inside >> the file handle. >> >> (There has been some discussion as to whether or not it is OK to include >> the PID in the initial pid namespace, but so far there hasn't been any >> conclusive reason given as to why this would be a bad idea) > IIUC, this is already exposed as st_ino on a 64bit arch? > If that is the case, then there is certainly no new info leak in this patch. pid.ino is exposed, but the init-ns pid isn't exposed anywhere to my knowledge. >> Signed-off-by: Erin Shepherd <erin.shepherd@e43.eu> >> --- >> Changes in v2: >> - Permit filesystems to opt out of CAP_DAC_READ_SEARCH >> - Inline find_pid_ns/get_pid logic; remove unnecessary put_pid >> - Squash fh_export & fh_to_dentry into one commit > Not sure why you did that. > It was pretty nice as separate commits if you ask me. Whatever. I can revert that if you prefer. I squashed them because there was some churn when adding the init-ns-pid necessary to restore them, but I am happy to do things in two steps. Do you prefer having the final handle format in the first step, or letting it evolve into final form over the series?
On Thu, Nov 14, 2024 at 01:48:02PM +0100, Erin Shepherd wrote: > > > On 14/11/2024 08:02, Amir Goldstein wrote: > > On Wed, Nov 13, 2024 at 7:01 PM Erin Shepherd <erin.shepherd@e43.eu> wrote: > >> Since the introduction of pidfs, we have had 64-bit process identifiers > >> that will not be reused for the entire uptime of the system. This greatly > >> facilitates process tracking in userspace. > >> > >> There are two limitations at present: > >> > >> * These identifiers are currently only exposed to processes on 64-bit > >> systems. On 32-bit systems, inode space is also limited to 32 bits and > >> therefore is subject to the same reuse issues. > >> * There is no way to go from one of these unique identifiers to a pid or > >> pidfd. > >> > >> This patch implements fh_export and fh_to_dentry which enables userspace to > >> convert PIDs to and from PID file handles. A process can convert a pidfd into > >> a file handle using name_to_handle_at, store it (in memory, on disk, or > >> elsewhere) and then convert it back into a pidfd suing open_by_handle_at. > >> > >> To support us going from a file handle to a pidfd, we have to store a pid > >> inside the file handle. To ensure file handles are invariant and can move > >> between pid namespaces, we stash a pid from the initial namespace inside > >> the file handle. > >> > >> (There has been some discussion as to whether or not it is OK to include > >> the PID in the initial pid namespace, but so far there hasn't been any > >> conclusive reason given as to why this would be a bad idea) > > IIUC, this is already exposed as st_ino on a 64bit arch? > > If that is the case, then there is certainly no new info leak in this patch. > > pid.ino is exposed, but the init-ns pid isn't exposed anywhere to my knowledge. I see what you mean. That might be an information leak. Not a very interesting one, I think but I need to think about it. > > >> Signed-off-by: Erin Shepherd <erin.shepherd@e43.eu> > >> --- > >> Changes in v2: > >> - Permit filesystems to opt out of CAP_DAC_READ_SEARCH > >> - Inline find_pid_ns/get_pid logic; remove unnecessary put_pid > >> - Squash fh_export & fh_to_dentry into one commit > > Not sure why you did that. > > It was pretty nice as separate commits if you ask me. Whatever. > > I can revert that if you prefer. I squashed them because there was some churn > when adding the init-ns-pid necessary to restore them, but I am happy to do > things in two steps. > > Do you prefer having the final handle format in the first step, or letting it > evolve into final form over the series? >
On Thu, Nov 14, 2024 at 4:51 PM Erin Shepherd <erin.shepherd@e43.eu> wrote: > > > > On 14/11/2024 08:02, Amir Goldstein wrote: > > On Wed, Nov 13, 2024 at 7:01 PM Erin Shepherd <erin.shepherd@e43.eu> wrote: > >> Since the introduction of pidfs, we have had 64-bit process identifiers > >> that will not be reused for the entire uptime of the system. This greatly > >> facilitates process tracking in userspace. > >> > >> There are two limitations at present: > >> > >> * These identifiers are currently only exposed to processes on 64-bit > >> systems. On 32-bit systems, inode space is also limited to 32 bits and > >> therefore is subject to the same reuse issues. > >> * There is no way to go from one of these unique identifiers to a pid or > >> pidfd. > >> > >> This patch implements fh_export and fh_to_dentry which enables userspace to > >> convert PIDs to and from PID file handles. A process can convert a pidfd into > >> a file handle using name_to_handle_at, store it (in memory, on disk, or > >> elsewhere) and then convert it back into a pidfd suing open_by_handle_at. > >> > >> To support us going from a file handle to a pidfd, we have to store a pid > >> inside the file handle. To ensure file handles are invariant and can move > >> between pid namespaces, we stash a pid from the initial namespace inside > >> the file handle. > >> > >> (There has been some discussion as to whether or not it is OK to include > >> the PID in the initial pid namespace, but so far there hasn't been any > >> conclusive reason given as to why this would be a bad idea) > > IIUC, this is already exposed as st_ino on a 64bit arch? > > If that is the case, then there is certainly no new info leak in this patch. > > pid.ino is exposed, but the init-ns pid isn't exposed anywhere to my knowledge. > > >> Signed-off-by: Erin Shepherd <erin.shepherd@e43.eu> > >> --- > >> Changes in v2: > >> - Permit filesystems to opt out of CAP_DAC_READ_SEARCH > >> - Inline find_pid_ns/get_pid logic; remove unnecessary put_pid > >> - Squash fh_export & fh_to_dentry into one commit > > Not sure why you did that. > > It was pretty nice as separate commits if you ask me. Whatever. > > I can revert that if you prefer. I squashed them because there was some churn > when adding the init-ns-pid necessary to restore them, but I am happy to do > things in two steps. > > Do you prefer having the final handle format in the first step, or letting it > evolve into final form over the series? > I don't really mind. Was just wondering. Either way is fine. Thanks, Amir.
Since the introduction of pidfs, we have had 64-bit process identifiers that will not be reused for the entire uptime of the system. This greatly facilitates process tracking in userspace. There are two limitations at present: * These identifiers are currently only exposed to processes on 64-bit systems. On 32-bit systems, inode space is also limited to 32 bits and therefore is subject to the same reuse issues. * There is no way to go from one of these unique identifiers to a pid or pidfd. This patch implements fh_export and fh_to_dentry which enables userspace to convert PIDs to and from PID file handles. A process can convert a pidfd into a file handle using name_to_handle_at, store it (in memory, on disk, or elsewhere) and then convert it back into a pidfd suing open_by_handle_at. To support us going from a file handle to a pidfd, we have to store a pid inside the file handle. To ensure file handles are invariant and can move between pid namespaces, we stash a pid from the initial namespace inside the file handle. (There has been some discussion as to whether or not it is OK to include the PID in the initial pid namespace, but so far there hasn't been any conclusive reason given as to why this would be a bad idea) Signed-off-by: Erin Shepherd <erin.shepherd@e43.eu> --- Changes in v2: - Permit filesystems to opt out of CAP_DAC_READ_SEARCH - Inline find_pid_ns/get_pid logic; remove unnecessary put_pid - Squash fh_export & fh_to_dentry into one commit - Link to v1: https://lore.kernel.org/r/2aa94713-c12a-4344-a45c-a01f26e16a0d@e43.eu Remaining: To address the PIDFD_THREAD question - Do we want to stash it in file handles (IMO no but there may be merits to doing so) - If not do we want PIDFD_THREAD/O_EXCL to open_by_handle_at to work or do we do something else? (Perhaps we could just add an ioctl which lets an app change the PIDFD_THREAD flag?) If PIDFD_SELF lands <https://lore.kernel.org/r/cover.1727644404.git.lorenzo.stoakes@oracle.com>: - Support for PIDFD_SELF(_THREAD) as reference fd in open_by_handle_at? (We can even detect that special case early there and bypass most of the file handle logic) --- Erin Shepherd (3): pseudofs: add support for export_ops exportfs: allow fs to disable CAP_DAC_READ_SEARCH check pidfs: implement file handle support fs/fhandle.c | 36 +++++++++++++++------------ fs/libfs.c | 1 + fs/pidfs.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++- include/linux/exportfs.h | 3 +++ include/linux/pseudo_fs.h | 1 + 5 files changed, 87 insertions(+), 16 deletions(-) --- base-commit: 14b6320953a3f856a3f93bf9a0e423395baa593d change-id: 20241112-pidfs_fh-fbb04b108a2b Best regards,