Add pidfd support to the fanotify API
Subsystems (4)
v4 cover Add pidfd support to the fanotify API
Hey Jan/Amir,
This is V5 of the FAN_REPORT_PIDFD patch series. It contains the minor comment/commit description fixes that were picked up by Amir in the last series review [0, 1].
LTP tests for this API change can be found here [2]. Man page updates for this change can be found here [3].
[0] https://lore.kernel.org/linux-fsdevel/CAOQ4uxhnCk+FXK_e_GA=jC_0HWO+3ZdwHSi=zCa2Kpb0NDxBSg@mail.gmail.com/
[1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxgO3oViTSFZ0zs6brrHrmw362r1C9SQ7g6=XgRwyrzMuw@mail.gmail.com/
[2] https://github.com/matthewbobrowski/ltp/tree/fanotify_pidfd_v2
[3] https://github.com/matthewbobrowski/man-pages/tree/fanotify_pidfd_v1
Matthew Bobrowski (5):
kernel/pid.c: remove static qualifier from pidfd_create() kernel/pid.c: implement additional checks upon pidfd_create()
parameters
fanotify: minor cosmetic adjustments to fid labels
fanotify: introduce a generic info record copying helper
fanotify: add pidfd support to the fanotify API
fs/notify/fanotify/fanotify_user.c | 251 ++++++++++++++++++++--------- include/linux/fanotify.h | 3 + include/linux/pid.h | 1 + include/uapi/linux/fanotify.h | 13 ++ kernel/pid.c | 15 +- 5 files changed, 204 insertions(+), 79 deletions(-)
Signature · 3 lines
2.32.0.605.g8dce9f2422-goog /M
PATCH v4 1/5 kernel/pid.c: remove static qualifier from pidfd_create()
With the idea of returning pidfds from the fanotify API, we need to expose a mechanism for creating pidfds. We drop the static qualifier from pidfd_create() and add its declaration to linux/pid.h so that the pidfd_create() helper can be called from other kernel subsystems i.e. fanotify.
--- include/linux/pid.h | 1 + kernel/pid.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-)
include/linux/pid.h +1 -0
@@ -78,6 +78,7 @@ struct file;extern struct pid *pidfd_pid (const struct file *file);struct pid *pidfd_get_pid (unsigned int fd,unsigned int *flags); +int pidfd_create (struct pid *pid,unsigned int flags);static inline struct pid *get_pid (struct pid *pid) {
kernel/pid.c +3 -1
@@ -550,10 +550,12 @@ struct pid *pidfd_get_pid(unsigned int fd, unsigned int *flags)* Note, that this function can only be called after the fd table has * been unshared to avoid leaking the pidfd to the new process. * +* This symbol should not be explicitly exported to loadable modules. +* * Return: On success, a cloexec pidfd is returned. * On error, a negative errno number will be returned. */ -static int pidfd_create (struct pid *pid,unsigned int flags) +int pidfd_create (struct pid *pid,unsigned int flags) {int fd;
Signature · 3 lines
2.32.0.605.g8dce9f2422-goog /M
PATCH v4 2/5 kernel/pid.c: implement additional checks upon pidfd_create() parameters
In reply to Matthew BobrowskiBy adding the pidfd_create() declaration to linux/pid.h, we effectively expose this function to the rest of the kernel. In order to avoid any unintended behavior, or set false expectations upon this function, ensure that constraints are forced upon each of the passed parameters. This includes the checking of whether the passed struct pid is a thread-group leader as pidfd creation is currently limited to such pid types.
--- kernel/pid.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
kernel/pid.c +7 -4
@@ -559,6 +559,12 @@ int pidfd_create(struct pid *pid, unsigned int flags) {int fd; +if (!pid || !pid_has_task (pid,PIDTYPE_TGID )) +return -EINVAL ; + +if (flags & ~(O_NONBLOCK |O_RDWR |O_CLOEXEC )) +return -EINVAL ; + fd =anon_inode_getfd ("[pidfd]" , &pidfd_fops,get_pid (pid), flags |O_RDWR |O_CLOEXEC );if (fd <0 )
@@ -598,10 +604,7 @@ SYSCALL_DEFINE2(pidfd_open, pid_t, pid, unsigned int, flags)if (!p)return -ESRCH ; -if (pid_has_task (p,PIDTYPE_TGID )) - fd =pidfd_create (p, flags); -else - fd = -EINVAL ; + fd =pidfd_create (p, flags);put_pid (p);return fd;
Signature · 3 lines
2.32.0.605.g8dce9f2422-goog /M
PATCH v4 3/5 fanotify: minor cosmetic adjustments to fid labels
In reply to Matthew BobrowskiWith the idea to support additional info record types in the future i.e. fanotify_event_info_pidfd, it's a good idea to rename some of the labels assigned to some of the existing fid related functions, parameters, etc which more accurately represent the intent behind their usage.
For example, copy_info_to_user() was defined with a generic function label, which arguably reads as being supportive of different info record types, however the parameter list for this function is explicitly tailored towards the creation and copying of the fanotify_event_info_fid records. This same point applies to the macro defined as FANOTIFY_INFO_HDR_LEN.
With fanotify_event_info_len(), we change the parameter label so that the function implies that it can be extended to calculate the length for additional info record types.
--- fs/notify/fanotify/fanotify_user.c | 33 +++++++++++++++++------------- 1 file changed, 19 insertions(+), 14 deletions(-)
fs/notify/fanotify/fanotify_user.c +19 -14
@@ -104,7 +104,7 @@ struct kmem_cache *fanotify_path_event_cachep __read_mostly;struct kmem_cache *fanotify_perm_event_cachep__read_mostly ;#define FANOTIFY_EVENT_ALIGN 4 -#define FANOTIFY_INFO_HDR_LEN \ +#define FANOTIFY_FID_INFO_HDR_LEN \ (sizeof(struct fanotify_event_info_fid) + sizeof(struct file_handle))static int fanotify_fid_info_len (int fh_len,int name_len)
@@ -114,10 +114,11 @@ static int fanotify_fid_info_len(int fh_len, int name_len)if (name_len) info_len += name_len +1 ; -return roundup (FANOTIFY_INFO_HDR_LEN + info_len,FANOTIFY_EVENT_ALIGN ); +return roundup (FANOTIFY_FID_INFO_HDR_LEN + info_len, +FANOTIFY_EVENT_ALIGN ); } -static int fanotify_event_info_len (unsigned int fid_mode, +static int fanotify_event_info_len (unsigned int info_mode,struct fanotify_event *event) {struct fanotify_info *info =fanotify_event_info (event);
@@ -128,7 +129,8 @@ static int fanotify_event_info_len(unsigned int fid_mode,if (dir_fh_len) { info_len +=fanotify_fid_info_len (dir_fh_len, info->name_len); - }else if ((fid_mode &FAN_REPORT_NAME ) && (event->mask &FAN_ONDIR )) { + }else if ((info_mode &FAN_REPORT_NAME ) && + (event->mask &FAN_ONDIR )) {/* * With group flag FAN_REPORT_NAME, if name was not recorded in * event on a directory, we will report the name ".".
@@ -303,9 +305,10 @@ static int process_access_response(struct fsnotify_group *group,return -ENOENT ; } -static int copy_info_to_user (__kernel_fsid_t *fsid,struct fanotify_fh *fh, -int info_type,const char *name,size_t name_len, -char __user *buf,size_t count) +static int copy_fid_info_to_user (__kernel_fsid_t *fsid,struct fanotify_fh *fh, +int info_type,const char *name, +size_t name_len, +char __user *buf,size_t count) {struct fanotify_event_info_fid info = { };struct file_handle handle = { };
@@ -466,10 +469,11 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,if (fanotify_event_dir_fh_len (event)) { info_type = info->name_len ?FAN_EVENT_INFO_TYPE_DFID_NAME :FAN_EVENT_INFO_TYPE_DFID ; - ret =copy_info_to_user (fanotify_event_fsid (event), -fanotify_info_dir_fh (info), - info_type,fanotify_info_name (info), - info->name_len, buf, count); + ret =copy_fid_info_to_user (fanotify_event_fsid (event), +fanotify_info_dir_fh (info), + info_type, +fanotify_info_name (info), + info->name_len, buf, count);if (ret <0 ) goto out_close_fd;
@@ -515,9 +519,10 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group, info_type =FAN_EVENT_INFO_TYPE_FID ; } - ret =copy_info_to_user (fanotify_event_fsid (event), -fanotify_event_object_fh (event), - info_type, dot, dot_len, buf, count); + ret =copy_fid_info_to_user (fanotify_event_fsid (event), +fanotify_event_object_fh (event), + info_type, dot, dot_len, + buf, count);if (ret <0 ) goto out_close_fd;
Signature · 3 lines
2.32.0.605.g8dce9f2422-goog /M
PATCH v4 4/5 fanotify: introduce a generic info record copying helper
In reply to Matthew BobrowskiThe copy_info_records_to_user() helper allows for the separation of info record copying routines/conditionals from copy_event_to_user(), which reduces the overall clutter within this function. This becomes especially true as we start introducing additional info records in the future i.e. struct fanotify_event_info_pidfd. On success, this helper returns the total amount of bytes that have been copied into the user supplied buffer and on error, a negative value is returned to the caller.
The newly defined macro FANOTIFY_INFO_MODES can be used to obtain info record types that have been enabled for a specific notification group. This macro becomes useful in the subsequent patch when the FAN_REPORT_PIDFD initialization flag is introduced.
---
Changes since v3:
* Updated a typo (s/modification \n/modification) in the copy_info_records_to_user() helper.
fs/notify/fanotify/fanotify_user.c | 155 ++++++++++++++++------------- include/linux/fanotify.h | 2 + 2 files changed, 90 insertions(+), 67 deletions(-)
fs/notify/fanotify/fanotify_user.c +88 -67
@@ -173,7 +173,7 @@ static struct fanotify_event *get_one_event(struct fsnotify_group *group,size_t event_size =FAN_EVENT_METADATA_LEN ;struct fanotify_event *event =NULL ;struct fsnotify_event *fsn_event; -unsigned int fid_mode =FAN_GROUP_FLAG (group,FANOTIFY_FID_BITS ); +unsigned int info_mode =FAN_GROUP_FLAG (group,FANOTIFY_INFO_MODES );pr_debug ("%s: group=%p count=%zd\n" , __func__, group, count);
@@ -183,8 +183,8 @@ static struct fanotify_event *get_one_event(struct fsnotify_group *group, goto out; event =FANOTIFY_E (fsn_event); -if (fid_mode) - event_size +=fanotify_event_info_len (fid_mode, event); +if (info_mode) + event_size +=fanotify_event_info_len (info_mode, event);if (event_size > count) { event =ERR_PTR (-EINVAL );
@@ -401,6 +401,86 @@ static int copy_fid_info_to_user(__kernel_fsid_t *fsid, struct fanotify_fh *fh,return info_len; } +static int copy_info_records_to_user (struct fanotify_event *event, +struct fanotify_info *info, +unsigned int info_mode, +char __user *buf,size_t count) +{ +int ret, total_bytes =0 , info_type =0 ; +unsigned int fid_mode = info_mode &FANOTIFY_FID_BITS ; + +/* +* Event info records order is as follows: dir fid + name, child fid. +*/ +if (fanotify_event_dir_fh_len (event)) { + info_type = info->name_len ?FAN_EVENT_INFO_TYPE_DFID_NAME : +FAN_EVENT_INFO_TYPE_DFID ; + ret =copy_fid_info_to_user (fanotify_event_fsid (event), +fanotify_info_dir_fh (info), + info_type, +fanotify_info_name (info), + info->name_len, buf, count); +if (ret <0 ) +return ret; + + buf += ret; + count -= ret; + total_bytes += ret; + } + +if (fanotify_event_object_fh_len (event)) { +const char *dot =NULL ; +int dot_len =0 ; + +if (fid_mode ==FAN_REPORT_FID || info_type) { +/* +* With only group flag FAN_REPORT_FID only type FID is +* reported. Second info record type is always FID. +*/ + info_type =FAN_EVENT_INFO_TYPE_FID ; + }else if ((fid_mode &FAN_REPORT_NAME ) && + (event->mask &FAN_ONDIR )) { +/* +* With group flag FAN_REPORT_NAME, if name was not +* recorded in an event on a directory, report the name +* "." with info type DFID_NAME. +*/ + info_type =FAN_EVENT_INFO_TYPE_DFID_NAME ; + dot ="." ; + dot_len =1 ; + }else if ((event->mask &ALL_FSNOTIFY_DIRENT_EVENTS ) || + (event->mask &FAN_ONDIR )) { +/* +* With group flag FAN_REPORT_DIR_FID, a single info +* record has type DFID for directory entry modification +* event and for event on a directory. +*/ + info_type =FAN_EVENT_INFO_TYPE_DFID ; + }else { +/* +* With group flags FAN_REPORT_DIR_FID|FAN_REPORT_FID, +* a single info record has type FID for event on a +* non-directory, when there is no directory to report. +* For example, on FAN_DELETE_SELF event. +*/ + info_type =FAN_EVENT_INFO_TYPE_FID ; + } + + ret =copy_fid_info_to_user (fanotify_event_fsid (event), +fanotify_event_object_fh (event), + info_type, dot, dot_len, + buf, count); +if (ret <0 ) +return ret; + + buf += ret; + count -= ret; + total_bytes += ret; + } + +return total_bytes; +} +static ssize_t copy_event_to_user (struct fsnotify_group *group,struct fanotify_event *event,char __user *buf,size_t count)
@@ -408,15 +488,14 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,struct fanotify_event_metadata metadata;struct path *path =fanotify_event_path (event);struct fanotify_info *info =fanotify_event_info (event); -unsigned int fid_mode =FAN_GROUP_FLAG (group,FANOTIFY_FID_BITS ); +unsigned int info_mode =FAN_GROUP_FLAG (group,FANOTIFY_INFO_MODES );struct file *f =NULL ;int ret, fd =FAN_NOFD ; -int info_type =0 ;pr_debug ("%s: group=%p event=%p\n" , __func__, group, event); metadata.event_len =FAN_EVENT_METADATA_LEN + -fanotify_event_info_len (fid_mode, event); +fanotify_event_info_len (info_mode, event); metadata.metadata_len =FAN_EVENT_METADATA_LEN ; metadata.vers =FANOTIFY_METADATA_VERSION ; metadata.reserved =0 ;
@@ -465,69 +544,11 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,if (f)fd_install (fd, f); -/* Event info records order is: dir fid + name, child fid */ -if (fanotify_event_dir_fh_len (event)) { - info_type = info->name_len ?FAN_EVENT_INFO_TYPE_DFID_NAME : -FAN_EVENT_INFO_TYPE_DFID ; - ret =copy_fid_info_to_user (fanotify_event_fsid (event), -fanotify_info_dir_fh (info), - info_type, -fanotify_info_name (info), - info->name_len, buf, count); -if (ret <0 ) - goto out_close_fd; - - buf += ret; - count -= ret; - } - -if (fanotify_event_object_fh_len (event)) { -const char *dot =NULL ; -int dot_len =0 ; - -if (fid_mode ==FAN_REPORT_FID || info_type) { -/* -* With only group flag FAN_REPORT_FID only type FID is -* reported. Second info record type is always FID. -*/ - info_type =FAN_EVENT_INFO_TYPE_FID ; - }else if ((fid_mode &FAN_REPORT_NAME ) && - (event->mask &FAN_ONDIR )) { -/* -* With group flag FAN_REPORT_NAME, if name was not -* recorded in an event on a directory, report the -* name "." with info type DFID_NAME. -*/ - info_type =FAN_EVENT_INFO_TYPE_DFID_NAME ; - dot ="." ; - dot_len =1 ; - }else if ((event->mask &ALL_FSNOTIFY_DIRENT_EVENTS ) || - (event->mask &FAN_ONDIR )) { -/* -* With group flag FAN_REPORT_DIR_FID, a single info -* record has type DFID for directory entry modification -* event and for event on a directory. -*/ - info_type =FAN_EVENT_INFO_TYPE_DFID ; - }else { -/* -* With group flags FAN_REPORT_DIR_FID|FAN_REPORT_FID, -* a single info record has type FID for event on a -* non-directory, when there is no directory to report. -* For example, on FAN_DELETE_SELF event. -*/ - info_type =FAN_EVENT_INFO_TYPE_FID ; - } - - ret =copy_fid_info_to_user (fanotify_event_fsid (event), -fanotify_event_object_fh (event), - info_type, dot, dot_len, - buf, count); +if (info_mode) { + ret =copy_info_records_to_user (event, info, info_mode, + buf, count);if (ret <0 ) goto out_close_fd; - - buf += ret; - count -= ret; }return metadata.event_len;
include/linux/fanotify.h +2 -0
@@ -27,6 +27,8 @@ extern struct ctl_table fanotify_table[]; /* for sysctl */#define FANOTIFY_FID_BITS (FAN_REPORT_FID | FAN_REPORT_DFID_NAME) +#define FANOTIFY_INFO_MODES (FANOTIFY_FID_BITS) +/* * fanotify_init() flags that require CAP_SYS_ADMIN. * We do not allow unprivileged groups to request permission events.
Signature · 3 lines
2.32.0.605.g8dce9f2422-goog /M
PATCH v4 5/5 fanotify: add pidfd support to the fanotify API
In reply to Matthew BobrowskiIntroduce a new flag FAN_REPORT_PIDFD for fanotify_init(2) which allows userspace applications to control whether a pidfd information record containing a pidfd is to be returned alongside the generic event metadata for each event.
If FAN_REPORT_PIDFD is enabled for a notification group, an additional struct fanotify_event_info_pidfd object type will be supplied alongside the generic struct fanotify_event_metadata for a single event. This functionality is analogous to that of FAN_REPORT_FID in terms of how the event structure is supplied to a userspace application. Usage of FAN_REPORT_PIDFD with FAN_REPORT_FID/FAN_REPORT_DFID_NAME is permitted, and in this case a struct fanotify_event_info_pidfd object will likely follow any struct fanotify_event_info_fid object.
Currently, the usage of the FAN_REPORT_TID flag is not permitted along with FAN_REPORT_PIDFD as the pidfd API currently only supports the creation of pidfds for thread-group leaders. Additionally, usage of the FAN_REPORT_PIDFD flag is limited to privileged processes only i.e. event listeners that are running with the CAP_SYS_ADMIN capability. Attempting to supply the FAN_REPORT_TID initialization flags with FAN_REPORT_PIDFD or creating a notification group without CAP_SYS_ADMIN will result with -EINVAL being returned to the caller.
In the event of a pidfd creation error, there are two types of error values that can be reported back to the listener. There is FAN_NOPIDFD, which will be reported in cases where the process responsible for generating the event has terminated prior to the event listener being able to read the event. Then there is FAN_EPIDFD, which will be reported when a more generic pidfd creation error has occurred when fanotify calls pidfd_create().
---
Changes since v3:
* Updated the commit message to describe in when/why FAN_NOPIDFD can be returned.
* Modified the comment above the coditional where we attempt to preemptively check for PIDTYPE_TGID as FAN_NOPIDFD can be quite common for one shot programs, so it's not appropriate to describe it as being "rare".
fs/notify/fanotify/fanotify_user.c | 87 ++++++++++++++++++++++++++++-- include/linux/fanotify.h | 3 +- include/uapi/linux/fanotify.h | 13 +++++ 3 files changed, 97 insertions(+), 6 deletions(-)
fs/notify/fanotify/fanotify_user.c +82 -5
@@ -1,6 +1,7 @@// SPDX-License-Identifier: GPL-2.0 #include <linux/fanotify.h> #include <linux/fcntl.h> +#include <linux/fdtable.h> #include <linux/file.h> #include <linux/fs.h> #include <linux/anon_inodes.h>
@@ -106,6 +107,8 @@ struct kmem_cache *fanotify_perm_event_cachep __read_mostly;#define FANOTIFY_EVENT_ALIGN 4#define FANOTIFY_FID_INFO_HDR_LEN \ (sizeof(struct fanotify_event_info_fid) + sizeof(struct file_handle)) +#define FANOTIFY_PIDFD_INFO_HDR_LEN \ + sizeof(struct fanotify_event_info_pidfd)static int fanotify_fid_info_len (int fh_len,int name_len) {
@@ -138,6 +141,9 @@ static int fanotify_event_info_len(unsigned int info_mode, dot_len =1 ; } +if (info_mode &FAN_REPORT_PIDFD ) + info_len +=FANOTIFY_PIDFD_INFO_HDR_LEN ; +if (fh_len) info_len +=fanotify_fid_info_len (fh_len, dot_len);
@@ -401,13 +407,34 @@ static int copy_fid_info_to_user(__kernel_fsid_t *fsid, struct fanotify_fh *fh,return info_len; } +static int copy_pidfd_info_to_user (int pidfd, +char __user *buf, +size_t count) +{ +struct fanotify_event_info_pidfd info = { }; +size_t info_len =FANOTIFY_PIDFD_INFO_HDR_LEN ; + +if (WARN_ON_ONCE (info_len > count)) +return -EFAULT ; + + info.hdr.info_type =FAN_EVENT_INFO_TYPE_PIDFD ; + info.hdr.len = info_len; + info.pidfd = pidfd; + +if (copy_to_user (buf, &info, info_len)) +return -EFAULT ; + +return info_len; +} +static int copy_info_records_to_user (struct fanotify_event *event,struct fanotify_info *info, -unsigned int info_mode, +unsigned int info_mode,int pidfd,char __user *buf,size_t count) {int ret, total_bytes =0 , info_type =0 ;unsigned int fid_mode = info_mode &FANOTIFY_FID_BITS ; +unsigned int pidfd_mode = info_mode &FAN_REPORT_PIDFD ;/* * Event info records order is as follows: dir fid + name, child fid.
@@ -478,6 +505,16 @@ static int copy_info_records_to_user(struct fanotify_event *event, total_bytes += ret; } +if (pidfd_mode) { + ret =copy_pidfd_info_to_user (pidfd, buf, count); +if (ret <0 ) +return ret; + + buf += ret; + count -= ret; + total_bytes += ret; + } +return total_bytes; }
@@ -489,8 +526,9 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,struct path *path =fanotify_event_path (event);struct fanotify_info *info =fanotify_event_info (event);unsigned int info_mode =FAN_GROUP_FLAG (group,FANOTIFY_INFO_MODES ); +unsigned int pidfd_mode = info_mode &FAN_REPORT_PIDFD ;struct file *f =NULL ; -int ret, fd =FAN_NOFD ; +int ret, pidfd =FAN_NOPIDFD , fd =FAN_NOFD ;pr_debug ("%s: group=%p event=%p\n" , __func__, group, event);
@@ -524,6 +562,33 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group, } metadata.fd = fd; +if (pidfd_mode) { +/* +* Complain if the FAN_REPORT_PIDFD and FAN_REPORT_TID mutual +* exclusion is ever lifted. At the time of incoporating pidfd +* support within fanotify, the pidfd API only supported the +* creation of pidfds for thread-group leaders. +*/ +WARN_ON_ONCE (FAN_GROUP_FLAG (group,FAN_REPORT_TID )); + +/* +* The PIDTYPE_TGID check for an event->pid is performed +* preemptively in an attempt to catch out cases where the event +* listener reads events after the event generating process has +* already terminated. Report FAN_NOPIDFD to the event listener +* in those cases, with all other pidfd creation errors being +* reported as FAN_EPIDFD. +*/ +if (metadata.pid ==0 || + !pid_has_task (event->pid,PIDTYPE_TGID )) { + pidfd =FAN_NOPIDFD ; + }else { + pidfd =pidfd_create (event->pid,0 ); +if (pidfd <0 ) + pidfd =FAN_EPIDFD ; + } + } + ret = -EFAULT ;/* * Sanity check copy size in case get_one_event() and
@@ -545,8 +610,8 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,fd_install (fd, f);if (info_mode) { - ret =copy_info_records_to_user (event, info, info_mode, - buf, count); + ret =copy_info_records_to_user (event, info, info_mode, pidfd, + buf, count);if (ret <0 ) goto out_close_fd; }
@@ -558,6 +623,10 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,put_unused_fd (fd);fput (f); } + +if (pidfd >=0 ) +close_fd (pidfd); +return ret; }
@@ -1103,6 +1172,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)#endif return -EINVAL ; +/* +* A pidfd can only be returned for a thread-group leader; thus +* FAN_REPORT_PIDFD and FAN_REPORT_TID need to remain mutually +* exclusive. +*/ +if ((flags &FAN_REPORT_PIDFD ) && (flags &FAN_REPORT_TID )) +return -EINVAL ; +if (event_f_flags & ~FANOTIFY_INIT_ALL_EVENT_F_BITS )return -EINVAL ;
@@ -1504,7 +1581,7 @@ static int __init fanotify_user_setup(void) FANOTIFY_DEFAULT_MAX_USER_MARKS);BUILD_BUG_ON (FANOTIFY_INIT_FLAGS &FANOTIFY_INTERNAL_GROUP_FLAGS ); -BUILD_BUG_ON (HWEIGHT32 (FANOTIFY_INIT_FLAGS ) !=10 ); +BUILD_BUG_ON (HWEIGHT32 (FANOTIFY_INIT_FLAGS ) !=11 );BUILD_BUG_ON (HWEIGHT32 (FANOTIFY_MARK_FLAGS ) !=9 ); fanotify_mark_cache = KMEM_CACHE(fsnotify_mark,
include/linux/fanotify.h +2 -1
@@ -27,7 +27,7 @@ extern struct ctl_table fanotify_table[]; /* for sysctl */#define FANOTIFY_FID_BITS (FAN_REPORT_FID | FAN_REPORT_DFID_NAME) -#define FANOTIFY_INFO_MODES (FANOTIFY_FID_BITS) +#define FANOTIFY_INFO_MODES (FANOTIFY_FID_BITS | FAN_REPORT_PIDFD)/* * fanotify_init() flags that require CAP_SYS_ADMIN.
@@ -37,6 +37,7 @@ extern struct ctl_table fanotify_table[]; /* for sysctl */*/ #define FANOTIFY_ADMIN_INIT_FLAGS (FANOTIFY_PERM_CLASSES | \ FAN_REPORT_TID | \ + FAN_REPORT_PIDFD | \ FAN_UNLIMITED_QUEUE | \ FAN_UNLIMITED_MARKS)
include/uapi/linux/fanotify.h +13 -0
@@ -51,6 +51,7 @@#define FAN_ENABLE_AUDIT 0x00000040/* Flags to determine fanotify event format */ +#define FAN_REPORT_PIDFD 0x00000080/* Report pidfd for event->pid */ #define FAN_REPORT_TID 0x00000100/* event->pid is thread id */ #define FAN_REPORT_FID 0x00000200/* Report unique file id */ #define FAN_REPORT_DIR_FID 0x00000400/* Report unique directory id */
@@ -123,6 +124,7 @@ struct fanotify_event_metadata {#define FAN_EVENT_INFO_TYPE_FID 1#define FAN_EVENT_INFO_TYPE_DFID_NAME 2#define FAN_EVENT_INFO_TYPE_DFID 3 +#define FAN_EVENT_INFO_TYPE_PIDFD 4/* Variable length info record following event metadata */ struct fanotify_event_info_header {
@@ -148,6 +150,15 @@ struct fanotify_event_info_fid {unsigned char handle[0 ]; }; +/* +* This structure is used for info records of type FAN_EVENT_INFO_TYPE_PIDFD. +* It holds a pidfd for the pid that was responsible for generating an event. +*/ +struct fanotify_event_info_pidfd { +struct fanotify_event_info_header hdr; +__s32 pidfd; +}; +struct fanotify_response {__s32 fd;__u32 response;
@@ -160,6 +171,8 @@ struct fanotify_response {/* No fd set in event */ #define FAN_NOFD -1 +#define FAN_NOPIDFD FAN_NOFD +#define FAN_EPIDFD -2/* Helper functions to deal with fanotify_event_metadata buffers */ #define FAN_EVENT_METADATA_LEN (sizeof(struct fanotify_event_metadata))
Signature · 3 lines
2.32.0.605.g8dce9f2422-goog /M
Jan Kara
maintainerHello Matthew!
Quoted 12 lines · Matthew Bobrowski
This is V5 of the
FAN_REPORT_PIDFDpatch series. It contains the minor comment/commit description fixes that were picked up by Amir in the last series review [0, 1].LTP tests for this API change can be found here [2]. Man page updates for this change can be found here [3].
[0] https://lore.kernel.org/linux-fsdevel/CAOQ4uxhnCk+FXK_e_GA=jC_0HWO+3ZdwHSi=zCa2Kpb0NDxBSg@mail.gmail.com/
[1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxgO3oViTSFZ0zs6brrHrmw362r1C9SQ7g6=XgRwyrzMuw@mail.gmail.com/
[2] https://github.com/matthewbobrowski/ltp/tree/fanotify_pidfd_v2
[3] https://github.com/matthewbobrowski/man-pages/tree/fanotify_pidfd_v1
Quoted 7 lines
Matthew Bobrowski (5):
kernel/pid.c: remove static qualifier frompidfd_create()kernel/pid.c: implement additional checks uponpidfd_create()
parameters
fanotify: minor cosmetic adjustments to fid labels
fanotify: introduce a generic info record copying helper
fanotify: add pidfd support to the fanotify API
Thanks! I've pulled the series into my tree. Note that your fanotify21 LTP testcase is broken with the current kernel because 'ino' entry got added to fdinfo. I think having to understand all possible keys that can occur in fdinfo is too fragile. I understand why you want to do that but I guess the test would be too faulty to be practical. So I'd just ignore unknown keys in fdinfo for that test.
Honza
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Quoted 23 lines · Jan Kara
Hello Matthew!
Quoted 12 lines · Matthew Bobrowski
This is V5 of the
FAN_REPORT_PIDFDpatch series. It contains the minor comment/commit description fixes that were picked up by Amir in the last series review [0, 1].LTP tests for this API change can be found here [2]. Man page updates for this change can be found here [3].
[0] https://lore.kernel.org/linux-fsdevel/CAOQ4uxhnCk+FXK_e_GA=jC_0HWO+3ZdwHSi=zCa2Kpb0NDxBSg@mail.gmail.com/
[1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxgO3oViTSFZ0zs6brrHrmw362r1C9SQ7g6=XgRwyrzMuw@mail.gmail.com/
[2] https://github.com/matthewbobrowski/ltp/tree/fanotify_pidfd_v2
[3] https://github.com/matthewbobrowski/man-pages/tree/fanotify_pidfd_v1Quoted 7 lines
Matthew Bobrowski (5):
kernel/pid.c: remove static qualifier frompidfd_create()kernel/pid.c: implement additional checks uponpidfd_create()
parameters
fanotify: minor cosmetic adjustments to fid labels
fanotify: introduce a generic info record copying helper
fanotify: add pidfd support to the fanotify API
Quoted 6 lines
Thanks! I've pulled the series into my tree. Note that your fanotify21 LTP testcase is broken with the current kernel because 'ino' entry got added to fdinfo. I think having to understand all possible keys that can occur in fdinfo is too fragile. I understand why you want to do that but I guess the test would be too faulty to be practical. So I'd just ignore unknown keys in fdinfo for that test.
Excellent, for merging these changes!
In regards to the LTP test (fanotify21), at the time of writing I had also shared a similar thought in the sense that it was too fragile, but wrongly so I weighed up my decision based on the likelihood and frequency of fields being changed/added to fdinfo. I was very wrong...
Anyway, I will fix it so that any "unknown" fields are ignored.
/M
Hey Jan,
Quoted 31 lines · Matthew Bobrowski
Quoted 23 lines · Jan Kara
Hello Matthew!
Quoted 12 lines · Matthew Bobrowski
This is V5 of the
FAN_REPORT_PIDFDpatch series. It contains the minor comment/commit description fixes that were picked up by Amir in the last series review [0, 1].LTP tests for this API change can be found here [2]. Man page updates for this change can be found here [3].
[0] https://lore.kernel.org/linux-fsdevel/CAOQ4uxhnCk+FXK_e_GA=jC_0HWO+3ZdwHSi=zCa2Kpb0NDxBSg@mail.gmail.com/
[1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxgO3oViTSFZ0zs6brrHrmw362r1C9SQ7g6=XgRwyrzMuw@mail.gmail.com/
[2] https://github.com/matthewbobrowski/ltp/tree/fanotify_pidfd_v2
[3] https://github.com/matthewbobrowski/man-pages/tree/fanotify_pidfd_v1Quoted 7 lines
Matthew Bobrowski (5):
kernel/pid.c: remove static qualifier frompidfd_create()kernel/pid.c: implement additional checks uponpidfd_create()
parameters
fanotify: minor cosmetic adjustments to fid labels
fanotify: introduce a generic info record copying helper
fanotify: add pidfd support to the fanotify APIQuoted 6 lines
Thanks! I've pulled the series into my tree. Note that your fanotify21 LTP testcase is broken with the current kernel because 'ino' entry got added to fdinfo. I think having to understand all possible keys that can occur in fdinfo is too fragile. I understand why you want to do that but I guess the test would be too faulty to be practical. So I'd just ignore unknown keys in fdinfo for that test.
Quoted 8 lines
Excellent, for merging these changes!
In regards to the LTP test (fanotify21), at the time of writing I had also shared a similar thought in the sense that it was too fragile, but wrongly so I weighed up my decision based on the likelihood and frequency of fields being changed/added to fdinfo. I was very wrong...
Anyway, I will fix it so that any "unknown" fields are ignored.
FWIW, I've dropped that last else statement in the parse_pidfd_fdinfo_line() helper in LTP fanotify21. An updated branch has been pushed here [0].
[0] https://github.com/matthewbobrowski/ltp/commits/fanotify_pidfd_v3
/M