* [PATCH v4 1/3] fanotify: add support for a variable length permission event
2025-07-11 18:30 [PATCH v4 0/3] fanotify: introduce response identifier Ibrahim Jirdeh
@ 2025-07-11 18:30 ` Ibrahim Jirdeh
2025-07-22 14:01 ` Jan Kara
2025-07-11 18:31 ` [PATCH v4 2/3] fanotify: allow pre-content events with fid info Ibrahim Jirdeh
2025-07-11 18:31 ` [PATCH v4 3/3] fanotify: introduce event response identifier Ibrahim Jirdeh
2 siblings, 1 reply; 11+ messages in thread
From: Ibrahim Jirdeh @ 2025-07-11 18:30 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] 11+ messages in thread* Re: [PATCH v4 1/3] fanotify: add support for a variable length permission event
2025-07-11 18:30 ` [PATCH v4 1/3] fanotify: add support for a variable length permission event Ibrahim Jirdeh
@ 2025-07-22 14:01 ` Jan Kara
2025-07-22 15:13 ` Amir Goldstein
0 siblings, 1 reply; 11+ messages in thread
From: Jan Kara @ 2025-07-22 14:01 UTC (permalink / raw)
To: Ibrahim Jirdeh; +Cc: jack, amir73il, josef, lesha, linux-fsdevel, sargun
Hello!
Sorry for a bit delayed reply I was busy with other work...
On Fri 11-07-25 11:30:59, Ibrahim Jirdeh wrote:
> 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>
As a procedural note, this patch should have your Signed-off-by as well
Ibrahim, when you resend it as part of your patch set.
Now to the content: Amir, this looks quite hacky to me and I think we can
do better. How about:
struct fanotify_perm_event {
struct fanotify_event fae;
const loff_t *ppos; /* optional file range info */
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 */
union {
struct fanotify_response_info_header hdr;
struct fanotify_response_info_audit_rule audit_rule;
};
union {
struct path path;
struct {
__kernel_fsid_t fsid;
struct fanotify_info info;
};
};
};
This actually doesn't grow struct fanotify_perm_event and should make
things more or less bussiness as usual.
Honza
> 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
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 1/3] fanotify: add support for a variable length permission event
2025-07-22 14:01 ` Jan Kara
@ 2025-07-22 15:13 ` Amir Goldstein
2025-07-23 7:43 ` Jan Kara
0 siblings, 1 reply; 11+ messages in thread
From: Amir Goldstein @ 2025-07-22 15:13 UTC (permalink / raw)
To: Jan Kara; +Cc: Ibrahim Jirdeh, josef, lesha, linux-fsdevel, sargun
On Tue, Jul 22, 2025 at 4:01 PM Jan Kara <jack@suse.cz> wrote:
>
> Hello!
>
> Sorry for a bit delayed reply I was busy with other work...
>
> On Fri 11-07-25 11:30:59, Ibrahim Jirdeh wrote:
> > 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>
>
> As a procedural note, this patch should have your Signed-off-by as well
> Ibrahim, when you resend it as part of your patch set.
Right, but I don't think Ibrahim needs those patches anymore,
because as you said FAN_RESTARTABLE_EVENTS do not require
a response id.
>
> Now to the content: Amir, this looks quite hacky to me and I think we can
> do better. How about:
>
> struct fanotify_perm_event {
> struct fanotify_event fae;
> const loff_t *ppos; /* optional file range info */
> 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 */
> union {
> struct fanotify_response_info_header hdr;
> struct fanotify_response_info_audit_rule audit_rule;
> };
> union {
> struct path path;
> struct {
> __kernel_fsid_t fsid;
> struct fanotify_info info;
> };
> };
> };
>
> This actually doesn't grow struct fanotify_perm_event and should make
> things more or less bussiness as usual.
Yeh, that's less hacky for sure.
My hacky patches were keeping the path alongside the variable fid,
but I agree that we can go for response_id for the pre-dir-access events.
However, unless it is urgent to Ibrahim, I am not going to pick this up before
the end of August..
Thanks,
Amir.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 1/3] fanotify: add support for a variable length permission event
2025-07-22 15:13 ` Amir Goldstein
@ 2025-07-23 7:43 ` Jan Kara
2025-07-23 10:24 ` Amir Goldstein
0 siblings, 1 reply; 11+ messages in thread
From: Jan Kara @ 2025-07-23 7:43 UTC (permalink / raw)
To: Amir Goldstein
Cc: Jan Kara, Ibrahim Jirdeh, josef, lesha, linux-fsdevel, sargun
On Tue 22-07-25 17:13:07, Amir Goldstein wrote:
> On Tue, Jul 22, 2025 at 4:01 PM Jan Kara <jack@suse.cz> wrote:
> > Sorry for a bit delayed reply I was busy with other work...
> >
> > On Fri 11-07-25 11:30:59, Ibrahim Jirdeh wrote:
> > > 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>
> >
> > As a procedural note, this patch should have your Signed-off-by as well
> > Ibrahim, when you resend it as part of your patch set.
>
> Right, but I don't think Ibrahim needs those patches anymore,
> because as you said FAN_RESTARTABLE_EVENTS do not require
> a response id.
Right, I've lost track a bit of who needs what :). So nobody is currently
striving to get the response ID changes merged? Just that I don't waste
time reviewing changes nobody is interested in at this point...
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v4 1/3] fanotify: add support for a variable length permission event
2025-07-23 7:43 ` Jan Kara
@ 2025-07-23 10:24 ` Amir Goldstein
0 siblings, 0 replies; 11+ messages in thread
From: Amir Goldstein @ 2025-07-23 10:24 UTC (permalink / raw)
To: Jan Kara; +Cc: Ibrahim Jirdeh, josef, lesha, linux-fsdevel, sargun
On Wed, Jul 23, 2025 at 9:43 AM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 22-07-25 17:13:07, Amir Goldstein wrote:
> > On Tue, Jul 22, 2025 at 4:01 PM Jan Kara <jack@suse.cz> wrote:
> > > Sorry for a bit delayed reply I was busy with other work...
> > >
> > > On Fri 11-07-25 11:30:59, Ibrahim Jirdeh wrote:
> > > > 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>
> > >
> > > As a procedural note, this patch should have your Signed-off-by as well
> > > Ibrahim, when you resend it as part of your patch set.
> >
> > Right, but I don't think Ibrahim needs those patches anymore,
> > because as you said FAN_RESTARTABLE_EVENTS do not require
> > a response id.
>
> Right, I've lost track a bit of who needs what :). So nobody is currently
> striving to get the response ID changes merged? Just that I don't waste
> time reviewing changes nobody is interested in at this point...
Yeh, it's hard to keep track, but IIUC, no need for respose_id patch
or my prep patch for FAN_CLASS_PRE_CONTENT_FID for now.
If you have time, it would be great if you could take a high-level peek at
https://github.com/amir73il/linux/commits/fan_pre_dir_access/
but as I wrote, I won't be getting back to that before end of August and
when I do, it is likely that I will pick up Ibrahim's response_id patch
and make those events fd-less.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 2/3] fanotify: allow pre-content events with fid info
2025-07-11 18:30 [PATCH v4 0/3] fanotify: introduce response identifier Ibrahim Jirdeh
2025-07-11 18:30 ` [PATCH v4 1/3] fanotify: add support for a variable length permission event Ibrahim Jirdeh
@ 2025-07-11 18:31 ` Ibrahim Jirdeh
2025-07-11 18:31 ` [PATCH v4 3/3] fanotify: introduce event response identifier Ibrahim Jirdeh
2 siblings, 0 replies; 11+ messages in thread
From: Ibrahim Jirdeh @ 2025-07-11 18:31 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] 11+ messages in thread* [PATCH v4 3/3] fanotify: introduce event response identifier
2025-07-11 18:30 [PATCH v4 0/3] fanotify: introduce response identifier Ibrahim Jirdeh
2025-07-11 18:30 ` [PATCH v4 1/3] fanotify: add support for a variable length permission event Ibrahim Jirdeh
2025-07-11 18:31 ` [PATCH v4 2/3] fanotify: allow pre-content events with fid info Ibrahim Jirdeh
@ 2025-07-11 18:31 ` Ibrahim Jirdeh
2025-07-12 6:16 ` Amir Goldstein
` (2 more replies)
2 siblings, 3 replies; 11+ messages in thread
From: Ibrahim Jirdeh @ 2025-07-11 18:31 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 ida for allocation
and restrict the id range to below -255 to ensure there is no overlap with
existing fd-as-identifier usage.
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 | 4 ++
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, 78 insertions(+), 24 deletions(-)
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 34acb7c16e8b..307532464226 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -1045,6 +1045,7 @@ static void fanotify_free_group_priv(struct fsnotify_group *group)
{
put_user_ns(group->user_ns);
kfree(group->fanotify_data.merge_hash);
+ ida_destroy(&group->response_ida);
if (group->fanotify_data.ucounts)
dec_ucount(group->fanotify_data.ucounts,
UCOUNT_FANOTIFY_GROUPS);
@@ -1106,6 +1107,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))
+ ida_free(&group->response_ida, -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..99af23b257f9 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 = ida_alloc_min(&group->response_ida, 256, GFP_KERNEL);
+ if (ret < 0)
+ return ret;
+ fd = -ret;
+ } else if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) && path &&
+ path->mnt && path->dentry) {
+ /*
+ * 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 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);
@@ -1583,6 +1599,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
(class | fid_mode) != FAN_CLASS_PRE_CONTENT_FID)
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;
}
+ ida_init(&group->response_ida);
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..27299d5ca668 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 ida response_ida; /* 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] 11+ messages in thread* Re: [PATCH v4 3/3] fanotify: introduce event response identifier
2025-07-11 18:31 ` [PATCH v4 3/3] fanotify: introduce event response identifier Ibrahim Jirdeh
@ 2025-07-12 6:16 ` Amir Goldstein
2025-07-12 20:51 ` kernel test robot
2025-07-16 6:25 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: Amir Goldstein @ 2025-07-12 6:16 UTC (permalink / raw)
To: Ibrahim Jirdeh; +Cc: jack, josef, lesha, linux-fsdevel, sargun
On Fri, Jul 11, 2025 at 8:32 PM 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 ida for allocation
> and restrict the id range to below -255 to ensure there is no overlap with
> existing fd-as-identifier usage.
>
> 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>
Looks nice!
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
> fs/notify/fanotify/fanotify.c | 4 ++
> 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, 78 insertions(+), 24 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 34acb7c16e8b..307532464226 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -1045,6 +1045,7 @@ static void fanotify_free_group_priv(struct fsnotify_group *group)
> {
> put_user_ns(group->user_ns);
> kfree(group->fanotify_data.merge_hash);
> + ida_destroy(&group->response_ida);
> if (group->fanotify_data.ucounts)
> dec_ucount(group->fanotify_data.ucounts,
> UCOUNT_FANOTIFY_GROUPS);
> @@ -1106,6 +1107,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))
> + ida_free(&group->response_ida, -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..99af23b257f9 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 = ida_alloc_min(&group->response_ida, 256, GFP_KERNEL);
> + if (ret < 0)
> + return ret;
> + fd = -ret;
> + } else if (!FAN_GROUP_FLAG(group, FANOTIFY_UNPRIV) && path &&
> + path->mnt && path->dentry) {
> + /*
> + * 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 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);
> @@ -1583,6 +1599,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
> (class | fid_mode) != FAN_CLASS_PRE_CONTENT_FID)
> 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;
> }
> + ida_init(&group->response_ida);
>
> 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..27299d5ca668 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 ida response_ida; /* 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 [flat|nested] 11+ messages in thread* Re: [PATCH v4 3/3] fanotify: introduce event response identifier
2025-07-11 18:31 ` [PATCH v4 3/3] fanotify: introduce event response identifier Ibrahim Jirdeh
2025-07-12 6:16 ` Amir Goldstein
@ 2025-07-12 20:51 ` kernel test robot
2025-07-16 6:25 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2025-07-12 20:51 UTC (permalink / raw)
To: Ibrahim Jirdeh
Cc: llvm, 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/20250712-023425
base: https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
patch link: https://lore.kernel.org/r/20250711183101.4074140-4-ibrahimjirdeh%40meta.com
patch subject: [PATCH v4 3/3] fanotify: introduce event response identifier
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20250713/202507130418.Prp26RtQ-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250713/202507130418.Prp26RtQ-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/202507130418.Prp26RtQ-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from fs/notify/fdinfo.c:17:
>> fs/notify/fanotify/fanotify.h:566:6: error: no member named 'fanotify_data' in 'struct fsnotify_group'
566 | if (FAN_GROUP_FLAG(group, FAN_REPORT_RESPONSE_ID))
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/fanotify.h:9:12: note: expanded from macro 'FAN_GROUP_FLAG'
9 | ((group)->fanotify_data.flags & (flag))
| ~~~~~~~ ^
1 error generated.
vim +566 fs/notify/fanotify/fanotify.h
562
563 static inline bool fanotify_is_valid_response_id(struct fsnotify_group *group,
564 int id)
565 {
> 566 if (FAN_GROUP_FLAG(group, FAN_REPORT_RESPONSE_ID))
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 3/3] fanotify: introduce event response identifier
2025-07-11 18:31 ` [PATCH v4 3/3] fanotify: introduce event response identifier Ibrahim Jirdeh
2025-07-12 6:16 ` Amir Goldstein
2025-07-12 20:51 ` kernel test robot
@ 2025-07-16 6:25 ` kernel test robot
2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2025-07-16 6:25 UTC (permalink / raw)
To: Ibrahim Jirdeh
Cc: oe-lkp, lkp, Amir Goldstein, linux-fsdevel, linux-kernel,
ibrahimjirdeh, jack, josef, lesha, sargun, oliver.sang
Hello,
kernel test robot noticed "INFO:trying_to_register_non-static_key" on:
commit: 16091e7d62ae910fee1e3bfc72943ec678e8df35 ("[PATCH v4 3/3] fanotify: introduce event response identifier")
url: https://github.com/intel-lab-lkp/linux/commits/Ibrahim-Jirdeh/fanotify-add-support-for-a-variable-length-permission-event/20250712-023425
base: https://git.kernel.org/cgit/linux/kernel/git/jack/linux-fs.git fsnotify
patch link: https://lore.kernel.org/all/20250711183101.4074140-4-ibrahimjirdeh@meta.com/
patch subject: [PATCH v4 3/3] fanotify: introduce event response identifier
in testcase: trinity
version:
with following parameters:
runtime: 300s
group: group-01
nr_groups: 5
config: x86_64-randconfig-006-20250712
compiler: gcc-12
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
(please refer to attached dmesg/kmsg for entire log/backtrace)
+----------------------------------------+------------+------------+
| | feb842cee4 | 16091e7d62 |
+----------------------------------------+------------+------------+
| INFO:trying_to_register_non-static_key | 0 | 11 |
+----------------------------------------+------------+------------+
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 <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202507160713.34ee11f5-lkp@intel.com
[ 570.145604][ T3660] INFO: trying to register non-static key.
[ 570.152934][ T3660] The code is fine but needs lockdep annotation, or maybe
[ 570.161254][ T3660] you didn't initialize this object before use?
[ 570.168674][ T3660] turning off the locking correctness validator.
[ 570.176388][ T3660] CPU: 0 UID: 0 PID: 3660 Comm: trinity-c3 Not tainted 6.16.0-rc3-00238-g16091e7d62ae #1 PREEMPT(lazy)
[ 570.189708][ T3660] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 570.202248][ T3660] Call Trace:
[ 570.206437][ T3660] <TASK>
[ 570.210125][ T3660] dump_stack_lvl (arch/x86/include/asm/irqflags.h:26 (discriminator 3) arch/x86/include/asm/irqflags.h:109 (discriminator 3) arch/x86/include/asm/irqflags.h:151 (discriminator 3) lib/dump_stack.c:123 (discriminator 3))
[ 570.215892][ T3660] dump_stack (lib/dump_stack.c:130)
[ 570.220985][ T3660] register_lock_class (kernel/locking/lockdep.c:988 kernel/locking/lockdep.c:1302)
[ 570.227173][ T3660] __lock_acquire (kernel/locking/lockdep.c:5116)
[ 570.232580][ T3660] lock_acquire (kernel/locking/lockdep.c:473 kernel/locking/lockdep.c:5873)
[ 570.237898][ T3660] ? ida_destroy (lib/idr.c:615)
[ 570.243703][ T3660] ? _raw_spin_lock_irqsave (include/linux/spinlock_api_smp.h:110 kernel/locking/spinlock.c:162)
[ 570.250176][ T3660] ? ida_destroy (lib/idr.c:615)
[ 570.255813][ T3660] ? trace_preempt_off (kernel/trace/trace_preemptirq.c:128)
[ 570.261457][ T3660] ? ida_destroy (lib/idr.c:615)
[ 570.266841][ T3660] _raw_spin_lock_irqsave (include/linux/spinlock_api_smp.h:111 kernel/locking/spinlock.c:162)
[ 570.272831][ T3660] ? ida_destroy (lib/idr.c:615)
[ 570.278373][ T3660] ida_destroy (lib/idr.c:615)
[ 570.283784][ T3660] ? xas_next_entry (include/trace/events/filemap.h:49)
[ 570.290885][ T3660] ? kfree (mm/slub.c:2312 mm/slub.c:4643 mm/slub.c:4842)
[ 570.295967][ T3660] fanotify_free_group_priv (fs/notify/fanotify/fanotify.c:1049)
[ 570.302446][ T3660] fsnotify_put_group (fs/notify/group.c:28 fs/notify/group.c:110)
[ 570.308643][ T3660] fsnotify_destroy_group (fs/notify/group.c:51)
[ 570.315249][ T3660] ? fsnotify_group_stop_queueing (fs/notify/group.c:51)
[ 570.322447][ T3660] ? lockdep_init_map_type (kernel/locking/lockdep.c:4976)
[ 570.329051][ T3660] ? lockdep_init_map_type (kernel/locking/lockdep.c:4976)
[ 570.335640][ T3660] ? __init_waitqueue_head (include/linux/list.h:37 kernel/sched/wait.c:12)
[ 570.342015][ T3660] __do_sys_fanotify_init (fs/notify/fanotify/fanotify_user.c:1718)
[ 570.348111][ T3660] ? __rcu_read_unlock (kernel/rcu/tree_plugin.h:445 (discriminator 3))
[ 570.353754][ T3660] __x64_sys_fanotify_init (fs/notify/fanotify/fanotify_user.c:1526)
[ 570.360325][ T3660] ? trace_hardirqs_on (kernel/trace/trace_preemptirq.c:80 (discriminator 22))
[ 570.366287][ T3660] x64_sys_call (arch/x86/entry/syscall_64.c:41)
[ 570.371831][ T3660] do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94)
[ 570.377436][ T3660] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
[ 570.384571][ T3660] RIP: 0033:0x463519
[ 570.389240][ T3660] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 db 59 00 00 c3 66 2e 0f 1f 84 00 00 00 00
All code
========
0: 00 f3 add %dh,%bl
2: c3 ret
3: 66 2e 0f 1f 84 00 00 cs nopw 0x0(%rax,%rax,1)
a: 00 00 00
d: 0f 1f 40 00 nopl 0x0(%rax)
11: 48 89 f8 mov %rdi,%rax
14: 48 89 f7 mov %rsi,%rdi
17: 48 89 d6 mov %rdx,%rsi
1a: 48 89 ca mov %rcx,%rdx
1d: 4d 89 c2 mov %r8,%r10
20: 4d 89 c8 mov %r9,%r8
23: 4c 8b 4c 24 08 mov 0x8(%rsp),%r9
28: 0f 05 syscall
2a:* 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax <-- trapping instruction
30: 0f 83 db 59 00 00 jae 0x5a11
36: c3 ret
37: 66 data16
38: 2e cs
39: 0f .byte 0xf
3a: 1f (bad)
3b: 84 00 test %al,(%rax)
3d: 00 00 add %al,(%rax)
...
Code starting with the faulting instruction
===========================================
0: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
6: 0f 83 db 59 00 00 jae 0x59e7
c: c3 ret
d: 66 data16
e: 2e cs
f: 0f .byte 0xf
10: 1f (bad)
11: 84 00 test %al,(%rax)
13: 00 00 add %al,(%rax)
...
[ 570.411823][ T3660] RSP: 002b:00007fff92c9e6d8 EFLAGS: 00000246 ORIG_RAX: 000000000000012c
[ 570.421741][ T3660] RAX: ffffffffffffffda RBX: 000000000000012c RCX: 0000000000463519
[ 570.430814][ T3660] RDX: 0000000004000055 RSI: 00000000001c1400 RDI: 000000000000002c
[ 570.440208][ T3660] RBP: 00007f4a06748000 R08: ffffffffffffffb7 R09: 0000000000000000
[ 570.449706][ T3660] R10: 0000000000002c85 R11: 0000000000000246 R12: 0000000000000002
[ 570.458904][ T3660] R13: 00007f4a06748058 R14: 0000000036007850 R15: 00007f4a06748000
[ 570.468210][ T3660] </TASK>
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250716/202507160713.34ee11f5-lkp@intel.com
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 11+ messages in thread