linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] fanotify: introduce response identifier
@ 2025-07-11  2:36 Ibrahim Jirdeh
  2025-07-11  2:36 ` [PATCH v3 1/3] fanotify: add support for a variable length permission event Ibrahim Jirdeh
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Ibrahim Jirdeh @ 2025-07-11  2:36 UTC (permalink / raw)
  To: ibrahimjirdeh; +Cc: jack, amir73il, josef, lesha, linux-fsdevel, sargun

These patches are in order to add an identifier other than fd which
can be used to respond to fanotify permission events. This is useful
for HSM use cases which are backed by a daemon to respond reliably
[1] and also for reporting pre-dir-content events without an event->fd [2]

The first few patches in the series are pulled in from Amir's work to
add support for pre-dir-content events.

In terms of testing, there are some additional LTP test cases to go with
these patches which exercise responding to events via response identifier
[3]

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

Changes v2 => v3:
- Add build assertions on new id fields and use union fields interchangeably


Changes v1 => v2:
- Moved away from adding a unique event identifier, and instead overload
event->fd to support a response identifier for now for FAN_REPORT_FID
setting.


[1] https://lore.kernel.org/all/CAMp4zn8aXNPzq1i8KYmbRfwDBvO5Qefa4isSyS1bwYuvkuBsHg@mail.gmail.com/
[2] https://lore.kernel.org/linux-fsdevel/2dx3pbcnv5w75fxb2ghqtsk6gzl6cuxmd2rinzwbq7xxfjf5z7@3nqidi3mno46/
[3] https://github.com/ibrahim-jirdeh/ltp/commit/91c963eb6758b12ce70b87ec060308017bf08ccb

Amir Goldstein (2):
  fanotify: add support for a variable length permission event
  fanotify: allow pre-content events with fid info

Ibrahim Jirdeh (1):
  fanotify: introduce event response identifier

 fs/notify/fanotify/fanotify.c       |  71 +++++++++++----
 fs/notify/fanotify/fanotify.h       |  30 +++++--
 fs/notify/fanotify/fanotify_user.c  | 133 +++++++++++++++++++++-------
 include/linux/fanotify.h            |   1 +
 include/linux/fsnotify_backend.h    |   2 +
 include/uapi/linux/fanotify.h       |  14 ++-
 tools/include/uapi/linux/fanotify.h |  11 ++-
 7 files changed, 203 insertions(+), 59 deletions(-)

--
2.47.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/3] fanotify: add support for a variable length permission event
  2025-07-11  2:36 [PATCH v3 0/3] fanotify: introduce response identifier Ibrahim Jirdeh
@ 2025-07-11  2:36 ` Ibrahim Jirdeh
  2025-07-11  2:36 ` [PATCH v3 2/3] fanotify: allow pre-content events with fid info Ibrahim Jirdeh
  2025-07-11  2:36 ` [PATCH v3 3/3] fanotify: introduce event response identifier Ibrahim Jirdeh
  2 siblings, 0 replies; 6+ messages in thread
From: Ibrahim Jirdeh @ 2025-07-11  2:36 UTC (permalink / raw)
  To: ibrahimjirdeh; +Cc: jack, amir73il, josef, lesha, linux-fsdevel, sargun

From: Amir Goldstein <amir73il@gmail.com>

In preparation for pre-content events that report fid info + name,
we need a new event type that is both variable length and can be
put on a user response wait list.

Create an event type FANOTIFY_EVENT_TYPE_FID_NAME_PERM with is a
combination of the variable length fanotify_name_event prefixed
with a fix length fanotify_perm_event and they share the common
fanotify_event memeber.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify.c | 68 +++++++++++++++++++++++++++--------
 fs/notify/fanotify/fanotify.h | 19 +++++++---
 2 files changed, 67 insertions(+), 20 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index bfe884d624e7..34acb7c16e8b 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -582,20 +582,13 @@ static struct fanotify_event *fanotify_alloc_mnt_event(u64 mnt_id, gfp_t gfp)
 	return &pevent->fae;
 }
 
-static struct fanotify_event *fanotify_alloc_perm_event(const void *data,
-							int data_type,
-							gfp_t gfp)
+static void fanotify_init_perm_event(struct fanotify_perm_event *pevent,
+				     const void *data, int data_type)
 {
 	const struct path *path = fsnotify_data_path(data, data_type);
 	const struct file_range *range =
 			    fsnotify_data_file_range(data, data_type);
-	struct fanotify_perm_event *pevent;
-
-	pevent = kmem_cache_alloc(fanotify_perm_event_cachep, gfp);
-	if (!pevent)
-		return NULL;
 
-	pevent->fae.type = FANOTIFY_EVENT_TYPE_PATH_PERM;
 	pevent->response = 0;
 	pevent->hdr.type = FAN_RESPONSE_INFO_NONE;
 	pevent->hdr.pad = 0;
@@ -606,6 +599,20 @@ static struct fanotify_event *fanotify_alloc_perm_event(const void *data,
 	pevent->ppos = range ? &range->pos : NULL;
 	pevent->count = range ? range->count : 0;
 	path_get(path);
+}
+
+static struct fanotify_event *fanotify_alloc_perm_event(const void *data,
+							int data_type,
+							gfp_t gfp)
+{
+	struct fanotify_perm_event *pevent;
+
+	pevent = kmem_cache_alloc(fanotify_perm_event_cachep, gfp);
+	if (!pevent)
+		return NULL;
+
+	pevent->fae.type = FANOTIFY_EVENT_TYPE_PATH_PERM;
+	fanotify_init_perm_event(pevent, data, data_type);
 
 	return &pevent->fae;
 }
@@ -636,11 +643,12 @@ static struct fanotify_event *fanotify_alloc_name_event(struct inode *dir,
 							struct inode *child,
 							struct dentry *moved,
 							unsigned int *hash,
-							gfp_t gfp)
+							gfp_t gfp, bool perm)
 {
 	struct fanotify_name_event *fne;
 	struct fanotify_info *info;
 	struct fanotify_fh *dfh, *ffh;
+	struct fanotify_perm_event *pevent;
 	struct inode *dir2 = moved ? d_inode(moved->d_parent) : NULL;
 	const struct qstr *name2 = moved ? &moved->d_name : NULL;
 	unsigned int dir_fh_len = fanotify_encode_fh_len(dir);
@@ -658,11 +666,26 @@ static struct fanotify_event *fanotify_alloc_name_event(struct inode *dir,
 		size += FANOTIFY_FH_HDR_LEN + dir2_fh_len;
 	if (child_fh_len)
 		size += FANOTIFY_FH_HDR_LEN + child_fh_len;
+	if (perm) {
+		BUILD_BUG_ON(offsetof(struct fanotify_perm_event, fae) +
+			     sizeof(struct fanotify_event) != sizeof(*pevent));
+		size += offsetof(struct fanotify_perm_event, fae);
+	}
 	fne = kmalloc(size, gfp);
 	if (!fne)
 		return NULL;
 
-	fne->fae.type = FANOTIFY_EVENT_TYPE_FID_NAME;
+	/*
+	 * fanotify_name_event follows fanotify_perm_event and they share the
+	 * fae member.
+	 */
+	if (perm) {
+		pevent = (void *)fne;
+		fne = FANOTIFY_NE(&pevent->fae);
+		fne->fae.type = FANOTIFY_EVENT_TYPE_FID_NAME_PERM;
+	} else {
+		fne->fae.type = FANOTIFY_EVENT_TYPE_FID_NAME;
+	}
 	fne->fsid = *fsid;
 	*hash ^= fanotify_hash_fsid(fsid);
 	info = &fne->info;
@@ -757,6 +780,7 @@ static struct fanotify_event *fanotify_alloc_event(
 	struct inode *dirid = fanotify_dfid_inode(mask, data, data_type, dir);
 	const struct path *path = fsnotify_data_path(data, data_type);
 	u64 mnt_id = fsnotify_data_mnt_id(data, data_type);
+	bool perm = fanotify_is_perm_event(mask);
 	struct mem_cgroup *old_memcg;
 	struct dentry *moved = NULL;
 	struct inode *child = NULL;
@@ -842,14 +866,18 @@ static struct fanotify_event *fanotify_alloc_event(
 	/* Whoever is interested in the event, pays for the allocation. */
 	old_memcg = set_active_memcg(group->memcg);
 
-	if (fanotify_is_perm_event(mask)) {
+	if (name_event && (file_name || moved || child || perm)) {
+		event = fanotify_alloc_name_event(dirid, fsid, file_name, child,
+						  moved, &hash, gfp, perm);
+		if (event && perm) {
+			fanotify_init_perm_event(FANOTIFY_PERM(event),
+						 data, data_type);
+		}
+	} else if (perm) {
 		event = fanotify_alloc_perm_event(data, data_type, gfp);
 	} else if (fanotify_is_error_event(mask)) {
 		event = fanotify_alloc_error_event(group, fsid, data,
 						   data_type, &hash);
-	} else if (name_event && (file_name || moved || child)) {
-		event = fanotify_alloc_name_event(dirid, fsid, file_name, child,
-						  moved, &hash, gfp);
 	} else if (fid_mode) {
 		event = fanotify_alloc_fid_event(id, fsid, &hash, gfp);
 	} else if (path) {
@@ -1037,6 +1065,13 @@ static void fanotify_free_perm_event(struct fanotify_event *event)
 	kmem_cache_free(fanotify_perm_event_cachep, FANOTIFY_PERM(event));
 }
 
+static void fanotify_free_name_perm_event(struct fanotify_event *event)
+{
+	path_put(fanotify_event_path(event));
+	/* Variable length perm event */
+	kfree(FANOTIFY_PERM(event));
+}
+
 static void fanotify_free_fid_event(struct fanotify_event *event)
 {
 	struct fanotify_fid_event *ffe = FANOTIFY_FE(event);
@@ -1084,6 +1119,9 @@ static void fanotify_free_event(struct fsnotify_group *group,
 	case FANOTIFY_EVENT_TYPE_FID_NAME:
 		fanotify_free_name_event(event);
 		break;
+	case FANOTIFY_EVENT_TYPE_FID_NAME_PERM:
+		fanotify_free_name_perm_event(event);
+		break;
 	case FANOTIFY_EVENT_TYPE_OVERFLOW:
 		kfree(event);
 		break;
diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
index b78308975082..f6d25fcf8692 100644
--- a/fs/notify/fanotify/fanotify.h
+++ b/fs/notify/fanotify/fanotify.h
@@ -240,6 +240,7 @@ static inline void fanotify_info_copy_name2(struct fanotify_info *info,
 enum fanotify_event_type {
 	FANOTIFY_EVENT_TYPE_FID, /* fixed length */
 	FANOTIFY_EVENT_TYPE_FID_NAME, /* variable length */
+	FANOTIFY_EVENT_TYPE_FID_NAME_PERM, /* variable length perm event */
 	FANOTIFY_EVENT_TYPE_PATH,
 	FANOTIFY_EVENT_TYPE_PATH_PERM,
 	FANOTIFY_EVENT_TYPE_OVERFLOW, /* struct fanotify_event */
@@ -326,7 +327,8 @@ static inline __kernel_fsid_t *fanotify_event_fsid(struct fanotify_event *event)
 {
 	if (event->type == FANOTIFY_EVENT_TYPE_FID)
 		return &FANOTIFY_FE(event)->fsid;
-	else if (event->type == FANOTIFY_EVENT_TYPE_FID_NAME)
+	else if (event->type == FANOTIFY_EVENT_TYPE_FID_NAME ||
+		 event->type == FANOTIFY_EVENT_TYPE_FID_NAME_PERM)
 		return &FANOTIFY_NE(event)->fsid;
 	else if (event->type == FANOTIFY_EVENT_TYPE_FS_ERROR)
 		return &FANOTIFY_EE(event)->fsid;
@@ -339,7 +341,8 @@ static inline struct fanotify_fh *fanotify_event_object_fh(
 {
 	if (event->type == FANOTIFY_EVENT_TYPE_FID)
 		return &FANOTIFY_FE(event)->object_fh;
-	else if (event->type == FANOTIFY_EVENT_TYPE_FID_NAME)
+	else if (event->type == FANOTIFY_EVENT_TYPE_FID_NAME ||
+		 event->type == FANOTIFY_EVENT_TYPE_FID_NAME_PERM)
 		return fanotify_info_file_fh(&FANOTIFY_NE(event)->info);
 	else if (event->type == FANOTIFY_EVENT_TYPE_FS_ERROR)
 		return &FANOTIFY_EE(event)->object_fh;
@@ -350,7 +353,8 @@ static inline struct fanotify_fh *fanotify_event_object_fh(
 static inline struct fanotify_info *fanotify_event_info(
 						struct fanotify_event *event)
 {
-	if (event->type == FANOTIFY_EVENT_TYPE_FID_NAME)
+	if (event->type == FANOTIFY_EVENT_TYPE_FID_NAME ||
+	    event->type == FANOTIFY_EVENT_TYPE_FID_NAME_PERM)
 		return &FANOTIFY_NE(event)->info;
 	else
 		return NULL;
@@ -435,7 +439,6 @@ FANOTIFY_ME(struct fanotify_event *event)
  * user response.
  */
 struct fanotify_perm_event {
-	struct fanotify_event fae;
 	struct path path;
 	const loff_t *ppos;		/* optional file range info */
 	size_t count;
@@ -446,6 +449,11 @@ struct fanotify_perm_event {
 		struct fanotify_response_info_header hdr;
 		struct fanotify_response_info_audit_rule audit_rule;
 	};
+	/*
+	 * Overlaps with fanotify_name_event::fae when type is
+	 * FANOTIFY_EVENT_TYPE_FID_NAME_PERM - Keep at the end!
+	 */
+	struct fanotify_event fae;
 };
 
 static inline struct fanotify_perm_event *
@@ -487,7 +495,8 @@ static inline const struct path *fanotify_event_path(struct fanotify_event *even
 {
 	if (event->type == FANOTIFY_EVENT_TYPE_PATH)
 		return &FANOTIFY_PE(event)->path;
-	else if (event->type == FANOTIFY_EVENT_TYPE_PATH_PERM)
+	else if (event->type == FANOTIFY_EVENT_TYPE_PATH_PERM ||
+		 event->type == FANOTIFY_EVENT_TYPE_FID_NAME_PERM)
 		return &FANOTIFY_PERM(event)->path;
 	else
 		return NULL;
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/3] fanotify: allow pre-content events with fid info
  2025-07-11  2:36 [PATCH v3 0/3] fanotify: introduce response identifier Ibrahim Jirdeh
  2025-07-11  2:36 ` [PATCH v3 1/3] fanotify: add support for a variable length permission event Ibrahim Jirdeh
@ 2025-07-11  2:36 ` Ibrahim Jirdeh
  2025-07-11  2:36 ` [PATCH v3 3/3] fanotify: introduce event response identifier Ibrahim Jirdeh
  2 siblings, 0 replies; 6+ messages in thread
From: Ibrahim Jirdeh @ 2025-07-11  2:36 UTC (permalink / raw)
  To: ibrahimjirdeh; +Cc: jack, amir73il, josef, lesha, linux-fsdevel, sargun

From: Amir Goldstein <amir73il@gmail.com>

Until now, the high priority classes (FAN_CLASS_*CONTENT), which are
required for permission and pre-content events, were not allowed to
report events with fid info.

This is partly because the event->fd is used as a key for the the
permission response and partly because we needed to chose between
allocating a permission event of fid event struct.

Allow reporting fid info with pre-content class with some restrictions:
1. Only pre-content events are allowed with such groups
2. No event flags are allowed (i.e. FAN_EVENT_ON_CHILD)

The flag FAN_EVENT_ON_CHILD is anyway ignored on sb/mount marks and
on an non-dir inode mark.

On a directory inode mark, FAN_PRE_ACCESS makes no sense without
FAN_EVENT_ON_CHILD, so in this case, this flag is implied.

Add a convenience macro FAN_CLASS_PRE_CONTENT_FID to initialize a
group for pre-content events which reports fid info and dedicate
a new higher priority for those groups.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify_user.c | 70 +++++++++++++++++++++++-------
 include/linux/fsnotify_backend.h   |  1 +
 include/uapi/linux/fanotify.h      |  3 ++
 3 files changed, 59 insertions(+), 15 deletions(-)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index b192ee068a7a..19d3f2d914fe 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -353,7 +353,7 @@ static int process_access_response(struct fsnotify_group *group,
 		break;
 	case FAN_DENY:
 		/* Custom errno is supported only for pre-content groups */
-		if (errno && group->priority != FSNOTIFY_PRIO_PRE_CONTENT)
+		if (errno && group->priority < FSNOTIFY_PRIO_PRE_CONTENT)
 			return -EINVAL;
 
 		/*
@@ -1383,6 +1383,16 @@ static int fanotify_group_init_error_pool(struct fsnotify_group *group)
 					 sizeof(struct fanotify_error_event));
 }
 
+/* Check for forbidden events/flags combinations */
+static bool fanotify_mask_is_valid(__u64 mask)
+{
+	/* Pre-content events do not support event flags */
+	if (mask & FANOTIFY_PRE_CONTENT_EVENTS && mask & FAN_ONDIR)
+		return false;
+
+	return true;
+}
+
 static int fanotify_may_update_existing_mark(struct fsnotify_mark *fsn_mark,
 					     __u32 mask, unsigned int fan_flags)
 {
@@ -1411,9 +1421,9 @@ static int fanotify_may_update_existing_mark(struct fsnotify_mark *fsn_mark,
 	    fsn_mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY)
 		return -EEXIST;
 
-	/* For now pre-content events are not generated for directories */
+	/* Check for forbidden event combinations after update */
 	mask |= fsn_mark->mask;
-	if (mask & FANOTIFY_PRE_CONTENT_EVENTS && mask & FAN_ONDIR)
+	if (!fanotify_mask_is_valid(mask))
 		return -EEXIST;
 
 	return 0;
@@ -1564,7 +1574,13 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 		return -EINVAL;
 	}
 
-	if (fid_mode && class != FAN_CLASS_NOTIF)
+	/*
+	 * Async events support any fid report mode.
+	 * Permission events do not support any fid report mode.
+	 * Pre-content events support only FAN_REPORT_DFID_NAME_TARGET mode.
+	 */
+	if (fid_mode && class != FAN_CLASS_NOTIF &&
+	    (class | fid_mode) != FAN_CLASS_PRE_CONTENT_FID)
 		return -EINVAL;
 
 	/*
@@ -1633,7 +1649,12 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 		group->priority = FSNOTIFY_PRIO_CONTENT;
 		break;
 	case FAN_CLASS_PRE_CONTENT:
-		group->priority = FSNOTIFY_PRIO_PRE_CONTENT;
+		/*
+		 * FAN_CLASS_PRE_CONTENT_FID is exclusively for pre-content
+		 * events, so it gets a higher priority.
+		 */
+		group->priority = fid_mode ? FSNOTIFY_PRIO_PRE_CONTENT_FID :
+					     FSNOTIFY_PRIO_PRE_CONTENT;
 		break;
 	default:
 		fd = -EINVAL;
@@ -1750,6 +1771,9 @@ static int fanotify_events_supported(struct fsnotify_group *group,
 				 (mask & FAN_RENAME) ||
 				 (flags & FAN_MARK_IGNORE);
 
+	if (!fanotify_mask_is_valid(mask))
+		return -EINVAL;
+
 	/*
 	 * Filesystems need to opt-into pre-content evnets (a.k.a HSM)
 	 * and they are only supported on regular files and directories.
@@ -1911,13 +1935,27 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 	/*
 	 * Permission events are not allowed for FAN_CLASS_NOTIF.
 	 * Pre-content permission events are not allowed for FAN_CLASS_CONTENT.
+	 * Only pre-content events are allowed for FAN_CLASS_PRE_CONTENT_FID.
 	 */
-	if (mask & FANOTIFY_PERM_EVENTS &&
-	    group->priority == FSNOTIFY_PRIO_NORMAL)
-		return -EINVAL;
-	else if (mask & FANOTIFY_PRE_CONTENT_EVENTS &&
-		 group->priority == FSNOTIFY_PRIO_CONTENT)
+	fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
+	switch (group->priority) {
+	case FSNOTIFY_PRIO_NORMAL:
+		if (mask & FANOTIFY_PERM_EVENTS)
+			return -EINVAL;
+		break;
+	case FSNOTIFY_PRIO_CONTENT:
+		if (mask & FANOTIFY_PRE_CONTENT_EVENTS)
+			return -EINVAL;
+		break;
+	case FSNOTIFY_PRIO_PRE_CONTENT:
+		break;
+	case FSNOTIFY_PRIO_PRE_CONTENT_FID:
+		if (mask & ~FANOTIFY_PRE_CONTENT_EVENTS)
+			return -EINVAL;
+		break;
+	default:
 		return -EINVAL;
+	}
 
 	if (mask & FAN_FS_ERROR &&
 	    mark_type != FAN_MARK_FILESYSTEM)
@@ -1938,7 +1976,6 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 	 * carry enough information (i.e. path) to be filtered by mount
 	 * point.
 	 */
-	fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
 	if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_MOUNT_EVENTS|FANOTIFY_EVENT_FLAGS) &&
 	    (!fid_mode || mark_type == FAN_MARK_MOUNT))
 		return -EINVAL;
@@ -1951,10 +1988,6 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 	if (mask & FAN_RENAME && !(fid_mode & FAN_REPORT_NAME))
 		return -EINVAL;
 
-	/* Pre-content events are not currently generated for directories. */
-	if (mask & FANOTIFY_PRE_CONTENT_EVENTS && mask & FAN_ONDIR)
-		return -EINVAL;
-
 	if (mark_cmd == FAN_MARK_FLUSH) {
 		fsnotify_clear_marks_by_group(group, obj_type);
 		return 0;
@@ -2041,6 +2074,13 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 		if ((fid_mode & FAN_REPORT_DIR_FID) &&
 		    (flags & FAN_MARK_ADD) && !ignore)
 			mask |= FAN_EVENT_ON_CHILD;
+	} else if (fid_mode && (mask & FANOTIFY_PRE_CONTENT_EVENTS)) {
+		/*
+		 * Pre-content events on directory inode mask implies that
+		 * we are watching access to children.
+		 */
+		mask |= FAN_EVENT_ON_CHILD;
+		umask = FAN_EVENT_ON_CHILD;
 	}
 
 	/* create/update an inode mark */
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index d4034ddaf392..832d94d783d9 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -201,6 +201,7 @@ enum fsnotify_group_prio {
 	FSNOTIFY_PRIO_NORMAL = 0,	/* normal notifiers, no permissions */
 	FSNOTIFY_PRIO_CONTENT,		/* fanotify permission events */
 	FSNOTIFY_PRIO_PRE_CONTENT,	/* fanotify pre-content events */
+	FSNOTIFY_PRIO_PRE_CONTENT_FID,	/* fanotify pre-content events with fid */
 	__FSNOTIFY_PRIO_NUM
 };
 
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index e710967c7c26..28074ab3e794 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -73,6 +73,9 @@
 /* Convenience macro - FAN_REPORT_TARGET_FID requires all other FID flags */
 #define FAN_REPORT_DFID_NAME_TARGET (FAN_REPORT_DFID_NAME | \
 				     FAN_REPORT_FID | FAN_REPORT_TARGET_FID)
+/* Convenience macro - FAN_CLASS_PRE_CONTENT requires all or no FID flags */
+#define FAN_CLASS_PRE_CONTENT_FID   (FAN_CLASS_PRE_CONTENT | \
+				     FAN_REPORT_DFID_NAME_TARGET)
 
 /* Deprecated - do not use this in programs and do not add new flags here! */
 #define FAN_ALL_INIT_FLAGS	(FAN_CLOEXEC | FAN_NONBLOCK | \
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/3] fanotify: introduce event response identifier
  2025-07-11  2:36 [PATCH v3 0/3] fanotify: introduce response identifier Ibrahim Jirdeh
  2025-07-11  2:36 ` [PATCH v3 1/3] fanotify: add support for a variable length permission event Ibrahim Jirdeh
  2025-07-11  2:36 ` [PATCH v3 2/3] fanotify: allow pre-content events with fid info Ibrahim Jirdeh
@ 2025-07-11  2:36 ` Ibrahim Jirdeh
  2025-07-11  6:33   ` Amir Goldstein
  2025-07-11 12:47   ` kernel test robot
  2 siblings, 2 replies; 6+ messages in thread
From: Ibrahim Jirdeh @ 2025-07-11  2:36 UTC (permalink / raw)
  To: ibrahimjirdeh; +Cc: jack, amir73il, josef, lesha, linux-fsdevel, sargun

This adds support for responding to events via response identifier. This
prevents races if there are multiple processes backing the same fanotify
group (eg. handover of fanotify group to new instance of a backing daemon).
It is also useful for reporting pre-dir-content events without an
event->fd:
https://lore.kernel.org/linux-fsdevel/2dx3pbcnv5w75fxb2ghqtsk6gzl6cuxmd2rinzwbq7xxfjf5z7@3nqidi3mno46/.

Rather than introducing a new event identifier field and extending
fanotify_event_metadata, we have opted to overload event->fd and restrict
this functionality to use-cases which are using file handle apis
(FAN_REPORT_FID).

In terms of how response ids are allocated, we use an idr for allocation
and restrict the id range to below -255 to ensure there is no overlap with
existing fd-as-identifier usage. We can also leverage the added idr for
more efficient lookup when handling response although that is not done
in this patch.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxheeLXdTLLWrixnTJcxVP+BV4ViXijbvERHPenzgDMUTA@mail.gmail.com/
Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
---
 fs/notify/fanotify/fanotify.c       |  3 ++
 fs/notify/fanotify/fanotify.h       | 11 ++++-
 fs/notify/fanotify/fanotify_user.c  | 63 ++++++++++++++++++++---------
 include/linux/fanotify.h            |  1 +
 include/linux/fsnotify_backend.h    |  1 +
 include/uapi/linux/fanotify.h       | 11 ++++-
 tools/include/uapi/linux/fanotify.h | 11 ++++-
 7 files changed, 77 insertions(+), 24 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 34acb7c16e8b..d9aebd359199 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -1106,6 +1106,9 @@ static void fanotify_free_event(struct fsnotify_group *group,
 
 	event = FANOTIFY_E(fsn_event);
 	put_pid(event->pid);
+	if (fanotify_is_perm_event(event->mask) &&
+	    FAN_GROUP_FLAG(group, FAN_REPORT_RESPONSE_ID))
+		idr_remove(&group->response_idr, -FANOTIFY_PERM(event)->id);
 	switch (event->type) {
 	case FANOTIFY_EVENT_TYPE_PATH:
 		fanotify_free_path_event(event);
diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
index f6d25fcf8692..b6a414f44acc 100644
--- a/fs/notify/fanotify/fanotify.h
+++ b/fs/notify/fanotify/fanotify.h
@@ -444,7 +444,7 @@ struct fanotify_perm_event {
 	size_t count;
 	u32 response;			/* userspace answer to the event */
 	unsigned short state;		/* state of the event */
-	int fd;		/* fd we passed to userspace for this event */
+	int id;		/* id we passed to userspace for this event */
 	union {
 		struct fanotify_response_info_header hdr;
 		struct fanotify_response_info_audit_rule audit_rule;
@@ -559,3 +559,12 @@ static inline u32 fanotify_get_response_errno(int res)
 {
 	return (res >> FAN_ERRNO_SHIFT) & FAN_ERRNO_MASK;
 }
+
+static inline bool fanotify_is_valid_response_id(struct fsnotify_group *group,
+						 int id)
+{
+	if (FAN_GROUP_FLAG(group, FAN_REPORT_RESPONSE_ID))
+		return id < -255;
+
+	return id >= 0;
+}
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 19d3f2d914fe..2e14db38d298 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -330,14 +330,19 @@ static int process_access_response(struct fsnotify_group *group,
 				   size_t info_len)
 {
 	struct fanotify_perm_event *event;
-	int fd = response_struct->fd;
+	int id = response_struct->id;
 	u32 response = response_struct->response;
 	int errno = fanotify_get_response_errno(response);
 	int ret = info_len;
 	struct fanotify_response_info_audit_rule friar;
 
-	pr_debug("%s: group=%p fd=%d response=%x errno=%d buf=%p size=%zu\n",
-		 __func__, group, fd, response, errno, info, info_len);
+	BUILD_BUG_ON(sizeof(response_struct->id) !=
+		     sizeof(response_struct->fd));
+	BUILD_BUG_ON(offsetof(struct fanotify_response, id) !=
+		     offsetof(struct fanotify_response, fd));
+
+	pr_debug("%s: group=%p id=%d response=%x errno=%d buf=%p size=%zu\n",
+		 __func__, group, id, response, errno, info, info_len);
 	/*
 	 * make sure the response is valid, if invalid we do nothing and either
 	 * userspace can send a valid response or we will clean it up after the
@@ -385,19 +390,18 @@ static int process_access_response(struct fsnotify_group *group,
 		ret = process_access_response_info(info, info_len, &friar);
 		if (ret < 0)
 			return ret;
-		if (fd == FAN_NOFD)
+		if (id == FAN_NOFD)
 			return ret;
 	} else {
 		ret = 0;
 	}
-
-	if (fd < 0)
+	if (!fanotify_is_valid_response_id(group, id))
 		return -EINVAL;
 
 	spin_lock(&group->notification_lock);
 	list_for_each_entry(event, &group->fanotify_data.access_list,
 			    fae.fse.list) {
-		if (event->fd != fd)
+		if (event->id != id)
 			continue;
 
 		list_del_init(&event->fae.fse.list);
@@ -765,14 +769,20 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
 	    task_tgid(current) != event->pid)
 		metadata.pid = 0;
 
-	/*
-	 * For now, fid mode is required for an unprivileged listener and
-	 * fid mode does not report fd in events.  Keep this check anyway
-	 * for safety in case fid mode requirement is relaxed in the future
-	 * to allow unprivileged listener to get events with no fd and no fid.
-	 */
-	if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
-	    path && path->mnt && path->dentry) {
+	if (FAN_GROUP_FLAG(group, FAN_REPORT_RESPONSE_ID)) {
+		ret = idr_alloc_cyclic(&group->response_idr, event, 256,
+				       INT_MAX, GFP_NOWAIT);
+		if (ret < 0)
+			return ret;
+		fd = -ret;
+	} else if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) && path &&
+		   path->mnt && path->dentry) {
+		/*
+		 * For now, fid mode is required for an unprivileged listener and
+		 * fid mode does not report fd in events.  Keep this check anyway
+		 * for safety in case fid mode requirement is relaxed in the future
+		 * to allow unprivileged listener to get events with no fd and no fid.
+		 */
 		fd = create_fd(group, path, &f);
 		/*
 		 * Opening an fd from dentry can fail for several reasons.
@@ -803,7 +813,11 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
 			}
 		}
 	}
-	if (FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR))
+
+	BUILD_BUG_ON(sizeof(metadata.id) != sizeof(metadata.fd));
+	BUILD_BUG_ON(offsetof(struct fanotify_event_metadata, id) !=
+		     offsetof(struct fanotify_event_metadata, fd));
+	if (FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR | FAN_REPORT_RESPONSE_ID))
 		metadata.fd = fd;
 	else
 		metadata.fd = fd >= 0 ? fd : FAN_NOFD;
@@ -859,7 +873,7 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
 		fd_install(pidfd, pidfd_file);
 
 	if (fanotify_is_perm_event(event->mask))
-		FANOTIFY_PERM(event)->fd = fd;
+		FANOTIFY_PERM(event)->id = fd;
 
 	return metadata.event_len;
 
@@ -944,7 +958,9 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
 		if (!fanotify_is_perm_event(event->mask)) {
 			fsnotify_destroy_event(group, &event->fse);
 		} else {
-			if (ret <= 0 || FANOTIFY_PERM(event)->fd < 0) {
+			if (ret <= 0 ||
+			    !fanotify_is_valid_response_id(
+				    group, FANOTIFY_PERM(event)->id)) {
 				spin_lock(&group->notification_lock);
 				finish_permission_event(group,
 					FANOTIFY_PERM(event), FAN_DENY, NULL);
@@ -1584,6 +1600,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 		return -EINVAL;
 
 	/*
+     * With group that reports fid info and allows pre-content events,
+     * user may request to get a response id instead of event->fd.
+     */
+	if ((flags & FAN_REPORT_RESPONSE_ID) &&
+	    (!fid_mode || class == FAN_CLASS_NOTIF))
+		return -EINVAL;
+
+	/*
 	 * Child name is reported with parent fid so requires dir fid.
 	 * We can report both child fid and dir fid with or without name.
 	 */
@@ -1660,6 +1684,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 		fd = -EINVAL;
 		goto out_destroy_group;
 	}
+	idr_init(&group->response_idr);
 
 	BUILD_BUG_ON(!(FANOTIFY_ADMIN_INIT_FLAGS & FAN_UNLIMITED_QUEUE));
 	if (flags & FAN_UNLIMITED_QUEUE) {
@@ -2145,7 +2170,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..85fce0a15005 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_REPORT_RESPONSE_ID | \
 					 FAN_UNLIMITED_QUEUE | \
 					 FAN_UNLIMITED_MARKS)
 
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 832d94d783d9..83c82331866b 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -232,6 +232,7 @@ struct fsnotify_group {
 	unsigned int max_events;		/* maximum events allowed on the list */
 	enum fsnotify_group_prio priority;	/* priority for sending events */
 	bool shutdown;		/* group is being shut down, don't queue more events */
+	struct idr response_idr; /* used for response id allocation */
 
 #define FSNOTIFY_GROUP_USER	0x01 /* user allocated group */
 #define FSNOTIFY_GROUP_DUPS	0x02 /* allow multiple marks per object */
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index 28074ab3e794..e705dda14dfc 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -67,6 +67,7 @@
 #define FAN_REPORT_TARGET_FID	0x00001000	/* Report dirent target id  */
 #define FAN_REPORT_FD_ERROR	0x00002000	/* event->fd can report error */
 #define FAN_REPORT_MNT		0x00004000	/* Report mount events */
+#define FAN_REPORT_RESPONSE_ID		0x00008000 /* event->fd is a response id */
 
 /* Convenience macro - FAN_REPORT_NAME requires FAN_REPORT_DIR_FID */
 #define FAN_REPORT_DFID_NAME	(FAN_REPORT_DIR_FID | FAN_REPORT_NAME)
@@ -144,7 +145,10 @@ struct fanotify_event_metadata {
 	__u8 reserved;
 	__u16 metadata_len;
 	__aligned_u64 mask;
-	__s32 fd;
+	union {
+		__s32 fd;
+		__s32 id; /* FAN_REPORT_RESPONSE_ID */
+	};
 	__s32 pid;
 };
 
@@ -228,7 +232,10 @@ struct fanotify_event_info_mnt {
 #define FAN_RESPONSE_INFO_AUDIT_RULE	1
 
 struct fanotify_response {
-	__s32 fd;
+	union {
+		__s32 fd;
+		__s32 id; /* FAN_REPORT_RESPONSE_ID */
+	};
 	__u32 response;
 };
 
diff --git a/tools/include/uapi/linux/fanotify.h b/tools/include/uapi/linux/fanotify.h
index e710967c7c26..6a3ada7c4abf 100644
--- a/tools/include/uapi/linux/fanotify.h
+++ b/tools/include/uapi/linux/fanotify.h
@@ -67,6 +67,7 @@
 #define FAN_REPORT_TARGET_FID	0x00001000	/* Report dirent target id  */
 #define FAN_REPORT_FD_ERROR	0x00002000	/* event->fd can report error */
 #define FAN_REPORT_MNT		0x00004000	/* Report mount events */
+#define FAN_REPORT_RESPONSE_ID		0x00008000 /* event->fd is a response id */
 
 /* Convenience macro - FAN_REPORT_NAME requires FAN_REPORT_DIR_FID */
 #define FAN_REPORT_DFID_NAME	(FAN_REPORT_DIR_FID | FAN_REPORT_NAME)
@@ -141,7 +142,10 @@ struct fanotify_event_metadata {
 	__u8 reserved;
 	__u16 metadata_len;
 	__aligned_u64 mask;
-	__s32 fd;
+	union {
+		__s32 fd;
+		__s32 id; /* FAN_REPORT_RESPONSE_ID */
+	};
 	__s32 pid;
 };
 
@@ -225,7 +229,10 @@ struct fanotify_event_info_mnt {
 #define FAN_RESPONSE_INFO_AUDIT_RULE	1
 
 struct fanotify_response {
-	__s32 fd;
+	union {
+		__s32 fd;
+		__s32 id; /* FAN_REPORT_RESPONSE_ID */
+	};
 	__u32 response;
 };
 
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 3/3] fanotify: introduce event response identifier
  2025-07-11  2:36 ` [PATCH v3 3/3] fanotify: introduce event response identifier Ibrahim Jirdeh
@ 2025-07-11  6:33   ` Amir Goldstein
  2025-07-11 12:47   ` kernel test robot
  1 sibling, 0 replies; 6+ messages in thread
From: Amir Goldstein @ 2025-07-11  6:33 UTC (permalink / raw)
  To: Ibrahim Jirdeh; +Cc: jack, josef, lesha, linux-fsdevel, sargun

On Fri, Jul 11, 2025 at 4:36 AM Ibrahim Jirdeh <ibrahimjirdeh@meta.com> wrote:
>
> This adds support for responding to events via response identifier. This
> prevents races if there are multiple processes backing the same fanotify
> group (eg. handover of fanotify group to new instance of a backing daemon).
> It is also useful for reporting pre-dir-content events without an
> event->fd:
> https://lore.kernel.org/linux-fsdevel/2dx3pbcnv5w75fxb2ghqtsk6gzl6cuxmd2rinzwbq7xxfjf5z7@3nqidi3mno46/.
>
> Rather than introducing a new event identifier field and extending
> fanotify_event_metadata, we have opted to overload event->fd and restrict
> this functionality to use-cases which are using file handle apis
> (FAN_REPORT_FID).
>
> In terms of how response ids are allocated, we use an idr for allocation
> and restrict the id range to below -255 to ensure there is no overlap with
> existing fd-as-identifier usage. We can also leverage the added idr for
> more efficient lookup when handling response although that is not done
> in this patch.
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxheeLXdTLLWrixnTJcxVP+BV4ViXijbvERHPenzgDMUTA@mail.gmail.com/
> Signed-off-by: Ibrahim Jirdeh <ibrahimjirdeh@meta.com>
> ---
>  fs/notify/fanotify/fanotify.c       |  3 ++
>  fs/notify/fanotify/fanotify.h       | 11 ++++-
>  fs/notify/fanotify/fanotify_user.c  | 63 ++++++++++++++++++++---------
>  include/linux/fanotify.h            |  1 +
>  include/linux/fsnotify_backend.h    |  1 +
>  include/uapi/linux/fanotify.h       | 11 ++++-
>  tools/include/uapi/linux/fanotify.h | 11 ++++-
>  7 files changed, 77 insertions(+), 24 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 34acb7c16e8b..d9aebd359199 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -1106,6 +1106,9 @@ static void fanotify_free_event(struct fsnotify_group *group,
>
>         event = FANOTIFY_E(fsn_event);
>         put_pid(event->pid);
> +       if (fanotify_is_perm_event(event->mask) &&
> +           FAN_GROUP_FLAG(group, FAN_REPORT_RESPONSE_ID))
> +               idr_remove(&group->response_idr, -FANOTIFY_PERM(event)->id);

idr_*() are not thread safe and there is no group lock held here.
But I misled you.
You should use ida_alloc_min()/ida_free() which are simpler and thread safe.


>         switch (event->type) {
>         case FANOTIFY_EVENT_TYPE_PATH:
>                 fanotify_free_path_event(event);
> diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
> index f6d25fcf8692..b6a414f44acc 100644
> --- a/fs/notify/fanotify/fanotify.h
> +++ b/fs/notify/fanotify/fanotify.h
> @@ -444,7 +444,7 @@ struct fanotify_perm_event {
>         size_t count;
>         u32 response;                   /* userspace answer to the event */
>         unsigned short state;           /* state of the event */
> -       int fd;         /* fd we passed to userspace for this event */
> +       int id;         /* id we passed to userspace for this event */
>         union {
>                 struct fanotify_response_info_header hdr;
>                 struct fanotify_response_info_audit_rule audit_rule;
> @@ -559,3 +559,12 @@ static inline u32 fanotify_get_response_errno(int res)
>  {
>         return (res >> FAN_ERRNO_SHIFT) & FAN_ERRNO_MASK;
>  }
> +
> +static inline bool fanotify_is_valid_response_id(struct fsnotify_group *group,
> +                                                int id)
> +{
> +       if (FAN_GROUP_FLAG(group, FAN_REPORT_RESPONSE_ID))
> +               return id < -255;
> +
> +       return id >= 0;
> +}
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 19d3f2d914fe..2e14db38d298 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -330,14 +330,19 @@ static int process_access_response(struct fsnotify_group *group,
>                                    size_t info_len)
>  {
>         struct fanotify_perm_event *event;
> -       int fd = response_struct->fd;
> +       int id = response_struct->id;
>         u32 response = response_struct->response;
>         int errno = fanotify_get_response_errno(response);
>         int ret = info_len;
>         struct fanotify_response_info_audit_rule friar;
>
> -       pr_debug("%s: group=%p fd=%d response=%x errno=%d buf=%p size=%zu\n",
> -                __func__, group, fd, response, errno, info, info_len);
> +       BUILD_BUG_ON(sizeof(response_struct->id) !=
> +                    sizeof(response_struct->fd));
> +       BUILD_BUG_ON(offsetof(struct fanotify_response, id) !=
> +                    offsetof(struct fanotify_response, fd));
> +
> +       pr_debug("%s: group=%p id=%d response=%x errno=%d buf=%p size=%zu\n",
> +                __func__, group, id, response, errno, info, info_len);
>         /*
>          * make sure the response is valid, if invalid we do nothing and either
>          * userspace can send a valid response or we will clean it up after the
> @@ -385,19 +390,18 @@ static int process_access_response(struct fsnotify_group *group,
>                 ret = process_access_response_info(info, info_len, &friar);
>                 if (ret < 0)
>                         return ret;
> -               if (fd == FAN_NOFD)
> +               if (id == FAN_NOFD)
>                         return ret;
>         } else {
>                 ret = 0;
>         }
> -
> -       if (fd < 0)
> +       if (!fanotify_is_valid_response_id(group, id))
>                 return -EINVAL;
>
>         spin_lock(&group->notification_lock);
>         list_for_each_entry(event, &group->fanotify_data.access_list,
>                             fae.fse.list) {
> -               if (event->fd != fd)
> +               if (event->id != id)
>                         continue;
>
>                 list_del_init(&event->fae.fse.list);
> @@ -765,14 +769,20 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>             task_tgid(current) != event->pid)
>                 metadata.pid = 0;
>
> -       /*
> -        * For now, fid mode is required for an unprivileged listener and
> -        * fid mode does not report fd in events.  Keep this check anyway
> -        * for safety in case fid mode requirement is relaxed in the future
> -        * to allow unprivileged listener to get events with no fd and no fid.
> -        */
> -       if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) &&
> -           path && path->mnt && path->dentry) {
> +       if (FAN_GROUP_FLAG(group, FAN_REPORT_RESPONSE_ID)) {
> +               ret = idr_alloc_cyclic(&group->response_idr, event, 256,
> +                                      INT_MAX, GFP_NOWAIT);

So please use ida_alloc_min() with GFP_KERNEL
there is no reason for special memory allocation flags in this context.

> +               if (ret < 0)
> +                       return ret;
> +               fd = -ret;
> +       } else if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) && path &&
> +                  path->mnt && path->dentry) {
> +               /*
> +                * For now, fid mode is required for an unprivileged listener and
> +                * fid mode does not report fd in events.

Actually, my FAN_CLASS_PRE_CONTENT_FID patch makes this comment
inaccurate. The code is still correct, but not the reasoning to explain it.

I could change it myself in my patch, but as you move the comment might as
well fix it.

 /*
  * For now, fid mode and no-permission-events class are required
  * for FANOTIFY_UNPRIV listener and fid mode does not report fd in
  * non-permission notification events.  Keep this check anyway...

> Keep this check anyway
> +                * for safety in case fid mode requirement is relaxed in the future
> +                * to allow unprivileged listener to get events with no fd and no fid.
> +                */
>                 fd = create_fd(group, path, &f);
>                 /*
>                  * Opening an fd from dentry can fail for several reasons.
> @@ -803,7 +813,11 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>                         }
>                 }
>         }
> -       if (FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR))
> +
> +       BUILD_BUG_ON(sizeof(metadata.id) != sizeof(metadata.fd));
> +       BUILD_BUG_ON(offsetof(struct fanotify_event_metadata, id) !=
> +                    offsetof(struct fanotify_event_metadata, fd));
> +       if (FAN_GROUP_FLAG(group, FAN_REPORT_FD_ERROR | FAN_REPORT_RESPONSE_ID))
>                 metadata.fd = fd;
>         else
>                 metadata.fd = fd >= 0 ? fd : FAN_NOFD;
> @@ -859,7 +873,7 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>                 fd_install(pidfd, pidfd_file);
>
>         if (fanotify_is_perm_event(event->mask))
> -               FANOTIFY_PERM(event)->fd = fd;
> +               FANOTIFY_PERM(event)->id = fd;
>
>         return metadata.event_len;
>
> @@ -944,7 +958,9 @@ static ssize_t fanotify_read(struct file *file, char __user *buf,
>                 if (!fanotify_is_perm_event(event->mask)) {
>                         fsnotify_destroy_event(group, &event->fse);
>                 } else {
> -                       if (ret <= 0 || FANOTIFY_PERM(event)->fd < 0) {
> +                       if (ret <= 0 ||
> +                           !fanotify_is_valid_response_id(
> +                                   group, FANOTIFY_PERM(event)->id)) {
>                                 spin_lock(&group->notification_lock);
>                                 finish_permission_event(group,
>                                         FANOTIFY_PERM(event), FAN_DENY, NULL);
> @@ -1584,6 +1600,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>                 return -EINVAL;
>
>         /*
> +     * With group that reports fid info and allows pre-content events,
> +     * user may request to get a response id instead of event->fd.
> +     */

Seems like the indentation here is off.

> +       if ((flags & FAN_REPORT_RESPONSE_ID) &&
> +           (!fid_mode || class == FAN_CLASS_NOTIF))
> +               return -EINVAL;
> +
> +       /*
>          * Child name is reported with parent fid so requires dir fid.
>          * We can report both child fid and dir fid with or without name.
>          */
> @@ -1660,6 +1684,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>                 fd = -EINVAL;
>                 goto out_destroy_group;
>         }
> +       idr_init(&group->response_idr);

So ida_init() and you are also missing ida_destroy() in
fanotify_free_group_priv()

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 3/3] fanotify: introduce event response identifier
  2025-07-11  2:36 ` [PATCH v3 3/3] fanotify: introduce event response identifier Ibrahim Jirdeh
  2025-07-11  6:33   ` Amir Goldstein
@ 2025-07-11 12:47   ` kernel test robot
  1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2025-07-11 12:47 UTC (permalink / raw)
  To: Ibrahim Jirdeh
  Cc: oe-kbuild-all, jack, amir73il, josef, lesha, linux-fsdevel,
	sargun

Hi Ibrahim,

kernel test robot noticed the following build errors:

[auto build test ERROR on jack-fs/fsnotify]
[also build test ERROR on linus/master v6.16-rc5 next-20250711]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ibrahim-Jirdeh/fanotify-add-support-for-a-variable-length-permission-event/20250711-103820
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
patch link:    https://lore.kernel.org/r/20250711023604.593885-4-ibrahimjirdeh%40meta.com
patch subject: [PATCH v3 3/3] fanotify: introduce event response identifier
config: csky-randconfig-002-20250711 (https://download.01.org/0day-ci/archive/20250711/202507112012.xU84okDN-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250711/202507112012.xU84okDN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507112012.xU84okDN-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from fs/notify/fdinfo.c:8:
   fs/notify/fanotify/fanotify.h: In function 'fanotify_is_valid_response_id':
>> include/linux/fanotify.h:9:19: error: 'struct fsnotify_group' has no member named 'fanotify_data'; did you mean 'inotify_data'?
       9 |         ((group)->fanotify_data.flags & (flag))
         |                   ^~~~~~~~~~~~~
   fs/notify/fanotify/fanotify.h:566:13: note: in expansion of macro 'FAN_GROUP_FLAG'
     566 |         if (FAN_GROUP_FLAG(group, FAN_REPORT_RESPONSE_ID))
         |             ^~~~~~~~~~~~~~


vim +9 include/linux/fanotify.h

ff0b16a9850e8a Eric Paris     2009-12-17   7  
96a71f21ef1fcc Amir Goldstein 2018-09-21   8  #define FAN_GROUP_FLAG(group, flag) \
96a71f21ef1fcc Amir Goldstein 2018-09-21  @9  	((group)->fanotify_data.flags & (flag))
96a71f21ef1fcc Amir Goldstein 2018-09-21  10  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-07-11 12:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-11  2:36 [PATCH v3 0/3] fanotify: introduce response identifier Ibrahim Jirdeh
2025-07-11  2:36 ` [PATCH v3 1/3] fanotify: add support for a variable length permission event Ibrahim Jirdeh
2025-07-11  2:36 ` [PATCH v3 2/3] fanotify: allow pre-content events with fid info Ibrahim Jirdeh
2025-07-11  2:36 ` [PATCH v3 3/3] fanotify: introduce event response identifier Ibrahim Jirdeh
2025-07-11  6:33   ` Amir Goldstein
2025-07-11 12:47   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).