[PATCH v3 0/2] fanotify: support restartable permission events

[PATCH v3 0/2] fanotify: support restartable permission events

[Ibrahim Jirdeh]

These patches are in order to add support for restarting permission
events which is useful for HSM use cases which are backed by a daemon
to respond reliably [1].

In terms of testing, there is an additional LTP test attached which
exercises releasing queue via the new api [2]

[1] https://lore.kernel.org/linux-fsdevel/6za2mngeqslmqjg3icoubz37hbbxi6bi44canfsg2aajgkialt@c3ujlrjzkppr/
[2] https://github.com/ibrahim-jirdeh/ltp/commit/efa7aa88c866f3ba4d6a5c6c436ac27837bcdcc8

Ibrahim Jirdeh (2):
  fanotify: add restart infrastructure for pending permission events
  fanotify: introduce restartable permission events

 fs/notify/fanotify/fanotify.c       |   1 +
 fs/notify/fanotify/fanotify.h       |   4 +
 fs/notify/fanotify/fanotify_user.c  | 153 ++++++++++++++++++++++++++--
 include/linux/fanotify.h            |   1 +
 include/linux/fsnotify_backend.h    |   3 +
 include/uapi/linux/fanotify.h       |   7 ++
 tools/include/uapi/linux/fanotify.h |   7 ++
 7 files changed, 170 insertions(+), 6 deletions(-)

--
2.52.0

v2: https://lore.kernel.org/linux-fsdevel/20260414185037.3541089-1-ibrahimjirdeh@meta.com/
v1: https://lore.kernel.org/linux-fsdevel/20250806220516.953114-1-ibrahimjirdeh@meta.com/

Changes v2 => v3:
- Removed fsnotify_restart_event helper, instead inlined reinsert logic
  into fanotify_restart_pending_events
- guard(mutex) instead of goto in fanotify_open_queue_fd
- Fixed permission event mask check to allow FANOTIFY_EVENT_FLAGS bits
- FAN_RESTARTABLE_EVENTS nibble changed from 0x00008000 to 0x00010000
- Have queue fd keep a group reference via fsnotify_get_group

Changes v1 => v2:
- Replace fsnotify_insert_event usage with dedicated
  fsnotify_restart_event that inserts at head of queue without
  merge or overflow handling
- Return -EBUSY instead of -EEXIST for duplicate queue open
- Separate fanotify_control_ioctl for control fd with only
  FAN_IOC_OPEN_QUEUE_FD, move FIONREAD to queue fops
- Move queue_mutex/queue_opened to fanotify_data
- return EOF on read from queue fd after group shutdown

[PATCH v3 1/2] fanotify: add restart infrastructure for pending permission events

[Ibrahim Jirdeh]

Add fanotify_restart_pending_events() helper that walks the
access_list and reinserts each pending permission event back into
the notification queue for reprocessing. Events are reinserted at
the head of the notification queue without merge or overflow handling.

Suggested-by: Jan Kara <jack@suse.cz>
Link: https://lore.kernel.org/linux-fsdevel/sx5g7pmkchjqucfbzi77xh7wx4wua5nteqi5bsa2hfqgxua2a2@v7x6ja3gsirn/
Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
---
 fs/notify/fanotify/fanotify_user.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index d0b9b984002f..787e57a00466 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1095,6 +1095,36 @@ static ssize_t fanotify_write(struct file *file, const char __user *buf, size_t
 	return count;
 }
 
+/*
+ * Reinsert pending permission events at the head of the notification
+ * queue for reprocessing. No merge or overflow handling.
+ */
+static void fanotify_restart_pending_events(struct fsnotify_group *group)
+{
+	spin_lock(&group->notification_lock);
+	while (!list_empty(&group->fanotify_data.access_list)) {
+		struct fanotify_perm_event *event;
+
+		event = list_first_entry(&group->fanotify_data.access_list,
+					 struct fanotify_perm_event,
+					 fae.fse.list);
+		list_del_init(&event->fae.fse.list);
+
+		if (group->shutdown) {
+			/* Group is being shut down. Reply ALLOW for the event. */
+			finish_permission_event(group, event, FAN_ALLOW, NULL);
+			spin_lock(&group->notification_lock);
+		} else {
+			group->q_len++;
+			list_add(&event->fae.fse.list,
+				 &group->notification_list);
+		}
+	}
+	spin_unlock(&group->notification_lock);
+
+	wake_up(&group->notification_waitq);
+}
+
 static int fanotify_release(struct inode *ignored, struct file *file)
 {
 	struct fsnotify_group *group = file->private_data;
-- 
2.52.0

[PATCH v3 2/2] fanotify: introduce restartable permission events

[Ibrahim Jirdeh]

This adds support for restarting permission events. The main goal of
the change is to provide better handling for pending events for lazy
file loading use cases which may back fanotify events by a long-lived
daemon. For prior discussion of approaches see [1][2].

In terms of implementation, we add a new control-fd/queue-fd api.
Control fd returned by fanotify_init keeps fanotify group alive and
supports operations like fanotify_mark as well as a new ioctl
FAN_IOC_OPEN_QUEUE_FD to open a queue fd. Queue fd is used
for reading events and writing back responses. Upon release of
queue fd, pending permission events are reinserted back into
notification queue for reprocessing.

Control-fd/queue-fd api is guarded by FAN_RESTARTABLE_EVENTS flag.
In addition FAN_RESTARTABLE_EVENTS can only be used in conjunction
with FAN_CLASS_CONTENT or FAN_CLASS_PRE_CONTENT, and only permission
events can be added to the mark mask if a group is initialized with
FAN_RESTARTABLE_EVENTS.

[1] https://lore.kernel.org/linux-fsdevel/6za2mngeqslmqjg3icoubz37hbbxi6bi44canfsg2aajgkialt@c3ujlrjzkppr
[2] https://lore.kernel.org/linux-fsdevel/20250623192503.2673076-1-ibrahimjirdeh@meta.com

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhN6ok6BCBGbxeUt9ULq6g=qL6=_2_QGi8MqTHv5ZN7Vg@mail.gmail.com
Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
---
 fs/notify/fanotify/fanotify.c       |   1 +
 fs/notify/fanotify/fanotify.h       |   4 +
 fs/notify/fanotify/fanotify_user.c  | 123 ++++++++++++++++++++++++++--
 include/linux/fanotify.h            |   1 +
 include/linux/fsnotify_backend.h    |   3 +
 include/uapi/linux/fanotify.h       |   7 ++
 tools/include/uapi/linux/fanotify.h |   7 ++
 7 files changed, 140 insertions(+), 6 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index bfe884d624e7..b22474e095cd 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -1015,6 +1015,7 @@ static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
 
 static void fanotify_free_group_priv(struct fsnotify_group *group)
 {
+	mutex_destroy(&group->fanotify_data.queue_mutex);
 	put_user_ns(group->user_ns);
 	kfree(group->fanotify_data.merge_hash);
 	if (group->fanotify_data.ucounts)
diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
index 39e60218df7c..dd64af8313a9 100644
--- a/fs/notify/fanotify/fanotify.h
+++ b/fs/notify/fanotify/fanotify.h
@@ -552,3 +552,7 @@ static inline u32 fanotify_get_response_errno(int res)
 {
 	return (res >> FAN_ERRNO_SHIFT) & FAN_ERRNO_MASK;
 }
+
+extern const struct file_operations fanotify_fops;
+extern const struct file_operations fanotify_control_fops;
+extern const struct file_operations fanotify_queue_fops;
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 787e57a00466..b4610c49daab 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1018,6 +1018,15 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
 			if (file->f_flags & O_NONBLOCK)
 				break;
 
+			/*
+			 * With FAN_RESTARTABLE_EVENTS, queue fd may
+			 * outlive control fd. Return EOF if group
+			 * is shut down.
+			 */
+			ret = 0;
+			if (group->shutdown)
+				break;
+
 			ret = -ERESTARTSYS;
 			if (signal_pending(current))
 				break;
@@ -1101,6 +1110,7 @@ static ssize_t fanotify_write(struct file *file, const char __user *buf, size_t
  */
 static void fanotify_restart_pending_events(struct fsnotify_group *group)
 {
+	mutex_lock(&group->fanotify_data.queue_mutex);
 	spin_lock(&group->notification_lock);
 	while (!list_empty(&group->fanotify_data.access_list)) {
 		struct fanotify_perm_event *event;
@@ -1121,10 +1131,21 @@ static void fanotify_restart_pending_events(struct fsnotify_group *group)
 		}
 	}
 	spin_unlock(&group->notification_lock);
+	group->fanotify_data.queue_opened = false;
+	mutex_unlock(&group->fanotify_data.queue_mutex);
 
 	wake_up(&group->notification_waitq);
 }
 
+static int fanotify_queue_release(struct inode *ignored, struct file *file)
+{
+	struct fsnotify_group *group = file->private_data;
+
+	fanotify_restart_pending_events(group);
+	fsnotify_put_group(group);
+	return 0;
+}
+
 static int fanotify_release(struct inode *ignored, struct file *file)
 {
 	struct fsnotify_group *group = file->private_data;
@@ -1182,6 +1203,44 @@ static int fanotify_release(struct inode *ignored, struct file *file)
 	return 0;
 }
 
+static int fanotify_open_queue_fd(struct file *file)
+{
+	struct fsnotify_group *group = file->private_data;
+	int f_flags, fd;
+	struct file *queue_file;
+
+	if (!FAN_GROUP_FLAG(group, FAN_RESTARTABLE_EVENTS))
+		return -EINVAL;
+
+	guard(mutex)(&group->fanotify_data.queue_mutex);
+
+	if (group->fanotify_data.queue_opened)
+		return -EBUSY;
+
+	f_flags = O_RDWR;
+	if (group->fanotify_data.flags & FAN_CLOEXEC)
+		f_flags |= O_CLOEXEC;
+	if (group->fanotify_data.flags & FAN_NONBLOCK)
+		f_flags |= O_NONBLOCK;
+
+	fd = get_unused_fd_flags(f_flags);
+	if (fd < 0)
+		return fd;
+
+	queue_file = anon_inode_getfile_fmode("[fanotify-queue]",
+					      &fanotify_queue_fops, group,
+					      f_flags, FMODE_NONOTIFY);
+	if (IS_ERR(queue_file)) {
+		put_unused_fd(fd);
+		return PTR_ERR(queue_file);
+	}
+	fsnotify_get_group(group);
+	fd_install(fd, queue_file);
+	group->fanotify_data.queue_opened = true;
+
+	return fd;
+}
+
 static long fanotify_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	struct fsnotify_group *group;
@@ -1207,7 +1266,21 @@ static long fanotify_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 	return ret;
 }
 
-static const struct file_operations fanotify_fops = {
+static long fanotify_control_ioctl(struct file *file, unsigned int cmd,
+				   unsigned long arg)
+{
+	int ret = -ENOTTY;
+
+	switch (cmd) {
+	case FAN_IOC_OPEN_QUEUE_FD:
+		ret = fanotify_open_queue_fd(file);
+		break;
+	}
+
+	return ret;
+}
+
+const struct file_operations fanotify_fops = {
 	.show_fdinfo	= fanotify_show_fdinfo,
 	.poll		= fanotify_poll,
 	.read		= fanotify_read,
@@ -1219,6 +1292,23 @@ static const struct file_operations fanotify_fops = {
 	.llseek		= noop_llseek,
 };
 
+const struct file_operations fanotify_control_fops = {
+	.show_fdinfo	= fanotify_show_fdinfo,
+	.release	= fanotify_release,
+	.unlocked_ioctl	= fanotify_control_ioctl,
+	.llseek		= noop_llseek,
+};
+
+const struct file_operations fanotify_queue_fops = {
+	.poll		= fanotify_poll,
+	.read		= fanotify_read,
+	.write		= fanotify_write,
+	.release	= fanotify_queue_release,
+	.unlocked_ioctl	= fanotify_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
+	.llseek		= noop_llseek,
+};
+
 static int fanotify_find_path(int dfd, const char __user *filename,
 			      struct path *path, unsigned int flags, __u64 mask,
 			      unsigned int obj_type)
@@ -1640,6 +1730,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	int f_flags, fd;
 	unsigned int fid_mode = flags & FANOTIFY_FID_BITS;
 	unsigned int class = flags & FANOTIFY_CLASS_BITS;
+	unsigned int restartable_events = flags & FAN_RESTARTABLE_EVENTS;
 	unsigned int internal_flags = 0;
 
 	pr_debug("%s: flags=%x event_f_flags=%x\n",
@@ -1718,10 +1809,17 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	    (!(fid_mode & FAN_REPORT_NAME) || !(fid_mode & FAN_REPORT_FID)))
 		return -EINVAL;
 
-	f_flags = O_RDWR;
+	/*
+	 * FAN_RESTARTABLE_EVENTS requires FAN_CLASS_CONTENT or
+	 * FAN_CLASS_PRE_CONTENT
+	 */
+	if (restartable_events && class == FAN_CLASS_NOTIF)
+		return -EINVAL;
+
+	f_flags = restartable_events ? O_RDONLY : O_RDWR;
 	if (flags & FAN_CLOEXEC)
 		f_flags |= O_CLOEXEC;
-	if (flags & FAN_NONBLOCK)
+	if (!restartable_events && (flags & FAN_NONBLOCK))
 		f_flags |= O_NONBLOCK;
 
 	CLASS(fsnotify_group, group)(&fanotify_fsnotify_ops,
@@ -1754,6 +1852,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	init_waitqueue_head(&group->fanotify_data.access_waitq);
 	INIT_LIST_HEAD(&group->fanotify_data.access_list);
 	INIT_LIST_HEAD(&group->fanotify_data.perm_grp_list);
+	mutex_init(&group->fanotify_data.queue_mutex);
 	switch (class) {
 	case FAN_CLASS_NOTIF:
 		group->priority = FSNOTIFY_PRIO_NORMAL;
@@ -1781,7 +1880,9 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	}
 
 	fd = FD_ADD(f_flags,
-		    anon_inode_getfile_fmode("[fanotify]", &fanotify_fops,
+		    anon_inode_getfile_fmode("[fanotify]",
+					     (restartable_events ? &fanotify_control_fops :
+					     &fanotify_fops),
 					     group, f_flags, FMODE_NONOTIFY));
 	if (fd >= 0)
 		retain_and_null_ptr(group);
@@ -1999,7 +2100,8 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 		return -EBADF;
 
 	/* verify that this is indeed an fanotify instance */
-	if (unlikely(fd_file(f)->f_op != &fanotify_fops))
+	if (unlikely(fd_file(f)->f_op != &fanotify_fops &&
+		fd_file(f)->f_op != &fanotify_control_fops))
 		return -EINVAL;
 	group = fd_file(f)->private_data;
 
@@ -2035,6 +2137,15 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 		 group->priority == FSNOTIFY_PRIO_CONTENT)
 		return -EINVAL;
 
+	/*
+	 * With FAN_RESTARTABLE_EVENTS, a user is only allowed to setup
+	 * permission events and modify event flags.
+	 */
+	if (FAN_GROUP_FLAG(group, FAN_RESTARTABLE_EVENTS) &&
+	    (mask & ~(FANOTIFY_FD_EVENTS | FANOTIFY_MOUNT_EVENTS |
+		      FANOTIFY_EVENT_FLAGS)))
+		return -EINVAL;
+
 	if (mask & FAN_FS_ERROR &&
 	    mark_type != FAN_MARK_FILESYSTEM)
 		return -EINVAL;
@@ -2224,7 +2335,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) != 14);
+	BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 15);
 	BUILD_BUG_ON(HWEIGHT32(FANOTIFY_MARK_FLAGS) != 11);
 
 	fanotify_mark_cache = KMEM_CACHE(fanotify_mark,
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index 879cff5eccd4..38854a1d6485 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -37,6 +37,7 @@
 					 FAN_REPORT_TID | \
 					 FAN_REPORT_PIDFD | \
 					 FAN_REPORT_FD_ERROR | \
+					 FAN_RESTARTABLE_EVENTS | \
 					 FAN_UNLIMITED_QUEUE | \
 					 FAN_UNLIMITED_MARKS)
 
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 0d954ea7b179..f1167d728a47 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -275,6 +275,9 @@ struct fsnotify_group {
 			mempool_t error_events_pool;
 			/* chained on perm_group_list */
 			struct list_head perm_grp_list;
+			/* protects queue fd open/release */
+			struct mutex queue_mutex;
+			bool queue_opened;
 		} fanotify_data;
 #endif /* CONFIG_FANOTIFY */
 	};
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index e710967c7c26..a838582e113d 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -68,6 +68,8 @@
 #define FAN_REPORT_FD_ERROR	0x00002000	/* event->fd can report error */
 #define FAN_REPORT_MNT		0x00004000	/* Report mount events */
 
+#define FAN_RESTARTABLE_EVENTS	0x00010000	/* enable control-fd/queue-fd api */
+
 /* Convenience macro - FAN_REPORT_NAME requires FAN_REPORT_DIR_FID */
 #define FAN_REPORT_DFID_NAME	(FAN_REPORT_DIR_FID | FAN_REPORT_NAME)
 /* Convenience macro - FAN_REPORT_TARGET_FID requires all other FID flags */
@@ -271,4 +273,9 @@ struct fanotify_response_info_audit_rule {
 				(long)(meta)->event_len >= (long)FAN_EVENT_METADATA_LEN && \
 				(long)(meta)->event_len <= (long)(len))
 
+/* fanotify ioctls */
+
+/* Open a queue fd used in control-fd api to read and respond to events */
+#define FAN_IOC_OPEN_QUEUE_FD	_IO('F', 0xF0)
+
 #endif /* _UAPI_LINUX_FANOTIFY_H */
diff --git a/tools/include/uapi/linux/fanotify.h b/tools/include/uapi/linux/fanotify.h
index e710967c7c26..a838582e113d 100644
--- a/tools/include/uapi/linux/fanotify.h
+++ b/tools/include/uapi/linux/fanotify.h
@@ -68,6 +68,8 @@
 #define FAN_REPORT_FD_ERROR	0x00002000	/* event->fd can report error */
 #define FAN_REPORT_MNT		0x00004000	/* Report mount events */
 
+#define FAN_RESTARTABLE_EVENTS	0x00010000	/* enable control-fd/queue-fd api */
+
 /* Convenience macro - FAN_REPORT_NAME requires FAN_REPORT_DIR_FID */
 #define FAN_REPORT_DFID_NAME	(FAN_REPORT_DIR_FID | FAN_REPORT_NAME)
 /* Convenience macro - FAN_REPORT_TARGET_FID requires all other FID flags */
@@ -271,4 +273,9 @@ struct fanotify_response_info_audit_rule {
 				(long)(meta)->event_len >= (long)FAN_EVENT_METADATA_LEN && \
 				(long)(meta)->event_len <= (long)(len))
 
+/* fanotify ioctls */
+
+/* Open a queue fd used in control-fd api to read and respond to events */
+#define FAN_IOC_OPEN_QUEUE_FD	_IO('F', 0xF0)
+
 #endif /* _UAPI_LINUX_FANOTIFY_H */
-- 
2.52.0

Re: [PATCH v3 2/2] fanotify: introduce restartable permission events

[Amir Goldstein]

On Thu, Apr 16, 2026 at 9:49 PM Ibrahim Jirdeh <ibrahimjirdeh@meta.com> wrote:
>
> This adds support for restarting permission events. The main goal of
> the change is to provide better handling for pending events for lazy
> file loading use cases which may back fanotify events by a long-lived
> daemon. For prior discussion of approaches see [1][2].
>
> In terms of implementation, we add a new control-fd/queue-fd api.
> Control fd returned by fanotify_init keeps fanotify group alive and
> supports operations like fanotify_mark as well as a new ioctl
> FAN_IOC_OPEN_QUEUE_FD to open a queue fd. Queue fd is used
> for reading events and writing back responses. Upon release of
> queue fd, pending permission events are reinserted back into
> notification queue for reprocessing.
>
> Control-fd/queue-fd api is guarded by FAN_RESTARTABLE_EVENTS flag.
> In addition FAN_RESTARTABLE_EVENTS can only be used in conjunction
> with FAN_CLASS_CONTENT or FAN_CLASS_PRE_CONTENT, and only permission
> events can be added to the mark mask if a group is initialized with
> FAN_RESTARTABLE_EVENTS.
>
> [1] https://lore.kernel.org/linux-fsdevel/6za2mngeqslmqjg3icoubz37hbbxi6bi44canfsg2aajgkialt@c3ujlrjzkppr
> [2] https://lore.kernel.org/linux-fsdevel/20250623192503.2673076-1-ibrahimjirdeh@meta.com
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhN6ok6BCBGbxeUt9ULq6g=qL6=_2_QGi8MqTHv5ZN7Vg@mail.gmail.com
> Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
> ---
...
> @@ -2035,6 +2137,15 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>                  group->priority == FSNOTIFY_PRIO_CONTENT)
>                 return -EINVAL;
>
> +       /*
> +        * With FAN_RESTARTABLE_EVENTS, a user is only allowed to setup
> +        * permission events and modify event flags.
> +        */
> +       if (FAN_GROUP_FLAG(group, FAN_RESTARTABLE_EVENTS) &&
> +           (mask & ~(FANOTIFY_FD_EVENTS | FANOTIFY_MOUNT_EVENTS |
> +                     FANOTIFY_EVENT_FLAGS)))
> +               return -EINVAL;
> +

Sorry, my review comment was misunderstood.
I obviously meant "like" this line, not exactly the same.
This code does not do what the comment says at all.
LLM flop or human flop? ;)

       if (FAN_GROUP_FLAG(group, FAN_RESTARTABLE_EVENTS) &&
           (mask & ~(FANOTIFY_PERM_EVENTS | FANOTIFY_EVENT_FLAGS)))
               return -EINVAL;

Thanks,
Amir.

Re: [PATCH v3 1/2] fanotify: add restart infrastructure for pending permission events

[Amir Goldstein]

On Thu, Apr 16, 2026 at 9:49 PM Ibrahim Jirdeh <ibrahimjirdeh@meta.com> wrote:
>
> Add fanotify_restart_pending_events() helper that walks the
> access_list and reinserts each pending permission event back into
> the notification queue for reprocessing. Events are reinserted at
> the head of the notification queue without merge or overflow handling.
>
> Suggested-by: Jan Kara <jack@suse.cz>
> Link: https://lore.kernel.org/linux-fsdevel/sx5g7pmkchjqucfbzi77xh7wx4wua5nteqi5bsa2hfqgxua2a2@v7x6ja3gsirn/
> Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
> ---
>  fs/notify/fanotify/fanotify_user.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index d0b9b984002f..787e57a00466 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1095,6 +1095,36 @@ static ssize_t fanotify_write(struct file *file, const char __user *buf, size_t
>         return count;
>  }
>
> +/*
> + * Reinsert pending permission events at the head of the notification
> + * queue for reprocessing. No merge or overflow handling.
> + */
> +static void fanotify_restart_pending_events(struct fsnotify_group *group)
> +{
> +       spin_lock(&group->notification_lock);
> +       while (!list_empty(&group->fanotify_data.access_list)) {
> +               struct fanotify_perm_event *event;
> +
> +               event = list_first_entry(&group->fanotify_data.access_list,
> +                                        struct fanotify_perm_event,
> +                                        fae.fse.list);
> +               list_del_init(&event->fae.fse.list);
> +
> +               if (group->shutdown) {
> +                       /* Group is being shut down. Reply ALLOW for the event. */
> +                       finish_permission_event(group, event, FAN_ALLOW, NULL);
> +                       spin_lock(&group->notification_lock);

This looks very awkward.

We are holding the notification lock for the non-shutdown case
throughout moving the events back into the notification_list
(could possibly do that with list_move() if we keep a count
of in-progress events).

Seems more reasonable to also finish all events in a single batch
if the group is shutdown and then maybe use the same helper in
fanotify_release().

But Ibrahim, please wait for Jan's input on this before sending v4.

There are many ways to do this, and reusing the existing
finish_permission_event() locking semantics in this loop may be
fine for simplicity.

It is also a matter of taste, so please wait for Jan to chime in.

Thanks,
Amir.

Re: [PATCH v3 1/2] fanotify: add restart infrastructure for pending permission events

[Jan Kara]

On Fri 17-04-26 13:09:59, Amir Goldstein wrote:
> On Thu, Apr 16, 2026 at 9:49 PM Ibrahim Jirdeh <ibrahimjirdeh@meta.com> wrote:
> >
> > Add fanotify_restart_pending_events() helper that walks the
> > access_list and reinserts each pending permission event back into
> > the notification queue for reprocessing. Events are reinserted at
> > the head of the notification queue without merge or overflow handling.
> >
> > Suggested-by: Jan Kara <jack@suse.cz>
> > Link: https://lore.kernel.org/linux-fsdevel/sx5g7pmkchjqucfbzi77xh7wx4wua5nteqi5bsa2hfqgxua2a2@v7x6ja3gsirn/
> > Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
> > ---
> >  fs/notify/fanotify/fanotify_user.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> > index d0b9b984002f..787e57a00466 100644
> > --- a/fs/notify/fanotify/fanotify_user.c
> > +++ b/fs/notify/fanotify/fanotify_user.c
> > @@ -1095,6 +1095,36 @@ static ssize_t fanotify_write(struct file *file, const char __user *buf, size_t
> >         return count;
> >  }
> >
> > +/*
> > + * Reinsert pending permission events at the head of the notification
> > + * queue for reprocessing. No merge or overflow handling.
> > + */
> > +static void fanotify_restart_pending_events(struct fsnotify_group *group)
> > +{
> > +       spin_lock(&group->notification_lock);
> > +       while (!list_empty(&group->fanotify_data.access_list)) {
> > +               struct fanotify_perm_event *event;
> > +
> > +               event = list_first_entry(&group->fanotify_data.access_list,
> > +                                        struct fanotify_perm_event,
> > +                                        fae.fse.list);
> > +               list_del_init(&event->fae.fse.list);
> > +
> > +               if (group->shutdown) {
> > +                       /* Group is being shut down. Reply ALLOW for the event. */
> > +                       finish_permission_event(group, event, FAN_ALLOW, NULL);
> > +                       spin_lock(&group->notification_lock);
> 
> This looks very awkward.
> 
> We are holding the notification lock for the non-shutdown case
> throughout moving the events back into the notification_list
> (could possibly do that with list_move() if we keep a count
> of in-progress events).
> 
> Seems more reasonable to also finish all events in a single batch
> if the group is shutdown and then maybe use the same helper in
> fanotify_release().
> 
> But Ibrahim, please wait for Jan's input on this before sending v4.
> 
> There are many ways to do this, and reusing the existing
> finish_permission_event() locking semantics in this loop may be
> fine for simplicity.
> 
> It is also a matter of taste, so please wait for Jan to chime in.

So I wouldn't overcomplicate and just go for something like:

	spin_lock(&group->notification_lock);
	/* Group is exiting, fanotify_release() will remove pending events */
	if (group->shutdown) {
		spin_unlock(&group->notification_lock);
		return;
	}
	group->q_len += list_count_nodes(&group->fanotify_data.access_list);
	list_splice_init(&group->fanotify_data.access_list,
			 &group->notification_list);
	spin_unlock(&group->notification_lock);

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v3 2/2] fanotify: introduce restartable permission events

[Jan Kara]

On Thu 16-04-26 12:48:44, Ibrahim Jirdeh wrote:
> This adds support for restarting permission events. The main goal of
> the change is to provide better handling for pending events for lazy
> file loading use cases which may back fanotify events by a long-lived
> daemon. For prior discussion of approaches see [1][2].
> 
> In terms of implementation, we add a new control-fd/queue-fd api.
> Control fd returned by fanotify_init keeps fanotify group alive and
> supports operations like fanotify_mark as well as a new ioctl
> FAN_IOC_OPEN_QUEUE_FD to open a queue fd. Queue fd is used
> for reading events and writing back responses. Upon release of
> queue fd, pending permission events are reinserted back into
> notification queue for reprocessing.
> 
> Control-fd/queue-fd api is guarded by FAN_RESTARTABLE_EVENTS flag.
> In addition FAN_RESTARTABLE_EVENTS can only be used in conjunction
> with FAN_CLASS_CONTENT or FAN_CLASS_PRE_CONTENT, and only permission
> events can be added to the mark mask if a group is initialized with
> FAN_RESTARTABLE_EVENTS.
> 
> [1] https://lore.kernel.org/linux-fsdevel/6za2mngeqslmqjg3icoubz37hbbxi6bi44canfsg2aajgkialt@c3ujlrjzkppr
> [2] https://lore.kernel.org/linux-fsdevel/20250623192503.2673076-1-ibrahimjirdeh@meta.com
> 
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhN6ok6BCBGbxeUt9ULq6g=qL6=_2_QGi8MqTHv5ZN7Vg@mail.gmail.com
> Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>

Thanks for the patch. Some comments below...

> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index 39e60218df7c..dd64af8313a9 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -552,3 +552,7 @@ static inline u32 fanotify_get_response_errno(int res)
>  {
>  	return (res >> FAN_ERRNO_SHIFT) & FAN_ERRNO_MASK;
>  }
> +
> +extern const struct file_operations fanotify_fops;
> +extern const struct file_operations fanotify_control_fops;
> +extern const struct file_operations fanotify_queue_fops;

Why cannot the ops remain static? I didn't find anything in this patch that
would require them to be extern...

> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 787e57a00466..b4610c49daab 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1018,6 +1018,15 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
>  			if (file->f_flags & O_NONBLOCK)
>  				break;
>  
> +			/*
> +			 * With FAN_RESTARTABLE_EVENTS, queue fd may
> +			 * outlive control fd. Return EOF if group
> +			 * is shut down.
> +			 */
> +			ret = 0;
> +			if (group->shutdown)
> +				break;
> +
>  			ret = -ERESTARTSYS;
>  			if (signal_pending(current))
>  				break;
> @@ -1101,6 +1110,7 @@ static ssize_t fanotify_write(struct file *file, const char __user *buf, size_t
>   */
>  static void fanotify_restart_pending_events(struct fsnotify_group *group)
>  {
> +	mutex_lock(&group->fanotify_data.queue_mutex);
>  	spin_lock(&group->notification_lock);
>  	while (!list_empty(&group->fanotify_data.access_list)) {
>  		struct fanotify_perm_event *event;
> @@ -1121,10 +1131,21 @@ static void fanotify_restart_pending_events(struct fsnotify_group *group)
>  		}
>  	}
>  	spin_unlock(&group->notification_lock);
> +	group->fanotify_data.queue_opened = false;
> +	mutex_unlock(&group->fanotify_data.queue_mutex);
>  
>  	wake_up(&group->notification_waitq);
>  }
>  
> +static int fanotify_queue_release(struct inode *ignored, struct file *file)
> +{
> +	struct fsnotify_group *group = file->private_data;
> +
> +	fanotify_restart_pending_events(group);

I think fanotify_restart_pending_events() can run without the mutex, cannot
it? The queue fd is closing, new cannot be created until we clear
queue_opened. Then we can just lock the mutex here, clear queue_opened, and
drop it. It looks a bit cleaner if handling of queue_opened isn't hidden in
fanotify_restart_pending_events(). Also given how trivial
fanotify_restart_pending_events() ended up being, I'd just squash that
commit with this one.

> +	fsnotify_put_group(group);
> +	return 0;
> +}
> +
>  static int fanotify_release(struct inode *ignored, struct file *file)
>  {
>  	struct fsnotify_group *group = file->private_data;
> @@ -1182,6 +1203,44 @@ static int fanotify_release(struct inode *ignored, struct file *file)
>  	return 0;
>  }
>  
> +static int fanotify_open_queue_fd(struct file *file)
> +{
> +	struct fsnotify_group *group = file->private_data;
> +	int f_flags, fd;
> +	struct file *queue_file;
> +
> +	if (!FAN_GROUP_FLAG(group, FAN_RESTARTABLE_EVENTS))
> +		return -EINVAL;
> +
> +	guard(mutex)(&group->fanotify_data.queue_mutex);

Do we really need a new mutex for this? AFAICS it is used only to serialize
fanotify_open_queue_fd() calls and fanotify_queue_release() calls. I think
we can happily use fsnotify_group_lock() / fsnotify_group_unlock() for
this?

> +
> +	if (group->fanotify_data.queue_opened)
> +		return -EBUSY;
> +
> +	f_flags = O_RDWR;
> +	if (group->fanotify_data.flags & FAN_CLOEXEC)
> +		f_flags |= O_CLOEXEC;
> +	if (group->fanotify_data.flags & FAN_NONBLOCK)
> +		f_flags |= O_NONBLOCK;
> +
> +	fd = get_unused_fd_flags(f_flags);
> +	if (fd < 0)
> +		return fd;
> +
> +	queue_file = anon_inode_getfile_fmode("[fanotify-queue]",
> +					      &fanotify_queue_fops, group,
> +					      f_flags, FMODE_NONOTIFY);

Please use new FD_ADD() infrastructure for this. fanotify_init() already
uses it.

> +	if (IS_ERR(queue_file)) {
> +		put_unused_fd(fd);
> +		return PTR_ERR(queue_file);
> +	}
> +	fsnotify_get_group(group);
> +	fd_install(fd, queue_file);
> +	group->fanotify_data.queue_opened = true;
> +
> +	return fd;
> +}
> +
>  static long fanotify_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>  {
>  	struct fsnotify_group *group;
> @@ -1207,7 +1266,21 @@ static long fanotify_ioctl(struct file *file, unsigned int cmd, unsigned long ar
>  	return ret;
>  }
>  
> -static const struct file_operations fanotify_fops = {
> +static long fanotify_control_ioctl(struct file *file, unsigned int cmd,
> +				   unsigned long arg)
> +{
> +	int ret = -ENOTTY;
> +
> +	switch (cmd) {
> +	case FAN_IOC_OPEN_QUEUE_FD:
> +		ret = fanotify_open_queue_fd(file);
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +const struct file_operations fanotify_fops = {
>  	.show_fdinfo	= fanotify_show_fdinfo,
>  	.poll		= fanotify_poll,
>  	.read		= fanotify_read,
> @@ -1219,6 +1292,23 @@ static const struct file_operations fanotify_fops = {
>  	.llseek		= noop_llseek,
>  };
>  
> +const struct file_operations fanotify_control_fops = {
> +	.show_fdinfo	= fanotify_show_fdinfo,
> +	.release	= fanotify_release,
> +	.unlocked_ioctl	= fanotify_control_ioctl,
> +	.llseek		= noop_llseek,
> +};
> +
> +const struct file_operations fanotify_queue_fops = {
> +	.poll		= fanotify_poll,
> +	.read		= fanotify_read,
> +	.write		= fanotify_write,
> +	.release	= fanotify_queue_release,
> +	.unlocked_ioctl	= fanotify_ioctl,
> +	.compat_ioctl	= compat_ptr_ioctl,
> +	.llseek		= noop_llseek,
> +};

The new scheme with queue fd & control fd opens up an interesting new set
of races which needs a deeper thought I think. As it is currently
implemented for example if there's a task waiting for event on queue fd, it
will never wake up if control fd is closed. Similar problem is there for
fanotify_poll(). So we need to carefully define how operations on queue fd
synchronize with operations on control fd and closing of it in particular.
Besides dealing with the hung waits (which can be dealt with by just waking
the notification_waitq from fanotify_release()) we also need to deal with
fanotify_read() moving events from notification list to access list. I'm a
bit unhappy with how ad hoc and fragile this looks but so far don't have an
idea for a cleaner solution.

> +
>  static int fanotify_find_path(int dfd, const char __user *filename,
>  			      struct path *path, unsigned int flags, __u64 mask,
>  			      unsigned int obj_type)
> @@ -1640,6 +1730,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>  	int f_flags, fd;
>  	unsigned int fid_mode = flags & FANOTIFY_FID_BITS;
>  	unsigned int class = flags & FANOTIFY_CLASS_BITS;
> +	unsigned int restartable_events = flags & FAN_RESTARTABLE_EVENTS;
						  ^^^ Can we perhaps call
this FAN_EVENT_FD or something like that? Because I can forsee we will find
other interesting ways how to use the scheme with two fds and not all of
them tied to restarting of permission events. So FAN_RESTARTABLE_EVENTS
will become confusing at that point.

>  	unsigned int internal_flags = 0;
>  
>  	pr_debug("%s: flags=%x event_f_flags=%x\n",
> @@ -1718,10 +1809,17 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>  	    (!(fid_mode & FAN_REPORT_NAME) || !(fid_mode & FAN_REPORT_FID)))
>  		return -EINVAL;
>  
> -	f_flags = O_RDWR;
> +	/*
> +	 * FAN_RESTARTABLE_EVENTS requires FAN_CLASS_CONTENT or
> +	 * FAN_CLASS_PRE_CONTENT
> +	 */
> +	if (restartable_events && class == FAN_CLASS_NOTIF)
> +		return -EINVAL;
> +
> +	f_flags = restartable_events ? O_RDONLY : O_RDWR;

I don't think we really need to differentiate control vs queue fd with
O_RDONLY / O_RDWR. I'd just leave it with O_RDWR. Without implemented ops
any attempt to read / write will fail with EINVAL anyway.

>  	if (flags & FAN_CLOEXEC)
>  		f_flags |= O_CLOEXEC;
> -	if (flags & FAN_NONBLOCK)
> +	if (!restartable_events && (flags & FAN_NONBLOCK))
>  		f_flags |= O_NONBLOCK;
>  
>  	CLASS(fsnotify_group, group)(&fanotify_fsnotify_ops,
> @@ -1781,7 +1880,9 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>  	}
>  
>  	fd = FD_ADD(f_flags,
> -		    anon_inode_getfile_fmode("[fanotify]", &fanotify_fops,
> +		    anon_inode_getfile_fmode("[fanotify]",
> +					     (restartable_events ? &fanotify_control_fops :
> +					     &fanotify_fops),
>  					     group, f_flags, FMODE_NONOTIFY));
>  	if (fd >= 0)
>  		retain_and_null_ptr(group);
> @@ -1999,7 +2100,8 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>  		return -EBADF;
>  
>  	/* verify that this is indeed an fanotify instance */
> -	if (unlikely(fd_file(f)->f_op != &fanotify_fops))
> +	if (unlikely(fd_file(f)->f_op != &fanotify_fops &&
> +		fd_file(f)->f_op != &fanotify_control_fops))

Please indent like:

	if (unlikely(fd_file(f)->f_op != &fanotify_fops &&
		     fd_file(f)->f_op != &fanotify_control_fops))

>  		return -EINVAL;
>  	group = fd_file(f)->private_data;
>  
> @@ -2035,6 +2137,15 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>  		 group->priority == FSNOTIFY_PRIO_CONTENT)
>  		return -EINVAL;
>  
> +	/*
> +	 * With FAN_RESTARTABLE_EVENTS, a user is only allowed to setup
> +	 * permission events and modify event flags.
> +	 */
> +	if (FAN_GROUP_FLAG(group, FAN_RESTARTABLE_EVENTS) &&
> +	    (mask & ~(FANOTIFY_FD_EVENTS | FANOTIFY_MOUNT_EVENTS |
> +		      FANOTIFY_EVENT_FLAGS)))
> +		return -EINVAL;
> +

Amir already commented this is wrong...

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v3 2/2] fanotify: introduce restartable permission events

[Amir Goldstein]

On Mon, Apr 20, 2026 at 4:37 PM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 16-04-26 12:48:44, Ibrahim Jirdeh wrote:
> > This adds support for restarting permission events. The main goal of
> > the change is to provide better handling for pending events for lazy
> > file loading use cases which may back fanotify events by a long-lived
> > daemon. For prior discussion of approaches see [1][2].
> >
> > In terms of implementation, we add a new control-fd/queue-fd api.
> > Control fd returned by fanotify_init keeps fanotify group alive and
> > supports operations like fanotify_mark as well as a new ioctl
> > FAN_IOC_OPEN_QUEUE_FD to open a queue fd. Queue fd is used
> > for reading events and writing back responses. Upon release of
> > queue fd, pending permission events are reinserted back into
> > notification queue for reprocessing.
> >
> > Control-fd/queue-fd api is guarded by FAN_RESTARTABLE_EVENTS flag.
> > In addition FAN_RESTARTABLE_EVENTS can only be used in conjunction
> > with FAN_CLASS_CONTENT or FAN_CLASS_PRE_CONTENT, and only permission
> > events can be added to the mark mask if a group is initialized with
> > FAN_RESTARTABLE_EVENTS.
> >
> > [1] https://lore.kernel.org/linux-fsdevel/6za2mngeqslmqjg3icoubz37hbbxi6bi44canfsg2aajgkialt@c3ujlrjzkppr
> > [2] https://lore.kernel.org/linux-fsdevel/20250623192503.2673076-1-ibrahimjirdeh@meta.com
> >
> > Suggested-by: Amir Goldstein <amir73il@gmail.com>
> > Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhN6ok6BCBGbxeUt9ULq6g=qL6=_2_QGi8MqTHv5ZN7Vg@mail.gmail.com
> > Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
>
...

> > +
> >  static int fanotify_find_path(int dfd, const char __user *filename,
> >                             struct path *path, unsigned int flags, __u64 mask,
> >                             unsigned int obj_type)
> > @@ -1640,6 +1730,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
> >       int f_flags, fd;
> >       unsigned int fid_mode = flags & FANOTIFY_FID_BITS;
> >       unsigned int class = flags & FANOTIFY_CLASS_BITS;
> > +     unsigned int restartable_events = flags & FAN_RESTARTABLE_EVENTS;
>                                                   ^^^ Can we perhaps call
> this FAN_EVENT_FD or something like that? Because I can forsee we will find
> other interesting ways how to use the scheme with two fds and not all of
> them tied to restarting of permission events. So FAN_RESTARTABLE_EVENTS
> will become confusing at that point.
>

FAN_EVENT_FD sounds too close to event->fd and to FANOTIFY_FD_EVENTS
Maybe FAN_CONTROL_FD, which is what is requested from fanotify_init()?

Thanks,
Amir.

Re: [PATCH v3 2/2] fanotify: introduce restartable permission events

[Jan Kara]

On Mon 20-04-26 20:07:45, Amir Goldstein wrote:
> On Mon, Apr 20, 2026 at 4:37 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Thu 16-04-26 12:48:44, Ibrahim Jirdeh wrote:
> > > This adds support for restarting permission events. The main goal of
> > > the change is to provide better handling for pending events for lazy
> > > file loading use cases which may back fanotify events by a long-lived
> > > daemon. For prior discussion of approaches see [1][2].
> > >
> > > In terms of implementation, we add a new control-fd/queue-fd api.
> > > Control fd returned by fanotify_init keeps fanotify group alive and
> > > supports operations like fanotify_mark as well as a new ioctl
> > > FAN_IOC_OPEN_QUEUE_FD to open a queue fd. Queue fd is used
> > > for reading events and writing back responses. Upon release of
> > > queue fd, pending permission events are reinserted back into
> > > notification queue for reprocessing.
> > >
> > > Control-fd/queue-fd api is guarded by FAN_RESTARTABLE_EVENTS flag.
> > > In addition FAN_RESTARTABLE_EVENTS can only be used in conjunction
> > > with FAN_CLASS_CONTENT or FAN_CLASS_PRE_CONTENT, and only permission
> > > events can be added to the mark mask if a group is initialized with
> > > FAN_RESTARTABLE_EVENTS.
> > >
> > > [1] https://lore.kernel.org/linux-fsdevel/6za2mngeqslmqjg3icoubz37hbbxi6bi44canfsg2aajgkialt@c3ujlrjzkppr
> > > [2] https://lore.kernel.org/linux-fsdevel/20250623192503.2673076-1-ibrahimjirdeh@meta.com
> > >
> > > Suggested-by: Amir Goldstein <amir73il@gmail.com>
> > > Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhN6ok6BCBGbxeUt9ULq6g=qL6=_2_QGi8MqTHv5ZN7Vg@mail.gmail.com
> > > Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
> >
> ...
> 
> > > +
> > >  static int fanotify_find_path(int dfd, const char __user *filename,
> > >                             struct path *path, unsigned int flags, __u64 mask,
> > >                             unsigned int obj_type)
> > > @@ -1640,6 +1730,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
> > >       int f_flags, fd;
> > >       unsigned int fid_mode = flags & FANOTIFY_FID_BITS;
> > >       unsigned int class = flags & FANOTIFY_CLASS_BITS;
> > > +     unsigned int restartable_events = flags & FAN_RESTARTABLE_EVENTS;
> >                                                   ^^^ Can we perhaps call
> > this FAN_EVENT_FD or something like that? Because I can forsee we will find
> > other interesting ways how to use the scheme with two fds and not all of
> > them tied to restarting of permission events. So FAN_RESTARTABLE_EVENTS
> > will become confusing at that point.
> >
> 
> FAN_EVENT_FD sounds too close to event->fd and to FANOTIFY_FD_EVENTS
> Maybe FAN_CONTROL_FD, which is what is requested from fanotify_init()?

Yes, FAN_CONTROL_FD sounds fine.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v3 1/2] fanotify: add restart infrastructure for pending permission events

[Ibrahim Jirdeh]

On Mon, Apr 20, 2026 at 7:37 AM Jan Kara <jack@suse.cz> wrote:
>
> On Fri 17-04-26 13:09:59, Amir Goldstein wrote:
> > On Thu, Apr 16, 2026 at 9:49 PM Ibrahim Jirdeh <ibrahimjirdeh@meta.com> wrote:
> > >
> > > Add fanotify_restart_pending_events() helper that walks the
> > > access_list and reinserts each pending permission event back into
> > > the notification queue for reprocessing. Events are reinserted at
> > > the head of the notification queue without merge or overflow handling.
> > >
> > > Suggested-by: Jan Kara <jack@suse.cz>
> > > Link: https://lore.kernel.org/linux-fsdevel/sx5g7pmkchjqucfbzi77xh7wx4wua5nteqi5bsa2hfqgxua2a2@v7x6ja3gsirn/
> > > Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
> > > ---
> > >  fs/notify/fanotify/fanotify_user.c | 30 ++++++++++++++++++++++++++++++
> > >  1 file changed, 30 insertions(+)
> > >
> > > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> > > index d0b9b984002f..787e57a00466 100644
> > > --- a/fs/notify/fanotify/fanotify_user.c
> > > +++ b/fs/notify/fanotify/fanotify_user.c
> > > @@ -1095,6 +1095,36 @@ static ssize_t fanotify_write(struct file *file, const char __user *buf, size_t
> > >         return count;
> > >  }
> > >
> > > +/*
> > > + * Reinsert pending permission events at the head of the notification
> > > + * queue for reprocessing. No merge or overflow handling.
> > > + */
> > > +static void fanotify_restart_pending_events(struct fsnotify_group *group)
> > > +{
> > > +       spin_lock(&group->notification_lock);
> > > +       while (!list_empty(&group->fanotify_data.access_list)) {
> > > +               struct fanotify_perm_event *event;
> > > +
> > > +               event = list_first_entry(&group->fanotify_data.access_list,
> > > +                                        struct fanotify_perm_event,
> > > +                                        fae.fse.list);
> > > +               list_del_init(&event->fae.fse.list);
> > > +
> > > +               if (group->shutdown) {
> > > +                       /* Group is being shut down. Reply ALLOW for the event. */
> > > +                       finish_permission_event(group, event, FAN_ALLOW, NULL);
> > > +                       spin_lock(&group->notification_lock);
> >
> > This looks very awkward.
> >
> > We are holding the notification lock for the non-shutdown case
> > throughout moving the events back into the notification_list
> > (could possibly do that with list_move() if we keep a count
> > of in-progress events).
> >
> > Seems more reasonable to also finish all events in a single batch
> > if the group is shutdown and then maybe use the same helper in
> > fanotify_release().
> >
> > But Ibrahim, please wait for Jan's input on this before sending v4.
> >
> > There are many ways to do this, and reusing the existing
> > finish_permission_event() locking semantics in this loop may be
> > fine for simplicity.
> >
> > It is also a matter of taste, so please wait for Jan to chime in.
>
> So I wouldn't overcomplicate and just go for something like:
>
>         spin_lock(&group->notification_lock);
>         /* Group is exiting, fanotify_release() will remove pending events */
>         if (group->shutdown) {
>                 spin_unlock(&group->notification_lock);
>                 return;
>         }
>         group->q_len += list_count_nodes(&group->fanotify_data.access_list);
>         list_splice_init(&group->fanotify_data.access_list,
>                          &group->notification_list);
>         spin_unlock(&group->notification_lock);
>

Thanks this is much simpler. I will update it.

Re: [PATCH v3 2/2] fanotify: introduce restartable permission events

[Ibrahim Jirdeh]

On Mon, Apr 20, 2026 at 7:37 AM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 16-04-26 12:48:44, Ibrahim Jirdeh wrote:
> > This adds support for restarting permission events. The main goal of
> > the change is to provide better handling for pending events for lazy
> > file loading use cases which may back fanotify events by a long-lived
> > daemon. For prior discussion of approaches see [1][2].
> >
> > In terms of implementation, we add a new control-fd/queue-fd api.
> > Control fd returned by fanotify_init keeps fanotify group alive and
> > supports operations like fanotify_mark as well as a new ioctl
> > FAN_IOC_OPEN_QUEUE_FD to open a queue fd. Queue fd is used
> > for reading events and writing back responses. Upon release of
> > queue fd, pending permission events are reinserted back into
> > notification queue for reprocessing.
> >
> > Control-fd/queue-fd api is guarded by FAN_RESTARTABLE_EVENTS flag.
> > In addition FAN_RESTARTABLE_EVENTS can only be used in conjunction
> > with FAN_CLASS_CONTENT or FAN_CLASS_PRE_CONTENT, and only permission
> > events can be added to the mark mask if a group is initialized with
> > FAN_RESTARTABLE_EVENTS.
> >
> > [1] https://lore.kernel.org/linux-fsdevel/6za2mngeqslmqjg3icoubz37hbbxi6bi44canfsg2aajgkialt@c3ujlrjzkppr
> > [2] https://lore.kernel.org/linux-fsdevel/20250623192503.2673076-1-ibrahimjirdeh@meta.com
> >
> > Suggested-by: Amir Goldstein <amir73il@gmail.com>
> > Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxhN6ok6BCBGbxeUt9ULq6g=qL6=_2_QGi8MqTHv5ZN7Vg@mail.gmail.com
> > Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
>
> Thanks for the patch. Some comments below...
>

Thanks for reviewing the patch Jan and Amir.

> > diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> > index 39e60218df7c..dd64af8313a9 100644
> > --- a/fs/notify/fanotify/fanotify.h
> > +++ b/fs/notify/fanotify/fanotify.h
> > @@ -552,3 +552,7 @@ static inline u32 fanotify_get_response_errno(int res)
> >  {
> >       return (res >> FAN_ERRNO_SHIFT) & FAN_ERRNO_MASK;
> >  }
> > +
> > +extern const struct file_operations fanotify_fops;
> > +extern const struct file_operations fanotify_control_fops;
> > +extern const struct file_operations fanotify_queue_fops;
>
> Why cannot the ops remain static? I didn't find anything in this patch that
> would require them to be extern...
>

Will update, static with forward declaration should also work.

> > diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> > index 787e57a00466..b4610c49daab 100644
> > --- a/fs/notify/fanotify/fanotify_user.c
> > +++ b/fs/notify/fanotify/fanotify_user.c
> > @@ -1018,6 +1018,15 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
> >                       if (file->f_flags & O_NONBLOCK)
> >                               break;
> >
> > +                     /*
> > +                      * With FAN_RESTARTABLE_EVENTS, queue fd may
> > +                      * outlive control fd. Return EOF if group
> > +                      * is shut down.
> > +                      */
> > +                     ret = 0;
> > +                     if (group->shutdown)
> > +                             break;
> > +
> >                       ret = -ERESTARTSYS;
> >                       if (signal_pending(current))
> >                               break;
> > @@ -1101,6 +1110,7 @@ static ssize_t fanotify_write(struct file *file, const char __user *buf, size_t
> >   */
> >  static void fanotify_restart_pending_events(struct fsnotify_group *group)
> >  {
> > +     mutex_lock(&group->fanotify_data.queue_mutex);
> >       spin_lock(&group->notification_lock);
> >       while (!list_empty(&group->fanotify_data.access_list)) {
> >               struct fanotify_perm_event *event;
> > @@ -1121,10 +1131,21 @@ static void fanotify_restart_pending_events(struct fsnotify_group *group)
> >               }
> >       }
> >       spin_unlock(&group->notification_lock);
> > +     group->fanotify_data.queue_opened = false;
> > +     mutex_unlock(&group->fanotify_data.queue_mutex);
> >
> >       wake_up(&group->notification_waitq);
> >  }
> >
> > +static int fanotify_queue_release(struct inode *ignored, struct file *file)
> > +{
> > +     struct fsnotify_group *group = file->private_data;
> > +
> > +     fanotify_restart_pending_events(group);
>
> I think fanotify_restart_pending_events() can run without the mutex, cannot
> it? The queue fd is closing, new cannot be created until we clear
> queue_opened. Then we can just lock the mutex here, clear queue_opened, and
> drop it. It looks a bit cleaner if handling of queue_opened isn't hidden in
> fanotify_restart_pending_events(). Also given how trivial
> fanotify_restart_pending_events() ended up being, I'd just squash that
> commit with this one.

That makes sense. I can change to only holding the lock for modifying
queue_opened
in the next version.

>
> > +     fsnotify_put_group(group);
> > +     return 0;
> > +}
> > +
> >  static int fanotify_release(struct inode *ignored, struct file *file)
> >  {
> >       struct fsnotify_group *group = file->private_data;
> > @@ -1182,6 +1203,44 @@ static int fanotify_release(struct inode *ignored, struct file *file)
> >       return 0;
> >  }
> >
> > +static int fanotify_open_queue_fd(struct file *file)
> > +{
> > +     struct fsnotify_group *group = file->private_data;
> > +     int f_flags, fd;
> > +     struct file *queue_file;
> > +
> > +     if (!FAN_GROUP_FLAG(group, FAN_RESTARTABLE_EVENTS))
> > +             return -EINVAL;
> > +
> > +     guard(mutex)(&group->fanotify_data.queue_mutex);
>
> Do we really need a new mutex for this? AFAICS it is used only to serialize
> fanotify_open_queue_fd() calls and fanotify_queue_release() calls. I think
> we can happily use fsnotify_group_lock() / fsnotify_group_unlock() for
> this?
>

Its accurate that the new mutex is only used for serializing queue open and
release operations. I can update to reuse the existing fsnotify_group_lock,
was unsure if it was acceptable to contend with existing usages like
fanotify_mark.

> > +
> > +     if (group->fanotify_data.queue_opened)
> > +             return -EBUSY;
> > +
> > +     f_flags = O_RDWR;
> > +     if (group->fanotify_data.flags & FAN_CLOEXEC)
> > +             f_flags |= O_CLOEXEC;
> > +     if (group->fanotify_data.flags & FAN_NONBLOCK)
> > +             f_flags |= O_NONBLOCK;
> > +
> > +     fd = get_unused_fd_flags(f_flags);
> > +     if (fd < 0)
> > +             return fd;
> > +
> > +     queue_file = anon_inode_getfile_fmode("[fanotify-queue]",
> > +                                           &fanotify_queue_fops, group,
> > +                                           f_flags, FMODE_NONOTIFY);
>
> Please use new FD_ADD() infrastructure for this. fanotify_init() already
> uses it.
>
> > +     if (IS_ERR(queue_file)) {
> > +             put_unused_fd(fd);
> > +             return PTR_ERR(queue_file);
> > +     }
> > +     fsnotify_get_group(group);
> > +     fd_install(fd, queue_file);
> > +     group->fanotify_data.queue_opened = true;
> > +
> > +     return fd;
> > +}
> > +
> >  static long fanotify_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> >  {
> >       struct fsnotify_group *group;
> > @@ -1207,7 +1266,21 @@ static long fanotify_ioctl(struct file *file, unsigned int cmd, unsigned long ar
> >       return ret;
> >  }
> >
> > -static const struct file_operations fanotify_fops = {
> > +static long fanotify_control_ioctl(struct file *file, unsigned int cmd,
> > +                                unsigned long arg)
> > +{
> > +     int ret = -ENOTTY;
> > +
> > +     switch (cmd) {
> > +     case FAN_IOC_OPEN_QUEUE_FD:
> > +             ret = fanotify_open_queue_fd(file);
> > +             break;
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +const struct file_operations fanotify_fops = {
> >       .show_fdinfo    = fanotify_show_fdinfo,
> >       .poll           = fanotify_poll,
> >       .read           = fanotify_read,
> > @@ -1219,6 +1292,23 @@ static const struct file_operations fanotify_fops = {
> >       .llseek         = noop_llseek,
> >  };
> >
> > +const struct file_operations fanotify_control_fops = {
> > +     .show_fdinfo    = fanotify_show_fdinfo,
> > +     .release        = fanotify_release,
> > +     .unlocked_ioctl = fanotify_control_ioctl,
> > +     .llseek         = noop_llseek,
> > +};
> > +
> > +const struct file_operations fanotify_queue_fops = {
> > +     .poll           = fanotify_poll,
> > +     .read           = fanotify_read,
> > +     .write          = fanotify_write,
> > +     .release        = fanotify_queue_release,
> > +     .unlocked_ioctl = fanotify_ioctl,
> > +     .compat_ioctl   = compat_ptr_ioctl,
> > +     .llseek         = noop_llseek,
> > +};
>
> The new scheme with queue fd & control fd opens up an interesting new set
> of races which needs a deeper thought I think. As it is currently
> implemented for example if there's a task waiting for event on queue fd, it
> will never wake up if control fd is closed. Similar problem is there for
> fanotify_poll(). So we need to carefully define how operations on queue fd
> synchronize with operations on control fd and closing of it in particular.
> Besides dealing with the hung waits (which can be dealt with by just waking
> the notification_waitq from fanotify_release()) we also need to deal with
> fanotify_read() moving events from notification list to access list. I'm a
> bit unhappy with how ad hoc and fragile this looks but so far don't have an
> idea for a cleaner solution.
>

I wanted to ask a few more questions here about the race conditions you are
outlining. Is it accurate that the main risk of races is primarily
related to early
control fd shutdown with this new two fd case, or are there other races to worry
about? I added a few protections in this patch so far and wanted to
check if there
are outsanding risks:
1) Have queue fd also maintain a group reference as well
2) Check for control fd close (group->shutdown) in fanotify_read. I will also
incorporate the wake up in fanotify_release you have suggested here.

Regarding fanotify_read moving events from notification list to access list, do
we need to allow pending events in fanotify_restart_pending_events
after all to prevent potential hung waits stemming from race with shutdown?

> > +
> >  static int fanotify_find_path(int dfd, const char __user *filename,
> >                             struct path *path, unsigned int flags, __u64 mask,
> >                             unsigned int obj_type)
> > @@ -1640,6 +1730,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
> >       int f_flags, fd;
> >       unsigned int fid_mode = flags & FANOTIFY_FID_BITS;
> >       unsigned int class = flags & FANOTIFY_CLASS_BITS;
> > +     unsigned int restartable_events = flags & FAN_RESTARTABLE_EVENTS;
>                                                   ^^^ Can we perhaps call
> this FAN_EVENT_FD or something like that? Because I can forsee we will find
> other interesting ways how to use the scheme with two fds and not all of
> them tied to restarting of permission events. So FAN_RESTARTABLE_EVENTS
> will become confusing at that point.
>

Will rename this to FAN_CONTROL_FD per Amir's suggestion.

> >       unsigned int internal_flags = 0;
> >
> >       pr_debug("%s: flags=%x event_f_flags=%x\n",
> > @@ -1718,10 +1809,17 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
> >           (!(fid_mode & FAN_REPORT_NAME) || !(fid_mode & FAN_REPORT_FID)))
> >               return -EINVAL;
> >
> > -     f_flags = O_RDWR;
> > +     /*
> > +      * FAN_RESTARTABLE_EVENTS requires FAN_CLASS_CONTENT or
> > +      * FAN_CLASS_PRE_CONTENT
> > +      */
> > +     if (restartable_events && class == FAN_CLASS_NOTIF)
> > +             return -EINVAL;
> > +
> > +     f_flags = restartable_events ? O_RDONLY : O_RDWR;
>
> I don't think we really need to differentiate control vs queue fd with
> O_RDONLY / O_RDWR. I'd just leave it with O_RDWR. Without implemented ops
> any attempt to read / write will fail with EINVAL anyway.
>
> >       if (flags & FAN_CLOEXEC)
> >               f_flags |= O_CLOEXEC;
> > -     if (flags & FAN_NONBLOCK)
> > +     if (!restartable_events && (flags & FAN_NONBLOCK))
> >               f_flags |= O_NONBLOCK;
> >
> >       CLASS(fsnotify_group, group)(&fanotify_fsnotify_ops,
> > @@ -1781,7 +1880,9 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
> >       }
> >
> >       fd = FD_ADD(f_flags,
> > -                 anon_inode_getfile_fmode("[fanotify]", &fanotify_fops,
> > +                 anon_inode_getfile_fmode("[fanotify]",
> > +                                          (restartable_events ? &fanotify_control_fops :
> > +                                          &fanotify_fops),
> >                                            group, f_flags, FMODE_NONOTIFY));
> >       if (fd >= 0)
> >               retain_and_null_ptr(group);
> > @@ -1999,7 +2100,8 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
> >               return -EBADF;
> >
> >       /* verify that this is indeed an fanotify instance */
> > -     if (unlikely(fd_file(f)->f_op != &fanotify_fops))
> > +     if (unlikely(fd_file(f)->f_op != &fanotify_fops &&
> > +             fd_file(f)->f_op != &fanotify_control_fops))
>
> Please indent like:
>
>         if (unlikely(fd_file(f)->f_op != &fanotify_fops &&
>                      fd_file(f)->f_op != &fanotify_control_fops))
>
> >               return -EINVAL;
> >       group = fd_file(f)->private_data;
> >
> > @@ -2035,6 +2137,15 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
> >                group->priority == FSNOTIFY_PRIO_CONTENT)
> >               return -EINVAL;
> >
> > +     /*
> > +      * With FAN_RESTARTABLE_EVENTS, a user is only allowed to setup
> > +      * permission events and modify event flags.
> > +      */
> > +     if (FAN_GROUP_FLAG(group, FAN_RESTARTABLE_EVENTS) &&
> > +         (mask & ~(FANOTIFY_FD_EVENTS | FANOTIFY_MOUNT_EVENTS |
> > +                   FANOTIFY_EVENT_FLAGS)))
> > +             return -EINVAL;
> > +
>
> Amir already commented this is wrong...

Re: [PATCH v3 2/2] fanotify: introduce restartable permission events

[Jan Kara]

On Wed 22-04-26 03:22:32, Ibrahim Jirdeh wrote:
> On Mon, Apr 20, 2026 at 7:37 AM Jan Kara <jack@suse.cz> wrote:
> > > +const struct file_operations fanotify_control_fops = {
> > > +     .show_fdinfo    = fanotify_show_fdinfo,
> > > +     .release        = fanotify_release,
> > > +     .unlocked_ioctl = fanotify_control_ioctl,
> > > +     .llseek         = noop_llseek,
> > > +};
> > > +
> > > +const struct file_operations fanotify_queue_fops = {
> > > +     .poll           = fanotify_poll,
> > > +     .read           = fanotify_read,
> > > +     .write          = fanotify_write,
> > > +     .release        = fanotify_queue_release,
> > > +     .unlocked_ioctl = fanotify_ioctl,
> > > +     .compat_ioctl   = compat_ptr_ioctl,
> > > +     .llseek         = noop_llseek,
> > > +};
> >
> > The new scheme with queue fd & control fd opens up an interesting new set
> > of races which needs a deeper thought I think. As it is currently
> > implemented for example if there's a task waiting for event on queue fd, it
> > will never wake up if control fd is closed. Similar problem is there for
> > fanotify_poll(). So we need to carefully define how operations on queue fd
> > synchronize with operations on control fd and closing of it in particular.
> > Besides dealing with the hung waits (which can be dealt with by just waking
> > the notification_waitq from fanotify_release()) we also need to deal with
> > fanotify_read() moving events from notification list to access list. I'm a
> > bit unhappy with how ad hoc and fragile this looks but so far don't have an
> > idea for a cleaner solution.
> >
> 
> I wanted to ask a few more questions here about the race conditions you are
> outlining. Is it accurate that the main risk of races is primarily
> related to early control fd shutdown with this new two fd case, or are
> there other races to worry about?

No, this is my main concern.

> I added a few protections in this patch so far and wanted to check if
> there are outsanding risks:
> 1) Have queue fd also maintain a group reference as well

Yes, this is good and takes care that the group is not freed. But it still
can be fully cleaned up even if you hold a reference.

> 2) Check for control fd close (group->shutdown) in fanotify_read. I will also
> incorporate the wake up in fanotify_release you have suggested here.

Yes, I've noticed that but that check is racy...

> Regarding fanotify_read moving events from notification list to access list, do
> we need to allow pending events in fanotify_restart_pending_events
> after all to prevent potential hung waits stemming from race with shutdown?

Thinking about this some more... Won't it be easier if the queue fd held a
struct file reference of the control fd? That way the notification group
couldn't be shut down until queue fd is also closed. That will deal with
all the possible races, we don't even have to hold a notification group
reference. I don't think this behavior would be surprising to userspace and
I don't see any other possible downsides either. Amir, any opinion?

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v3 2/2] fanotify: introduce restartable permission events

[Amir Goldstein]

On Wed, Apr 22, 2026 at 12:36 PM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 22-04-26 03:22:32, Ibrahim Jirdeh wrote:
> > On Mon, Apr 20, 2026 at 7:37 AM Jan Kara <jack@suse.cz> wrote:
> > > > +const struct file_operations fanotify_control_fops = {
> > > > +     .show_fdinfo    = fanotify_show_fdinfo,
> > > > +     .release        = fanotify_release,
> > > > +     .unlocked_ioctl = fanotify_control_ioctl,
> > > > +     .llseek         = noop_llseek,
> > > > +};
> > > > +
> > > > +const struct file_operations fanotify_queue_fops = {
> > > > +     .poll           = fanotify_poll,
> > > > +     .read           = fanotify_read,
> > > > +     .write          = fanotify_write,
> > > > +     .release        = fanotify_queue_release,
> > > > +     .unlocked_ioctl = fanotify_ioctl,
> > > > +     .compat_ioctl   = compat_ptr_ioctl,
> > > > +     .llseek         = noop_llseek,
> > > > +};
> > >
> > > The new scheme with queue fd & control fd opens up an interesting new set
> > > of races which needs a deeper thought I think. As it is currently
> > > implemented for example if there's a task waiting for event on queue fd, it
> > > will never wake up if control fd is closed. Similar problem is there for
> > > fanotify_poll(). So we need to carefully define how operations on queue fd
> > > synchronize with operations on control fd and closing of it in particular.
> > > Besides dealing with the hung waits (which can be dealt with by just waking
> > > the notification_waitq from fanotify_release()) we also need to deal with
> > > fanotify_read() moving events from notification list to access list. I'm a
> > > bit unhappy with how ad hoc and fragile this looks but so far don't have an
> > > idea for a cleaner solution.
> > >
> >
> > I wanted to ask a few more questions here about the race conditions you are
> > outlining. Is it accurate that the main risk of races is primarily
> > related to early control fd shutdown with this new two fd case, or are
> > there other races to worry about?
>
> No, this is my main concern.
>
> > I added a few protections in this patch so far and wanted to check if
> > there are outsanding risks:
> > 1) Have queue fd also maintain a group reference as well
>
> Yes, this is good and takes care that the group is not freed. But it still
> can be fully cleaned up even if you hold a reference.
>
> > 2) Check for control fd close (group->shutdown) in fanotify_read. I will also
> > incorporate the wake up in fanotify_release you have suggested here.
>
> Yes, I've noticed that but that check is racy...
>
> > Regarding fanotify_read moving events from notification list to access list, do
> > we need to allow pending events in fanotify_restart_pending_events
> > after all to prevent potential hung waits stemming from race with shutdown?
>
> Thinking about this some more... Won't it be easier if the queue fd held a
> struct file reference of the control fd? That way the notification group
> couldn't be shut down until queue fd is also closed. That will deal with
> all the possible races, we don't even have to hold a notification group
> reference. I don't think this behavior would be surprising to userspace and
> I don't see any other possible downsides either. Amir, any opinion?
>

hmm, that sounds pretty neat.

queue_file->private_data = control_file;
control_file->private_data = group;

Makes sense to me.

Please remove the group->shutdown check in read and any other
safe race guards that were added.

Thanks,
Amir.

Re: [PATCH v3 2/2] fanotify: introduce restartable permission events

[Ibrahim Jirdeh]

On Wed, Apr 22, 2026 at 4:18 AM Amir Goldstein <amir73il@gmail.com> wrote:
> On Wed, Apr 22, 2026 at 12:36 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Wed 22-04-26 03:22:32, Ibrahim Jirdeh wrote:
> > > On Mon, Apr 20, 2026 at 7:37 AM Jan Kara <jack@suse.cz> wrote:
> > > > > +const struct file_operations fanotify_control_fops = {
> > > > > +     .show_fdinfo    = fanotify_show_fdinfo,
> > > > > +     .release        = fanotify_release,
> > > > > +     .unlocked_ioctl = fanotify_control_ioctl,
> > > > > +     .llseek         = noop_llseek,
> > > > > +};
> > > > > +
> > > > > +const struct file_operations fanotify_queue_fops = {
> > > > > +     .poll           = fanotify_poll,
> > > > > +     .read           = fanotify_read,
> > > > > +     .write          = fanotify_write,
> > > > > +     .release        = fanotify_queue_release,
> > > > > +     .unlocked_ioctl = fanotify_ioctl,
> > > > > +     .compat_ioctl   = compat_ptr_ioctl,
> > > > > +     .llseek         = noop_llseek,
> > > > > +};
> > > >
> > > > The new scheme with queue fd & control fd opens up an interesting new set
> > > > of races which needs a deeper thought I think. As it is currently
> > > > implemented for example if there's a task waiting for event on queue fd, it
> > > > will never wake up if control fd is closed. Similar problem is there for
> > > > fanotify_poll(). So we need to carefully define how operations on queue fd
> > > > synchronize with operations on control fd and closing of it in particular.
> > > > Besides dealing with the hung waits (which can be dealt with by just waking
> > > > the notification_waitq from fanotify_release()) we also need to deal with
> > > > fanotify_read() moving events from notification list to access list. I'm a
> > > > bit unhappy with how ad hoc and fragile this looks but so far don't have an
> > > > idea for a cleaner solution.
> > > >
> > >
> > > I wanted to ask a few more questions here about the race conditions you are
> > > outlining. Is it accurate that the main risk of races is primarily
> > > related to early control fd shutdown with this new two fd case, or are
> > > there other races to worry about?
> >
> > No, this is my main concern.
> >
> > > I added a few protections in this patch so far and wanted to check if
> > > there are outsanding risks:
> > > 1) Have queue fd also maintain a group reference as well
> >
> > Yes, this is good and takes care that the group is not freed. But it still
> > can be fully cleaned up even if you hold a reference.
> >
> > > 2) Check for control fd close (group->shutdown) in fanotify_read. I will also
> > > incorporate the wake up in fanotify_release you have suggested here.
> >
> > Yes, I've noticed that but that check is racy...
> >
> > > Regarding fanotify_read moving events from notification list to access list, do
> > > we need to allow pending events in fanotify_restart_pending_events
> > > after all to prevent potential hung waits stemming from race with shutdown?
> >
> > Thinking about this some more... Won't it be easier if the queue fd held a
> > struct file reference of the control fd? That way the notification group
> > couldn't be shut down until queue fd is also closed. That will deal with
> > all the possible races, we don't even have to hold a notification group
> > reference. I don't think this behavior would be surprising to userspace and
> > I don't see any other possible downsides either. Amir, any opinion?
> >
>
> hmm, that sounds pretty neat.
>
> queue_file->private_data = control_file;
> control_file->private_data = group;
>
> Makes sense to me.
>
> Please remove the group->shutdown check in read and any other
> safe race guards that were added.

Thanks, this should be much more straightforward. I will send over an updated
version containing this change to hold file reference and other fixes soon.

Re: [PATCH v3 2/2] fanotify: introduce restartable permission events

[Gao Xiang]

Hi Ibrahim, all,

On Wed, Apr 22, 2026 at 09:54:37AM -0700, Ibrahim Jirdeh wrote:
> On Wed, Apr 22, 2026 at 4:18 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > On Wed, Apr 22, 2026 at 12:36 PM Jan Kara <jack@suse.cz> wrote:
> > >
> > > On Wed 22-04-26 03:22:32, Ibrahim Jirdeh wrote:
> > > > On Mon, Apr 20, 2026 at 7:37 AM Jan Kara <jack@suse.cz> wrote:
> > > > > > +const struct file_operations fanotify_control_fops = {
> > > > > > +     .show_fdinfo    = fanotify_show_fdinfo,
> > > > > > +     .release        = fanotify_release,
> > > > > > +     .unlocked_ioctl = fanotify_control_ioctl,
> > > > > > +     .llseek         = noop_llseek,
> > > > > > +};
> > > > > > +
> > > > > > +const struct file_operations fanotify_queue_fops = {
> > > > > > +     .poll           = fanotify_poll,
> > > > > > +     .read           = fanotify_read,
> > > > > > +     .write          = fanotify_write,
> > > > > > +     .release        = fanotify_queue_release,
> > > > > > +     .unlocked_ioctl = fanotify_ioctl,
> > > > > > +     .compat_ioctl   = compat_ptr_ioctl,
> > > > > > +     .llseek         = noop_llseek,
> > > > > > +};
> > > > >
> > > > > The new scheme with queue fd & control fd opens up an interesting new set
> > > > > of races which needs a deeper thought I think. As it is currently
> > > > > implemented for example if there's a task waiting for event on queue fd, it
> > > > > will never wake up if control fd is closed. Similar problem is there for
> > > > > fanotify_poll(). So we need to carefully define how operations on queue fd
> > > > > synchronize with operations on control fd and closing of it in particular.
> > > > > Besides dealing with the hung waits (which can be dealt with by just waking
> > > > > the notification_waitq from fanotify_release()) we also need to deal with
> > > > > fanotify_read() moving events from notification list to access list. I'm a
> > > > > bit unhappy with how ad hoc and fragile this looks but so far don't have an
> > > > > idea for a cleaner solution.
> > > > >
> > > >
> > > > I wanted to ask a few more questions here about the race conditions you are
> > > > outlining. Is it accurate that the main risk of races is primarily
> > > > related to early control fd shutdown with this new two fd case, or are
> > > > there other races to worry about?
> > >
> > > No, this is my main concern.
> > >
> > > > I added a few protections in this patch so far and wanted to check if
> > > > there are outsanding risks:
> > > > 1) Have queue fd also maintain a group reference as well
> > >
> > > Yes, this is good and takes care that the group is not freed. But it still
> > > can be fully cleaned up even if you hold a reference.
> > >
> > > > 2) Check for control fd close (group->shutdown) in fanotify_read. I will also
> > > > incorporate the wake up in fanotify_release you have suggested here.
> > >
> > > Yes, I've noticed that but that check is racy...
> > >
> > > > Regarding fanotify_read moving events from notification list to access list, do
> > > > we need to allow pending events in fanotify_restart_pending_events
> > > > after all to prevent potential hung waits stemming from race with shutdown?
> > >
> > > Thinking about this some more... Won't it be easier if the queue fd held a
> > > struct file reference of the control fd? That way the notification group
> > > couldn't be shut down until queue fd is also closed. That will deal with
> > > all the possible races, we don't even have to hold a notification group
> > > reference. I don't think this behavior would be surprising to userspace and
> > > I don't see any other possible downsides either. Amir, any opinion?
> > >
> >
> > hmm, that sounds pretty neat.
> >
> > queue_file->private_data = control_file;
> > control_file->private_data = group;
> >
> > Makes sense to me.
> >
> > Please remove the group->shutdown check in read and any other
> > safe race guards that were added.
> 
> Thanks, this should be much more straightforward. I will send over an updated
> version containing this change to hold file reference and other fixes soon.

Just a friendly ping: I wonder if this series is still working in progress?
Currently fanotify lacks of failover implementation, and our EROFS fanotify
lazy pulling needs this too.

Thanks,
Gao Xiang