* [PATCH v5 0/3] mount notification
@ 2025-01-29 16:57 Miklos Szeredi
2025-01-29 16:57 ` [PATCH v5 1/3] fsnotify: add mount notification infrastructure Miklos Szeredi
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Miklos Szeredi @ 2025-01-29 16:57 UTC (permalink / raw)
To: linux-fsdevel
Cc: Christian Brauner, Jan Kara, Amir Goldstein, Karel Zak,
Lennart Poettering, Ian Kent, Alexander Viro, Paul Moore, selinux,
linux-security-module, selinux-refpolicy
This should be ready for adding to the v6.15 queue. I don't see the
SELinux discussion converging, so I took the simpler version out of the two
that were suggested.
Will work on adding selftests.
Thanks to everyone for the reviews!
Miklos
---
v5:
- drop FS_MNT_CHANGE (Christian)
- rebased on current mainline (Amir)
- add FSNOTIFY_MNT_EVENTS (Amir)
- change selinux permission check to FILE__WATCH_MOUNT (Paul)
v4:
- add notification on attribute change
- deal with two FIXMEs
- move data and code to #ifdef CONFIG_FSNOTIFY regions
- function renames for more consistentcy (Christian)
- explanation comment in umount_tree() (Christian)
- style cleanups in fanotify (Amir, Jan)
- changed FAN_MNT_* values (Amir)
v3:
- use a global list protected for temporarily storing (Christian)
- move fsnotify_* calls to namespace_unlock() (Christian)
- downgrade namespace_sem to read for fsnotify_* calls (Christian)
- add notification for reparenting in propagate_umount (Christian)
- require nsfs file (/proc/PID/ns/mnt) in fanotify_mark(2) (Christian)
- cleaner check for fsnotify being initialized (Amir)
- fix stub __fsnotify_mntns_delete (kernel test robot)
- don't add FANOTIFY_MOUNT_EVENTS to FANOTIFY_FD_EVENTS (Amir)
v2:
- notify for whole namespace as this seems to be what people prefer
- move fsnotify() calls outside of mount_lock
- only report mnt_id, not parent_id
Miklos Szeredi (3):
fsnotify: add mount notification infrastructure
fanotify: notify on mount attach and detach
vfs: add notifications for mount attach and detach
fs/mount.h | 26 +++++++++
fs/namespace.c | 93 ++++++++++++++++++++++++++++--
fs/notify/fanotify/fanotify.c | 38 +++++++++++-
fs/notify/fanotify/fanotify.h | 18 ++++++
fs/notify/fanotify/fanotify_user.c | 87 +++++++++++++++++++++++-----
fs/notify/fdinfo.c | 5 ++
fs/notify/fsnotify.c | 47 ++++++++++++---
fs/notify/fsnotify.h | 11 ++++
fs/notify/mark.c | 14 ++++-
fs/pnode.c | 4 +-
include/linux/fanotify.h | 12 ++--
include/linux/fsnotify.h | 20 +++++++
include/linux/fsnotify_backend.h | 42 ++++++++++++++
include/uapi/linux/fanotify.h | 10 ++++
security/selinux/hooks.c | 4 ++
15 files changed, 396 insertions(+), 35 deletions(-)
--
2.48.1
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v5 1/3] fsnotify: add mount notification infrastructure
2025-01-29 16:57 [PATCH v5 0/3] mount notification Miklos Szeredi
@ 2025-01-29 16:57 ` Miklos Szeredi
2025-02-11 13:05 ` Jan Kara
2025-01-29 16:58 ` [PATCH v5 2/3] fanotify: notify on mount attach and detach Miklos Szeredi
` (2 subsequent siblings)
3 siblings, 1 reply; 19+ messages in thread
From: Miklos Szeredi @ 2025-01-29 16:57 UTC (permalink / raw)
To: linux-fsdevel
Cc: Christian Brauner, Jan Kara, Amir Goldstein, Karel Zak,
Lennart Poettering, Ian Kent, Alexander Viro, Paul Moore, selinux,
linux-security-module, selinux-refpolicy
This is just the plumbing between the event source (fs/namespace.c) and the
event consumer (fanotify). In itself it does nothing.
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/mount.h | 4 +++
fs/notify/fsnotify.c | 47 +++++++++++++++++++++++++++-----
fs/notify/fsnotify.h | 11 ++++++++
fs/notify/mark.c | 14 ++++++++--
include/linux/fsnotify.h | 20 ++++++++++++++
include/linux/fsnotify_backend.h | 42 ++++++++++++++++++++++++++++
6 files changed, 128 insertions(+), 10 deletions(-)
diff --git a/fs/mount.h b/fs/mount.h
index ffb613cdfeee..82aa3bad7cf5 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -21,6 +21,10 @@ struct mnt_namespace {
struct rcu_head mnt_ns_rcu;
};
u64 event;
+#ifdef CONFIG_FSNOTIFY
+ __u32 n_fsnotify_mask;
+ struct fsnotify_mark_connector __rcu *n_fsnotify_marks;
+#endif
unsigned int nr_mounts; /* # of mounts in the namespace */
unsigned int pending_mounts;
struct rb_node mnt_ns_tree_node; /* node in the mnt_ns_tree */
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 8ee495a58d0a..c64b95cf50c7 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -28,6 +28,11 @@ void __fsnotify_vfsmount_delete(struct vfsmount *mnt)
fsnotify_clear_marks_by_mount(mnt);
}
+void __fsnotify_mntns_delete(struct mnt_namespace *mntns)
+{
+ fsnotify_clear_marks_by_mntns(mntns);
+}
+
/**
* fsnotify_unmount_inodes - an sb is unmounting. handle any watched inodes.
* @sb: superblock being unmounted.
@@ -420,7 +425,7 @@ static int send_to_group(__u32 mask, const void *data, int data_type,
file_name, cookie, iter_info);
}
-static struct fsnotify_mark *fsnotify_first_mark(struct fsnotify_mark_connector **connp)
+static struct fsnotify_mark *fsnotify_first_mark(struct fsnotify_mark_connector *const *connp)
{
struct fsnotify_mark_connector *conn;
struct hlist_node *node = NULL;
@@ -538,14 +543,15 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
{
const struct path *path = fsnotify_data_path(data, data_type);
struct super_block *sb = fsnotify_data_sb(data, data_type);
- struct fsnotify_sb_info *sbinfo = fsnotify_sb_info(sb);
+ const struct fsnotify_mnt *mnt_data = fsnotify_data_mnt(data, data_type);
+ struct fsnotify_sb_info *sbinfo = sb ? fsnotify_sb_info(sb) : NULL;
struct fsnotify_iter_info iter_info = {};
struct mount *mnt = NULL;
struct inode *inode2 = NULL;
struct dentry *moved;
int inode2_type;
int ret = 0;
- __u32 test_mask, marks_mask;
+ __u32 test_mask, marks_mask = 0;
if (path)
mnt = real_mount(path->mnt);
@@ -578,17 +584,20 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
if ((!sbinfo || !sbinfo->sb_marks) &&
(!mnt || !mnt->mnt_fsnotify_marks) &&
(!inode || !inode->i_fsnotify_marks) &&
- (!inode2 || !inode2->i_fsnotify_marks))
+ (!inode2 || !inode2->i_fsnotify_marks) &&
+ (!mnt_data || !mnt_data->ns->n_fsnotify_marks))
return 0;
- marks_mask = READ_ONCE(sb->s_fsnotify_mask);
+ if (sb)
+ marks_mask |= READ_ONCE(sb->s_fsnotify_mask);
if (mnt)
marks_mask |= READ_ONCE(mnt->mnt_fsnotify_mask);
if (inode)
marks_mask |= READ_ONCE(inode->i_fsnotify_mask);
if (inode2)
marks_mask |= READ_ONCE(inode2->i_fsnotify_mask);
-
+ if (mnt_data)
+ marks_mask |= READ_ONCE(mnt_data->ns->n_fsnotify_mask);
/*
* If this is a modify event we may need to clear some ignore masks.
@@ -618,6 +627,10 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
iter_info.marks[inode2_type] =
fsnotify_first_mark(&inode2->i_fsnotify_marks);
}
+ if (mnt_data) {
+ iter_info.marks[FSNOTIFY_ITER_TYPE_MNTNS] =
+ fsnotify_first_mark(&mnt_data->ns->n_fsnotify_marks);
+ }
/*
* We need to merge inode/vfsmount/sb mark lists so that e.g. inode mark
@@ -702,11 +715,31 @@ void file_set_fsnotify_mode(struct file *file)
}
#endif
+void fsnotify_mnt(__u32 mask, struct mnt_namespace *ns, struct vfsmount *mnt)
+{
+ struct fsnotify_mnt data = {
+ .ns = ns,
+ .mnt_id = real_mount(mnt)->mnt_id_unique,
+ };
+
+ if (WARN_ON_ONCE(!ns))
+ return;
+
+ /*
+ * This is an optimization as well as making sure fsnotify_init() has
+ * been called.
+ */
+ if (!ns->n_fsnotify_marks)
+ return;
+
+ fsnotify(mask, &data, FSNOTIFY_EVENT_MNT, NULL, NULL, NULL, 0);
+}
+
static __init int fsnotify_init(void)
{
int ret;
- BUILD_BUG_ON(HWEIGHT32(ALL_FSNOTIFY_BITS) != 24);
+ BUILD_BUG_ON(HWEIGHT32(ALL_FSNOTIFY_BITS) != 26);
ret = init_srcu_struct(&fsnotify_mark_srcu);
if (ret)
diff --git a/fs/notify/fsnotify.h b/fs/notify/fsnotify.h
index 663759ed6fbc..5950c7a67f41 100644
--- a/fs/notify/fsnotify.h
+++ b/fs/notify/fsnotify.h
@@ -33,6 +33,12 @@ static inline struct super_block *fsnotify_conn_sb(
return conn->obj;
}
+static inline struct mnt_namespace *fsnotify_conn_mntns(
+ struct fsnotify_mark_connector *conn)
+{
+ return conn->obj;
+}
+
static inline struct super_block *fsnotify_object_sb(void *obj,
enum fsnotify_obj_type obj_type)
{
@@ -89,6 +95,11 @@ static inline void fsnotify_clear_marks_by_sb(struct super_block *sb)
fsnotify_destroy_marks(fsnotify_sb_marks(sb));
}
+static inline void fsnotify_clear_marks_by_mntns(struct mnt_namespace *mntns)
+{
+ fsnotify_destroy_marks(&mntns->n_fsnotify_marks);
+}
+
/*
* update the dentry->d_flags of all of inode's children to indicate if inode cares
* about events that happen to its children.
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index 4981439e6209..798340db69d7 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -107,6 +107,8 @@ static fsnotify_connp_t *fsnotify_object_connp(void *obj,
return &real_mount(obj)->mnt_fsnotify_marks;
case FSNOTIFY_OBJ_TYPE_SB:
return fsnotify_sb_marks(obj);
+ case FSNOTIFY_OBJ_TYPE_MNTNS:
+ return &((struct mnt_namespace *)obj)->n_fsnotify_marks;
default:
return NULL;
}
@@ -120,6 +122,8 @@ static __u32 *fsnotify_conn_mask_p(struct fsnotify_mark_connector *conn)
return &fsnotify_conn_mount(conn)->mnt_fsnotify_mask;
else if (conn->type == FSNOTIFY_OBJ_TYPE_SB)
return &fsnotify_conn_sb(conn)->s_fsnotify_mask;
+ else if (conn->type == FSNOTIFY_OBJ_TYPE_MNTNS)
+ return &fsnotify_conn_mntns(conn)->n_fsnotify_mask;
return NULL;
}
@@ -346,12 +350,15 @@ static void *fsnotify_detach_connector_from_object(
fsnotify_conn_mount(conn)->mnt_fsnotify_mask = 0;
} else if (conn->type == FSNOTIFY_OBJ_TYPE_SB) {
fsnotify_conn_sb(conn)->s_fsnotify_mask = 0;
+ } else if (conn->type == FSNOTIFY_OBJ_TYPE_MNTNS) {
+ fsnotify_conn_mntns(conn)->n_fsnotify_mask = 0;
}
rcu_assign_pointer(*connp, NULL);
conn->obj = NULL;
conn->type = FSNOTIFY_OBJ_TYPE_DETACHED;
- fsnotify_update_sb_watchers(sb, conn);
+ if (sb)
+ fsnotify_update_sb_watchers(sb, conn);
return inode;
}
@@ -724,7 +731,7 @@ static int fsnotify_add_mark_list(struct fsnotify_mark *mark, void *obj,
* Attach the sb info before attaching a connector to any object on sb.
* The sb info will remain attached as long as sb lives.
*/
- if (!fsnotify_sb_info(sb)) {
+ if (sb && !fsnotify_sb_info(sb)) {
err = fsnotify_attach_info_to_sb(sb);
if (err)
return err;
@@ -770,7 +777,8 @@ static int fsnotify_add_mark_list(struct fsnotify_mark *mark, void *obj,
/* mark should be the last entry. last is the current last entry */
hlist_add_behind_rcu(&mark->obj_list, &last->obj_list);
added:
- fsnotify_update_sb_watchers(sb, conn);
+ if (sb)
+ fsnotify_update_sb_watchers(sb, conn);
/*
* Since connector is attached to object using cmpxchg() we are
* guaranteed that connector initialization is fully visible by anyone
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 1a9ef8f6784d..589e274adc7d 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -299,6 +299,11 @@ static inline void fsnotify_vfsmount_delete(struct vfsmount *mnt)
__fsnotify_vfsmount_delete(mnt);
}
+static inline void fsnotify_mntns_delete(struct mnt_namespace *mntns)
+{
+ __fsnotify_mntns_delete(mntns);
+}
+
/*
* fsnotify_inoderemove - an inode is going away
*/
@@ -507,4 +512,19 @@ static inline int fsnotify_sb_error(struct super_block *sb, struct inode *inode,
NULL, NULL, NULL, 0);
}
+static inline void fsnotify_mnt_attach(struct mnt_namespace *ns, struct vfsmount *mnt)
+{
+ fsnotify_mnt(FS_MNT_ATTACH, ns, mnt);
+}
+
+static inline void fsnotify_mnt_detach(struct mnt_namespace *ns, struct vfsmount *mnt)
+{
+ fsnotify_mnt(FS_MNT_DETACH, ns, mnt);
+}
+
+static inline void fsnotify_mnt_move(struct mnt_namespace *ns, struct vfsmount *mnt)
+{
+ fsnotify_mnt(FS_MNT_MOVE, ns, mnt);
+}
+
#endif /* _LINUX_FS_NOTIFY_H */
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 0d24a21a8e60..6cd8d1d28b8b 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -59,6 +59,10 @@
#define FS_PRE_ACCESS 0x00100000 /* Pre-content access hook */
+#define FS_MNT_ATTACH 0x01000000 /* Mount was attached */
+#define FS_MNT_DETACH 0x02000000 /* Mount was detached */
+#define FS_MNT_MOVE (FS_MNT_ATTACH | FS_MNT_DETACH)
+
/*
* Set on inode mark that cares about things that happen to its children.
* Always set for dnotify and inotify.
@@ -80,6 +84,9 @@
*/
#define ALL_FSNOTIFY_DIRENT_EVENTS (FS_CREATE | FS_DELETE | FS_MOVE | FS_RENAME)
+/* Mount namespace events */
+#define FSNOTIFY_MNT_EVENTS (FS_MNT_ATTACH | FS_MNT_DETACH)
+
/* Content events can be used to inspect file content */
#define FSNOTIFY_CONTENT_PERM_EVENTS (FS_OPEN_PERM | FS_OPEN_EXEC_PERM | \
FS_ACCESS_PERM)
@@ -108,6 +115,7 @@
/* Events that can be reported to backends */
#define ALL_FSNOTIFY_EVENTS (ALL_FSNOTIFY_DIRENT_EVENTS | \
+ FSNOTIFY_MNT_EVENTS | \
FS_EVENTS_POSS_ON_CHILD | \
FS_DELETE_SELF | FS_MOVE_SELF | \
FS_UNMOUNT | FS_Q_OVERFLOW | FS_IN_IGNORED | \
@@ -298,6 +306,7 @@ enum fsnotify_data_type {
FSNOTIFY_EVENT_PATH,
FSNOTIFY_EVENT_INODE,
FSNOTIFY_EVENT_DENTRY,
+ FSNOTIFY_EVENT_MNT,
FSNOTIFY_EVENT_ERROR,
};
@@ -318,6 +327,11 @@ static inline const struct path *file_range_path(const struct file_range *range)
return range->path;
}
+struct fsnotify_mnt {
+ const struct mnt_namespace *ns;
+ u64 mnt_id;
+};
+
static inline struct inode *fsnotify_data_inode(const void *data, int data_type)
{
switch (data_type) {
@@ -383,6 +397,24 @@ static inline struct super_block *fsnotify_data_sb(const void *data,
}
}
+static inline const struct fsnotify_mnt *fsnotify_data_mnt(const void *data,
+ int data_type)
+{
+ switch (data_type) {
+ case FSNOTIFY_EVENT_MNT:
+ return data;
+ default:
+ return NULL;
+ }
+}
+
+static inline u64 fsnotify_data_mnt_id(const void *data, int data_type)
+{
+ const struct fsnotify_mnt *mnt_data = fsnotify_data_mnt(data, data_type);
+
+ return mnt_data ? mnt_data->mnt_id : 0;
+}
+
static inline struct fs_error_report *fsnotify_data_error_report(
const void *data,
int data_type)
@@ -420,6 +452,7 @@ enum fsnotify_iter_type {
FSNOTIFY_ITER_TYPE_SB,
FSNOTIFY_ITER_TYPE_PARENT,
FSNOTIFY_ITER_TYPE_INODE2,
+ FSNOTIFY_ITER_TYPE_MNTNS,
FSNOTIFY_ITER_TYPE_COUNT
};
@@ -429,6 +462,7 @@ enum fsnotify_obj_type {
FSNOTIFY_OBJ_TYPE_INODE,
FSNOTIFY_OBJ_TYPE_VFSMOUNT,
FSNOTIFY_OBJ_TYPE_SB,
+ FSNOTIFY_OBJ_TYPE_MNTNS,
FSNOTIFY_OBJ_TYPE_COUNT,
FSNOTIFY_OBJ_TYPE_DETACHED = FSNOTIFY_OBJ_TYPE_COUNT
};
@@ -613,8 +647,10 @@ extern int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data
extern void __fsnotify_inode_delete(struct inode *inode);
extern void __fsnotify_vfsmount_delete(struct vfsmount *mnt);
extern void fsnotify_sb_delete(struct super_block *sb);
+extern void __fsnotify_mntns_delete(struct mnt_namespace *mntns);
extern void fsnotify_sb_free(struct super_block *sb);
extern u32 fsnotify_get_cookie(void);
+extern void fsnotify_mnt(__u32 mask, struct mnt_namespace *ns, struct vfsmount *mnt);
static inline __u32 fsnotify_parent_needed_mask(__u32 mask)
{
@@ -928,6 +964,9 @@ static inline void __fsnotify_vfsmount_delete(struct vfsmount *mnt)
static inline void fsnotify_sb_delete(struct super_block *sb)
{}
+static inline void __fsnotify_mntns_delete(struct mnt_namespace *mntns)
+{}
+
static inline void fsnotify_sb_free(struct super_block *sb)
{}
@@ -942,6 +981,9 @@ static inline u32 fsnotify_get_cookie(void)
static inline void fsnotify_unmount_inodes(struct super_block *sb)
{}
+static inline void fsnotify_mnt(__u32 mask, struct mnt_namespace *ns, struct vfsmount *mnt)
+{}
+
#endif /* CONFIG_FSNOTIFY */
#endif /* __KERNEL __ */
--
2.48.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-01-29 16:57 [PATCH v5 0/3] mount notification Miklos Szeredi
2025-01-29 16:57 ` [PATCH v5 1/3] fsnotify: add mount notification infrastructure Miklos Szeredi
@ 2025-01-29 16:58 ` Miklos Szeredi
2025-01-30 21:05 ` Paul Moore
2025-02-11 13:32 ` Jan Kara
2025-01-29 16:58 ` [PATCH v5 3/3] vfs: add notifications for " Miklos Szeredi
2025-01-30 16:07 ` [PATCH v5 0/3] mount notification Christian Brauner
3 siblings, 2 replies; 19+ messages in thread
From: Miklos Szeredi @ 2025-01-29 16:58 UTC (permalink / raw)
To: linux-fsdevel
Cc: Christian Brauner, Jan Kara, Amir Goldstein, Karel Zak,
Lennart Poettering, Ian Kent, Alexander Viro, Paul Moore, selinux,
linux-security-module, selinux-refpolicy
Add notifications for attaching and detaching mounts. The following new
event masks are added:
FAN_MNT_ATTACH - Mount was attached
FAN_MNT_DETACH - Mount was detached
If a mount is moved, then the event is reported with (FAN_MNT_ATTACH |
FAN_MNT_DETACH).
These events add an info record of type FAN_EVENT_INFO_TYPE_MNT containing
these fields identifying the affected mounts:
__u64 mnt_id - the ID of the mount (see statmount(2))
FAN_REPORT_MNT must be supplied to fanotify_init() to receive these events
and no other type of event can be received with this report type.
Marks are added with FAN_MARK_MNTNS, which records the mount namespace from
an nsfs file (e.g. /proc/self/ns/mnt).
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/mount.h | 2 +
fs/namespace.c | 14 +++--
fs/notify/fanotify/fanotify.c | 38 +++++++++++--
fs/notify/fanotify/fanotify.h | 18 +++++++
fs/notify/fanotify/fanotify_user.c | 87 +++++++++++++++++++++++++-----
fs/notify/fdinfo.c | 5 ++
include/linux/fanotify.h | 12 +++--
include/uapi/linux/fanotify.h | 10 ++++
security/selinux/hooks.c | 4 ++
9 files changed, 167 insertions(+), 23 deletions(-)
diff --git a/fs/mount.h b/fs/mount.h
index 82aa3bad7cf5..5324a931b403 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -181,3 +181,5 @@ static inline struct mnt_namespace *to_mnt_ns(struct ns_common *ns)
{
return container_of(ns, struct mnt_namespace, ns);
}
+
+struct mnt_namespace *mnt_ns_from_dentry(struct dentry *dentry);
diff --git a/fs/namespace.c b/fs/namespace.c
index 4013fbac354a..d8d70da56e7b 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2145,16 +2145,24 @@ struct mnt_namespace *get_sequential_mnt_ns(struct mnt_namespace *mntns, bool pr
}
}
+struct mnt_namespace *mnt_ns_from_dentry(struct dentry *dentry)
+{
+ if (!is_mnt_ns_file(dentry))
+ return NULL;
+
+ return to_mnt_ns(get_proc_ns(dentry->d_inode));
+}
+
static bool mnt_ns_loop(struct dentry *dentry)
{
/* Could bind mounting the mount namespace inode cause a
* mount namespace loop?
*/
- struct mnt_namespace *mnt_ns;
- if (!is_mnt_ns_file(dentry))
+ struct mnt_namespace *mnt_ns = mnt_ns_from_dentry(dentry);
+
+ if (!mnt_ns)
return false;
- mnt_ns = to_mnt_ns(get_proc_ns(dentry->d_inode));
return current->nsproxy->mnt_ns->seq >= mnt_ns->seq;
}
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 95646f7c46ca..6d386080faf2 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -166,6 +166,8 @@ static bool fanotify_should_merge(struct fanotify_event *old,
case FANOTIFY_EVENT_TYPE_FS_ERROR:
return fanotify_error_event_equal(FANOTIFY_EE(old),
FANOTIFY_EE(new));
+ case FANOTIFY_EVENT_TYPE_MNT:
+ return false;
default:
WARN_ON_ONCE(1);
}
@@ -312,7 +314,10 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
pr_debug("%s: report_mask=%x mask=%x data=%p data_type=%d\n",
__func__, iter_info->report_mask, event_mask, data, data_type);
- if (!fid_mode) {
+ if (FAN_GROUP_FLAG(group, FAN_REPORT_MNT)) {
+ if (data_type != FSNOTIFY_EVENT_MNT)
+ return 0;
+ } else if (!fid_mode) {
/* Do we have path to open a file descriptor? */
if (!path)
return 0;
@@ -557,6 +562,20 @@ static struct fanotify_event *fanotify_alloc_path_event(const struct path *path,
return &pevent->fae;
}
+static struct fanotify_event *fanotify_alloc_mnt_event(u64 mnt_id, gfp_t gfp)
+{
+ struct fanotify_mnt_event *pevent;
+
+ pevent = kmem_cache_alloc(fanotify_mnt_event_cachep, gfp);
+ if (!pevent)
+ return NULL;
+
+ pevent->fae.type = FANOTIFY_EVENT_TYPE_MNT;
+ pevent->mnt_id = mnt_id;
+
+ return &pevent->fae;
+}
+
static struct fanotify_event *fanotify_alloc_perm_event(const void *data,
int data_type,
gfp_t gfp)
@@ -731,6 +750,7 @@ static struct fanotify_event *fanotify_alloc_event(
fid_mode);
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);
struct mem_cgroup *old_memcg;
struct dentry *moved = NULL;
struct inode *child = NULL;
@@ -826,8 +846,12 @@ static struct fanotify_event *fanotify_alloc_event(
moved, &hash, gfp);
} else if (fid_mode) {
event = fanotify_alloc_fid_event(id, fsid, &hash, gfp);
- } else {
+ } else if (path) {
event = fanotify_alloc_path_event(path, &hash, gfp);
+ } else if (mnt_id) {
+ event = fanotify_alloc_mnt_event(mnt_id, gfp);
+ } else {
+ WARN_ON_ONCE(1);
}
if (!event)
@@ -927,7 +951,7 @@ static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
BUILD_BUG_ON(FAN_RENAME != FS_RENAME);
BUILD_BUG_ON(FAN_PRE_ACCESS != FS_PRE_ACCESS);
- BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 22);
+ BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 24);
mask = fanotify_group_event_mask(group, iter_info, &match_mask,
mask, data, data_type, dir);
@@ -1028,6 +1052,11 @@ static void fanotify_free_error_event(struct fsnotify_group *group,
mempool_free(fee, &group->fanotify_data.error_events_pool);
}
+static void fanotify_free_mnt_event(struct fanotify_event *event)
+{
+ kmem_cache_free(fanotify_mnt_event_cachep, FANOTIFY_ME(event));
+}
+
static void fanotify_free_event(struct fsnotify_group *group,
struct fsnotify_event *fsn_event)
{
@@ -1054,6 +1083,9 @@ static void fanotify_free_event(struct fsnotify_group *group,
case FANOTIFY_EVENT_TYPE_FS_ERROR:
fanotify_free_error_event(group, event);
break;
+ case FANOTIFY_EVENT_TYPE_MNT:
+ fanotify_free_mnt_event(event);
+ break;
default:
WARN_ON_ONCE(1);
}
diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
index c12cbc270539..b44e70e44be6 100644
--- a/fs/notify/fanotify/fanotify.h
+++ b/fs/notify/fanotify/fanotify.h
@@ -9,6 +9,7 @@ extern struct kmem_cache *fanotify_mark_cache;
extern struct kmem_cache *fanotify_fid_event_cachep;
extern struct kmem_cache *fanotify_path_event_cachep;
extern struct kmem_cache *fanotify_perm_event_cachep;
+extern struct kmem_cache *fanotify_mnt_event_cachep;
/* Possible states of the permission event */
enum {
@@ -244,6 +245,7 @@ enum fanotify_event_type {
FANOTIFY_EVENT_TYPE_PATH_PERM,
FANOTIFY_EVENT_TYPE_OVERFLOW, /* struct fanotify_event */
FANOTIFY_EVENT_TYPE_FS_ERROR, /* struct fanotify_error_event */
+ FANOTIFY_EVENT_TYPE_MNT,
__FANOTIFY_EVENT_TYPE_NUM
};
@@ -409,12 +411,23 @@ struct fanotify_path_event {
struct path path;
};
+struct fanotify_mnt_event {
+ struct fanotify_event fae;
+ u64 mnt_id;
+};
+
static inline struct fanotify_path_event *
FANOTIFY_PE(struct fanotify_event *event)
{
return container_of(event, struct fanotify_path_event, fae);
}
+static inline struct fanotify_mnt_event *
+FANOTIFY_ME(struct fanotify_event *event)
+{
+ return container_of(event, struct fanotify_mnt_event, fae);
+}
+
/*
* Structure for permission fanotify events. It gets allocated and freed in
* fanotify_handle_event() since we wait there for user response. When the
@@ -466,6 +479,11 @@ static inline bool fanotify_is_error_event(u32 mask)
return mask & FAN_FS_ERROR;
}
+static inline bool fanotify_is_mnt_event(u32 mask)
+{
+ return mask & (FAN_MNT_ATTACH | FAN_MNT_DETACH);
+}
+
static inline const struct path *fanotify_event_path(struct fanotify_event *event)
{
if (event->type == FANOTIFY_EVENT_TYPE_PATH)
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 6ff94e312232..d0fe7d7880a6 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -113,6 +113,7 @@ struct kmem_cache *fanotify_mark_cache __ro_after_init;
struct kmem_cache *fanotify_fid_event_cachep __ro_after_init;
struct kmem_cache *fanotify_path_event_cachep __ro_after_init;
struct kmem_cache *fanotify_perm_event_cachep __ro_after_init;
+struct kmem_cache *fanotify_mnt_event_cachep __ro_after_init;
#define FANOTIFY_EVENT_ALIGN 4
#define FANOTIFY_FID_INFO_HDR_LEN \
@@ -123,6 +124,8 @@ struct kmem_cache *fanotify_perm_event_cachep __ro_after_init;
(sizeof(struct fanotify_event_info_error))
#define FANOTIFY_RANGE_INFO_LEN \
(sizeof(struct fanotify_event_info_range))
+#define FANOTIFY_MNT_INFO_LEN \
+ (sizeof(struct fanotify_event_info_mnt))
static int fanotify_fid_info_len(int fh_len, int name_len)
{
@@ -178,6 +181,8 @@ static size_t fanotify_event_len(unsigned int info_mode,
fh_len = fanotify_event_object_fh_len(event);
event_len += fanotify_fid_info_len(fh_len, dot_len);
}
+ if (fanotify_is_mnt_event(event->mask))
+ event_len += FANOTIFY_MNT_INFO_LEN;
if (info_mode & FAN_REPORT_PIDFD)
event_len += FANOTIFY_PIDFD_INFO_LEN;
@@ -405,6 +410,25 @@ static int process_access_response(struct fsnotify_group *group,
return -ENOENT;
}
+static size_t copy_mnt_info_to_user(struct fanotify_event *event,
+ char __user *buf, int count)
+{
+ struct fanotify_event_info_mnt info = { };
+
+ info.hdr.info_type = FAN_EVENT_INFO_TYPE_MNT;
+ info.hdr.len = FANOTIFY_MNT_INFO_LEN;
+
+ if (WARN_ON(count < info.hdr.len))
+ return -EFAULT;
+
+ info.mnt_id = FANOTIFY_ME(event)->mnt_id;
+
+ if (copy_to_user(buf, &info, sizeof(info)))
+ return -EFAULT;
+
+ return info.hdr.len;
+}
+
static size_t copy_error_info_to_user(struct fanotify_event *event,
char __user *buf, int count)
{
@@ -700,6 +724,15 @@ static int copy_info_records_to_user(struct fanotify_event *event,
total_bytes += ret;
}
+ if (fanotify_is_mnt_event(event->mask)) {
+ ret = copy_mnt_info_to_user(event, buf, count);
+ if (ret < 0)
+ return ret;
+ buf += ret;
+ count -= ret;
+ total_bytes += ret;
+ }
+
return total_bytes;
}
@@ -1508,6 +1541,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
if ((flags & FAN_REPORT_PIDFD) && (flags & FAN_REPORT_TID))
return -EINVAL;
+ /* Don't allow mixing mnt events with inode events for now */
+ if (flags & FAN_REPORT_MNT) {
+ if (class != FAN_CLASS_NOTIF)
+ return -EINVAL;
+ if (flags & (FANOTIFY_FID_BITS | FAN_REPORT_FD_ERROR))
+ return -EINVAL;
+ }
+
if (event_f_flags & ~FANOTIFY_INIT_ALL_EVENT_F_BITS)
return -EINVAL;
@@ -1767,7 +1808,6 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
int dfd, const char __user *pathname)
{
struct inode *inode = NULL;
- struct vfsmount *mnt = NULL;
struct fsnotify_group *group;
struct path path;
struct fan_fsid __fsid, *fsid = NULL;
@@ -1800,6 +1840,9 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
case FAN_MARK_FILESYSTEM:
obj_type = FSNOTIFY_OBJ_TYPE_SB;
break;
+ case FAN_MARK_MNTNS:
+ obj_type = FSNOTIFY_OBJ_TYPE_MNTNS;
+ break;
default:
return -EINVAL;
}
@@ -1847,6 +1890,19 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
return -EINVAL;
group = fd_file(f)->private_data;
+ /* Only report mount events on mnt namespace */
+ if (FAN_GROUP_FLAG(group, FAN_REPORT_MNT)) {
+ if (mask & ~FANOTIFY_MOUNT_EVENTS)
+ return -EINVAL;
+ if (mark_type != FAN_MARK_MNTNS)
+ return -EINVAL;
+ } else {
+ if (mask & FANOTIFY_MOUNT_EVENTS)
+ return -EINVAL;
+ if (mark_type == FAN_MARK_MNTNS)
+ return -EINVAL;
+ }
+
/*
* An unprivileged user is not allowed to setup mount nor filesystem
* marks. This also includes setting up such marks by a group that
@@ -1888,7 +1944,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
* point.
*/
fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
- if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_EVENT_FLAGS) &&
+ if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_MOUNT_EVENTS|FANOTIFY_EVENT_FLAGS) &&
(!fid_mode || mark_type == FAN_MARK_MOUNT))
return -EINVAL;
@@ -1938,17 +1994,21 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
}
/* inode held in place by reference to path; group by fget on fd */
- if (mark_type == FAN_MARK_INODE) {
+ if (obj_type == FSNOTIFY_OBJ_TYPE_INODE) {
inode = path.dentry->d_inode;
obj = inode;
- } else {
- mnt = path.mnt;
- if (mark_type == FAN_MARK_MOUNT)
- obj = mnt;
- else
- obj = mnt->mnt_sb;
+ } else if (obj_type == FSNOTIFY_OBJ_TYPE_VFSMOUNT) {
+ obj = path.mnt;
+ } else if (obj_type == FSNOTIFY_OBJ_TYPE_SB) {
+ obj = path.mnt->mnt_sb;
+ } else if (obj_type == FSNOTIFY_OBJ_TYPE_MNTNS) {
+ obj = mnt_ns_from_dentry(path.dentry);
}
+ ret = -EINVAL;
+ if (!obj)
+ goto path_put_and_out;
+
/*
* If some other task has this inode open for write we should not add
* an ignore mask, unless that ignore mask is supposed to survive
@@ -1956,10 +2016,10 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
*/
if (mark_cmd == FAN_MARK_ADD && (flags & FANOTIFY_MARK_IGNORE_BITS) &&
!(flags & FAN_MARK_IGNORED_SURV_MODIFY)) {
- ret = mnt ? -EINVAL : -EISDIR;
+ ret = !inode ? -EINVAL : -EISDIR;
/* FAN_MARK_IGNORE requires SURV_MODIFY for sb/mount/dir marks */
if (ignore == FAN_MARK_IGNORE &&
- (mnt || S_ISDIR(inode->i_mode)))
+ (!inode || S_ISDIR(inode->i_mode)))
goto path_put_and_out;
ret = 0;
@@ -1968,7 +2028,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
}
/* Mask out FAN_EVENT_ON_CHILD flag for sb/mount/non-dir marks */
- if (mnt || !S_ISDIR(inode->i_mode)) {
+ if (!inode || !S_ISDIR(inode->i_mode)) {
mask &= ~FAN_EVENT_ON_CHILD;
umask = FAN_EVENT_ON_CHILD;
/*
@@ -2042,7 +2102,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) != 13);
+ BUILD_BUG_ON(HWEIGHT32(FANOTIFY_INIT_FLAGS) != 14);
BUILD_BUG_ON(HWEIGHT32(FANOTIFY_MARK_FLAGS) != 11);
fanotify_mark_cache = KMEM_CACHE(fanotify_mark,
@@ -2055,6 +2115,7 @@ static int __init fanotify_user_setup(void)
fanotify_perm_event_cachep =
KMEM_CACHE(fanotify_perm_event, SLAB_PANIC);
}
+ fanotify_mnt_event_cachep = KMEM_CACHE(fanotify_mnt_event, SLAB_PANIC);
fanotify_max_queued_events = FANOTIFY_DEFAULT_MAX_EVENTS;
init_user_ns.ucount_max[UCOUNT_FANOTIFY_GROUPS] =
diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
index e933f9c65d90..1161eabf11ee 100644
--- a/fs/notify/fdinfo.c
+++ b/fs/notify/fdinfo.c
@@ -121,6 +121,11 @@ static void fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
seq_printf(m, "fanotify sdev:%x mflags:%x mask:%x ignored_mask:%x\n",
sb->s_dev, mflags, mark->mask, mark->ignore_mask);
+ } else if (mark->connector->type == FSNOTIFY_OBJ_TYPE_MNTNS) {
+ struct mnt_namespace *mnt_ns = fsnotify_conn_mntns(mark->connector);
+
+ seq_printf(m, "fanotify mnt_ns:%u mflags:%x mask:%x ignored_mask:%x\n",
+ mnt_ns->ns.inum, mflags, mark->mask, mark->ignore_mask);
}
}
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index 78f660ebc318..3c817dc6292e 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -25,7 +25,7 @@
#define FANOTIFY_FID_BITS (FAN_REPORT_DFID_NAME_TARGET)
-#define FANOTIFY_INFO_MODES (FANOTIFY_FID_BITS | FAN_REPORT_PIDFD)
+#define FANOTIFY_INFO_MODES (FANOTIFY_FID_BITS | FAN_REPORT_PIDFD | FAN_REPORT_MNT)
/*
* fanotify_init() flags that require CAP_SYS_ADMIN.
@@ -38,7 +38,8 @@
FAN_REPORT_PIDFD | \
FAN_REPORT_FD_ERROR | \
FAN_UNLIMITED_QUEUE | \
- FAN_UNLIMITED_MARKS)
+ FAN_UNLIMITED_MARKS | \
+ FAN_REPORT_MNT)
/*
* fanotify_init() flags that are allowed for user without CAP_SYS_ADMIN.
@@ -58,7 +59,7 @@
#define FANOTIFY_INTERNAL_GROUP_FLAGS (FANOTIFY_UNPRIV)
#define FANOTIFY_MARK_TYPE_BITS (FAN_MARK_INODE | FAN_MARK_MOUNT | \
- FAN_MARK_FILESYSTEM)
+ FAN_MARK_FILESYSTEM | FAN_MARK_MNTNS)
#define FANOTIFY_MARK_CMD_BITS (FAN_MARK_ADD | FAN_MARK_REMOVE | \
FAN_MARK_FLUSH)
@@ -109,10 +110,13 @@
/* Events that can only be reported with data type FSNOTIFY_EVENT_ERROR */
#define FANOTIFY_ERROR_EVENTS (FAN_FS_ERROR)
+#define FANOTIFY_MOUNT_EVENTS (FAN_MNT_ATTACH | FAN_MNT_DETACH)
+
/* Events that user can request to be notified on */
#define FANOTIFY_EVENTS (FANOTIFY_PATH_EVENTS | \
FANOTIFY_INODE_EVENTS | \
- FANOTIFY_ERROR_EVENTS)
+ FANOTIFY_ERROR_EVENTS | \
+ FANOTIFY_MOUNT_EVENTS)
/* Extra flags that may be reported with event or control handling of events */
#define FANOTIFY_EVENT_FLAGS (FAN_EVENT_ON_CHILD | FAN_ONDIR)
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index bd8167979707..e710967c7c26 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -28,6 +28,8 @@
/* #define FAN_DIR_MODIFY 0x00080000 */ /* Deprecated (reserved) */
#define FAN_PRE_ACCESS 0x00100000 /* Pre-content access hook */
+#define FAN_MNT_ATTACH 0x01000000 /* Mount was attached */
+#define FAN_MNT_DETACH 0x02000000 /* Mount was detached */
#define FAN_EVENT_ON_CHILD 0x08000000 /* Interested in child events */
@@ -64,6 +66,7 @@
#define FAN_REPORT_NAME 0x00000800 /* Report events with name */
#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 */
/* Convenience macro - FAN_REPORT_NAME requires FAN_REPORT_DIR_FID */
#define FAN_REPORT_DFID_NAME (FAN_REPORT_DIR_FID | FAN_REPORT_NAME)
@@ -94,6 +97,7 @@
#define FAN_MARK_INODE 0x00000000
#define FAN_MARK_MOUNT 0x00000010
#define FAN_MARK_FILESYSTEM 0x00000100
+#define FAN_MARK_MNTNS 0x00000110
/*
* Convenience macro - FAN_MARK_IGNORE requires FAN_MARK_IGNORED_SURV_MODIFY
@@ -147,6 +151,7 @@ struct fanotify_event_metadata {
#define FAN_EVENT_INFO_TYPE_PIDFD 4
#define FAN_EVENT_INFO_TYPE_ERROR 5
#define FAN_EVENT_INFO_TYPE_RANGE 6
+#define FAN_EVENT_INFO_TYPE_MNT 7
/* Special info types for FAN_RENAME */
#define FAN_EVENT_INFO_TYPE_OLD_DFID_NAME 10
@@ -200,6 +205,11 @@ struct fanotify_event_info_range {
__u64 count;
};
+struct fanotify_event_info_mnt {
+ struct fanotify_event_info_header hdr;
+ __u64 mnt_id;
+};
+
/*
* User space may need to record additional information about its decision.
* The extra information type records what kind of information is included.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 7b867dfec88b..06d073eab53c 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3395,6 +3395,10 @@ static int selinux_path_notify(const struct path *path, u64 mask,
case FSNOTIFY_OBJ_TYPE_INODE:
perm = FILE__WATCH;
break;
+ case FSNOTIFY_OBJ_TYPE_MNTNS:
+ /* Maybe introduce FILE__WATCH_MOUNTNS? */
+ perm = FILE__WATCH_MOUNT;
+ break;
default:
return -EINVAL;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v5 3/3] vfs: add notifications for mount attach and detach
2025-01-29 16:57 [PATCH v5 0/3] mount notification Miklos Szeredi
2025-01-29 16:57 ` [PATCH v5 1/3] fsnotify: add mount notification infrastructure Miklos Szeredi
2025-01-29 16:58 ` [PATCH v5 2/3] fanotify: notify on mount attach and detach Miklos Szeredi
@ 2025-01-29 16:58 ` Miklos Szeredi
2025-02-11 13:04 ` Jan Kara
2025-01-30 16:07 ` [PATCH v5 0/3] mount notification Christian Brauner
3 siblings, 1 reply; 19+ messages in thread
From: Miklos Szeredi @ 2025-01-29 16:58 UTC (permalink / raw)
To: linux-fsdevel
Cc: Christian Brauner, Jan Kara, Amir Goldstein, Karel Zak,
Lennart Poettering, Ian Kent, Alexander Viro, Paul Moore, selinux,
linux-security-module, selinux-refpolicy
Add notifications for attaching and detaching mounts to fs/namespace.c
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
fs/mount.h | 20 +++++++++++++
fs/namespace.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++-
fs/pnode.c | 4 ++-
3 files changed, 101 insertions(+), 2 deletions(-)
diff --git a/fs/mount.h b/fs/mount.h
index 5324a931b403..946dc8b792d7 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -5,6 +5,8 @@
#include <linux/ns_common.h>
#include <linux/fs_pin.h>
+extern struct list_head notify_list;
+
struct mnt_namespace {
struct ns_common ns;
struct mount * root;
@@ -80,6 +82,8 @@ struct mount {
#ifdef CONFIG_FSNOTIFY
struct fsnotify_mark_connector __rcu *mnt_fsnotify_marks;
__u32 mnt_fsnotify_mask;
+ struct list_head to_notify; /* need to queue notification */
+ struct mnt_namespace *prev_ns; /* previous namespace (NULL if none) */
#endif
int mnt_id; /* mount identifier, reused */
u64 mnt_id_unique; /* mount ID unique until reboot */
@@ -182,4 +186,20 @@ static inline struct mnt_namespace *to_mnt_ns(struct ns_common *ns)
return container_of(ns, struct mnt_namespace, ns);
}
+#ifdef CONFIG_FSNOTIFY
+static inline void mnt_notify_add(struct mount *m)
+{
+ /* Optimize the case where there are no watches */
+ if ((m->mnt_ns && m->mnt_ns->n_fsnotify_marks) ||
+ (m->prev_ns && m->prev_ns->n_fsnotify_marks))
+ list_add_tail(&m->to_notify, ¬ify_list);
+ else
+ m->prev_ns = m->mnt_ns;
+}
+#else
+static inline void mnt_notify_add(struct mount *m)
+{
+}
+#endif
+
struct mnt_namespace *mnt_ns_from_dentry(struct dentry *dentry);
diff --git a/fs/namespace.c b/fs/namespace.c
index d8d70da56e7b..1e964b646509 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -81,6 +81,9 @@ static HLIST_HEAD(unmounted); /* protected by namespace_sem */
static LIST_HEAD(ex_mountpoints); /* protected by namespace_sem */
static DEFINE_SEQLOCK(mnt_ns_tree_lock);
+#ifdef CONFIG_FSNOTIFY
+LIST_HEAD(notify_list); /* protected by namespace_sem */
+#endif
static struct rb_root mnt_ns_tree = RB_ROOT; /* protected by mnt_ns_tree_lock */
static LIST_HEAD(mnt_ns_list); /* protected by mnt_ns_tree_lock */
@@ -163,6 +166,7 @@ static void mnt_ns_release(struct mnt_namespace *ns)
{
/* keep alive for {list,stat}mount() */
if (refcount_dec_and_test(&ns->passive)) {
+ fsnotify_mntns_delete(ns);
put_user_ns(ns->user_ns);
kfree(ns);
}
@@ -1176,6 +1180,8 @@ static void mnt_add_to_ns(struct mnt_namespace *ns, struct mount *mnt)
ns->mnt_first_node = &mnt->mnt_node;
rb_link_node(&mnt->mnt_node, parent, link);
rb_insert_color(&mnt->mnt_node, &ns->mounts);
+
+ mnt_notify_add(mnt);
}
/*
@@ -1723,6 +1729,50 @@ int may_umount(struct vfsmount *mnt)
EXPORT_SYMBOL(may_umount);
+#ifdef CONFIG_FSNOTIFY
+static void mnt_notify(struct mount *p)
+{
+ if (!p->prev_ns && p->mnt_ns) {
+ fsnotify_mnt_attach(p->mnt_ns, &p->mnt);
+ } else if (p->prev_ns && !p->mnt_ns) {
+ fsnotify_mnt_detach(p->prev_ns, &p->mnt);
+ } else if (p->prev_ns == p->mnt_ns) {
+ fsnotify_mnt_move(p->mnt_ns, &p->mnt);
+ } else {
+ fsnotify_mnt_detach(p->prev_ns, &p->mnt);
+ fsnotify_mnt_attach(p->mnt_ns, &p->mnt);
+ }
+ p->prev_ns = p->mnt_ns;
+}
+
+static void notify_mnt_list(void)
+{
+ struct mount *m, *tmp;
+ /*
+ * Notify about mounts that were added/reparented/detached/remain
+ * connected after unmount.
+ */
+ list_for_each_entry_safe(m, tmp, ¬ify_list, to_notify) {
+ mnt_notify(m);
+ list_del_init(&m->to_notify);
+ }
+}
+
+static bool need_notify_mnt_list(void)
+{
+ return !list_empty(¬ify_list);
+}
+#else
+static void notify_mnt_list(void)
+{
+}
+
+static bool need_notify_mnt_list(void)
+{
+ return false;
+}
+#endif
+
static void namespace_unlock(void)
{
struct hlist_head head;
@@ -1733,7 +1783,18 @@ static void namespace_unlock(void)
hlist_move_list(&unmounted, &head);
list_splice_init(&ex_mountpoints, &list);
- up_write(&namespace_sem);
+ if (need_notify_mnt_list()) {
+ /*
+ * No point blocking out concurrent readers while notifications
+ * are sent. This will also allow statmount()/listmount() to run
+ * concurrently.
+ */
+ downgrade_write(&namespace_sem);
+ notify_mnt_list();
+ up_read(&namespace_sem);
+ } else {
+ up_write(&namespace_sem);
+ }
shrink_dentry_list(&list);
@@ -1846,6 +1907,19 @@ static void umount_tree(struct mount *mnt, enum umount_tree_flags how)
change_mnt_propagation(p, MS_PRIVATE);
if (disconnect)
hlist_add_head(&p->mnt_umount, &unmounted);
+
+ /*
+ * At this point p->mnt_ns is NULL, notification will be queued
+ * only if
+ *
+ * - p->prev_ns is non-NULL *and*
+ * - p->prev_ns->n_fsnotify_marks is non-NULL
+ *
+ * This will preclude queuing the mount if this is a cleanup
+ * after a failed copy_tree() or destruction of an anonymous
+ * namespace, etc.
+ */
+ mnt_notify_add(p);
}
}
@@ -2555,6 +2629,7 @@ static int attach_recursive_mnt(struct mount *source_mnt,
dest_mp = smp;
unhash_mnt(source_mnt);
attach_mnt(source_mnt, top_mnt, dest_mp, beneath);
+ mnt_notify_add(source_mnt);
touch_mnt_namespace(source_mnt->mnt_ns);
} else {
if (source_mnt->mnt_ns) {
@@ -4476,6 +4551,8 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root,
list_del_init(&new_mnt->mnt_expire);
put_mountpoint(root_mp);
unlock_mount_hash();
+ mnt_notify_add(root_mnt);
+ mnt_notify_add(new_mnt);
chroot_fs_refs(&root, &new);
error = 0;
out4:
diff --git a/fs/pnode.c b/fs/pnode.c
index ef048f008bdd..82d809c785ec 100644
--- a/fs/pnode.c
+++ b/fs/pnode.c
@@ -549,8 +549,10 @@ static void restore_mounts(struct list_head *to_restore)
mp = parent->mnt_mp;
parent = parent->mnt_parent;
}
- if (parent != mnt->mnt_parent)
+ if (parent != mnt->mnt_parent) {
mnt_change_mountpoint(parent, mp, mnt);
+ mnt_notify_add(mnt);
+ }
}
}
--
2.48.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v5 0/3] mount notification
2025-01-29 16:57 [PATCH v5 0/3] mount notification Miklos Szeredi
` (2 preceding siblings ...)
2025-01-29 16:58 ` [PATCH v5 3/3] vfs: add notifications for " Miklos Szeredi
@ 2025-01-30 16:07 ` Christian Brauner
3 siblings, 0 replies; 19+ messages in thread
From: Christian Brauner @ 2025-01-30 16:07 UTC (permalink / raw)
To: linux-fsdevel, Miklos Szeredi
Cc: Christian Brauner, Jan Kara, Amir Goldstein, Karel Zak,
Lennart Poettering, Ian Kent, Alexander Viro, Paul Moore, selinux,
linux-security-module, selinux-refpolicy
On Wed, 29 Jan 2025 17:57:58 +0100, Miklos Szeredi wrote:
> This should be ready for adding to the v6.15 queue. I don't see the
> SELinux discussion converging, so I took the simpler version out of the two
> that were suggested.
>
> Will work on adding selftests.
>
> Thanks to everyone for the reviews!
>
> [...]
Applied to the vfs-6.15.mount branch of the vfs/vfs.git tree.
Patches in the vfs-6.15.mount branch should appear in linux-next soon.
Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.
It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.
Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-6.15.mount
[1/3] fsnotify: add mount notification infrastructure
https://git.kernel.org/vfs/vfs/c/3b02618006ea
[2/3] fanotify: notify on mount attach and detach
https://git.kernel.org/vfs/vfs/c/681803d6e3e1
[3/3] vfs: add notifications for mount attach and detach
https://git.kernel.org/vfs/vfs/c/48d9da32719f
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-01-29 16:58 ` [PATCH v5 2/3] fanotify: notify on mount attach and detach Miklos Szeredi
@ 2025-01-30 21:05 ` Paul Moore
2025-01-31 10:53 ` Miklos Szeredi
2025-01-31 12:09 ` Christian Brauner
2025-02-11 13:32 ` Jan Kara
1 sibling, 2 replies; 19+ messages in thread
From: Paul Moore @ 2025-01-30 21:05 UTC (permalink / raw)
To: Miklos Szeredi, Christian Brauner
Cc: linux-fsdevel, Jan Kara, Amir Goldstein, Karel Zak,
Lennart Poettering, Ian Kent, Alexander Viro, selinux,
linux-security-module, selinux-refpolicy
On Wed, Jan 29, 2025 at 11:58 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
>
> Add notifications for attaching and detaching mounts. The following new
> event masks are added:
>
> FAN_MNT_ATTACH - Mount was attached
> FAN_MNT_DETACH - Mount was detached
>
> If a mount is moved, then the event is reported with (FAN_MNT_ATTACH |
> FAN_MNT_DETACH).
>
> These events add an info record of type FAN_EVENT_INFO_TYPE_MNT containing
> these fields identifying the affected mounts:
>
> __u64 mnt_id - the ID of the mount (see statmount(2))
>
> FAN_REPORT_MNT must be supplied to fanotify_init() to receive these events
> and no other type of event can be received with this report type.
>
> Marks are added with FAN_MARK_MNTNS, which records the mount namespace from
> an nsfs file (e.g. /proc/self/ns/mnt).
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
> fs/mount.h | 2 +
> fs/namespace.c | 14 +++--
> fs/notify/fanotify/fanotify.c | 38 +++++++++++--
> fs/notify/fanotify/fanotify.h | 18 +++++++
> fs/notify/fanotify/fanotify_user.c | 87 +++++++++++++++++++++++++-----
> fs/notify/fdinfo.c | 5 ++
> include/linux/fanotify.h | 12 +++--
> include/uapi/linux/fanotify.h | 10 ++++
> security/selinux/hooks.c | 4 ++
> 9 files changed, 167 insertions(+), 23 deletions(-)
...
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7b867dfec88b..06d073eab53c 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3395,6 +3395,10 @@ static int selinux_path_notify(const struct path *path, u64 mask,
> case FSNOTIFY_OBJ_TYPE_INODE:
> perm = FILE__WATCH;
> break;
> + case FSNOTIFY_OBJ_TYPE_MNTNS:
> + /* Maybe introduce FILE__WATCH_MOUNTNS? */
> + perm = FILE__WATCH_MOUNT;
> + break;
> default:
> return -EINVAL;
> }
Ignoring for a moment that this patch was merged without an explicit
ACK for the SELinux changes, let's talk about these SELinux changes
...
I understand that you went with the "simpler version" because you
didn't believe the discussion was converging, which is fair, however,
I believe Daniel's argument is convincing enough to warrant the new
permission. Yes, it has taken me approximately two days to find the
time to revisit this topic and reply with some clarity, but personally
I feel like that is not an unreasonable period of time, especially for
a new feature discussion occurring during the merge window.
If you need an example on how to add a new SELinux permission, you can
look at commit ed5d44d42c95 ("selinux: Implement userns_create hook")
for a fairly simple example. In the watch_mountns case things are
slightly different due to the existence of the COMMON_FILE_PERMS
macro, but you basically want to add "watch_mountns" to the end of the
COMMON_FILE_PERMS macro in security/selinux/include/classmap.h. Of
course if you aren't sure about something, let me know. As a FYI, my
network access will be spotty starting tonight and extending through
the weekend, but I will have network/mail access at least once a day.
Now back to the merge into the VFS tree ... I was very surprised to
open this patchset and see that Christian had merged v5 after less
than 24 hours (at least according to the email timestamps that I see)
and without an explicit ACK for the SELinux changes. I've mentioned
this to you before Christian, please do not merge any SELinux, LSM
framework, or audit related patches without an explicit ACK. I
recognize that sometimes there are highly critical security issues
that need immediate attention, but that is clearly not the case here,
and there are other procedures to help deal with those emergency
scenarios.
--
paul-moore.com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-01-30 21:05 ` Paul Moore
@ 2025-01-31 10:53 ` Miklos Szeredi
2025-01-31 14:28 ` Paul Moore
2025-02-04 10:20 ` Christian Brauner
2025-01-31 12:09 ` Christian Brauner
1 sibling, 2 replies; 19+ messages in thread
From: Miklos Szeredi @ 2025-01-31 10:53 UTC (permalink / raw)
To: Paul Moore
Cc: Miklos Szeredi, Christian Brauner, linux-fsdevel, Jan Kara,
Amir Goldstein, Karel Zak, Lennart Poettering, Ian Kent,
Alexander Viro, selinux, linux-security-module, selinux-refpolicy
On Thu, 30 Jan 2025 at 22:06, Paul Moore <paul@paul-moore.com> wrote:
>
> On Wed, Jan 29, 2025 at 11:58 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> >
> > Add notifications for attaching and detaching mounts. The following new
> > event masks are added:
> >
> > FAN_MNT_ATTACH - Mount was attached
> > FAN_MNT_DETACH - Mount was detached
> >
> > If a mount is moved, then the event is reported with (FAN_MNT_ATTACH |
> > FAN_MNT_DETACH).
> >
> > These events add an info record of type FAN_EVENT_INFO_TYPE_MNT containing
> > these fields identifying the affected mounts:
> >
> > __u64 mnt_id - the ID of the mount (see statmount(2))
> >
> > FAN_REPORT_MNT must be supplied to fanotify_init() to receive these events
> > and no other type of event can be received with this report type.
> >
> > Marks are added with FAN_MARK_MNTNS, which records the mount namespace from
> > an nsfs file (e.g. /proc/self/ns/mnt).
> >
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> > fs/mount.h | 2 +
> > fs/namespace.c | 14 +++--
> > fs/notify/fanotify/fanotify.c | 38 +++++++++++--
> > fs/notify/fanotify/fanotify.h | 18 +++++++
> > fs/notify/fanotify/fanotify_user.c | 87 +++++++++++++++++++++++++-----
> > fs/notify/fdinfo.c | 5 ++
> > include/linux/fanotify.h | 12 +++--
> > include/uapi/linux/fanotify.h | 10 ++++
> > security/selinux/hooks.c | 4 ++
> > 9 files changed, 167 insertions(+), 23 deletions(-)
>
> ...
>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 7b867dfec88b..06d073eab53c 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -3395,6 +3395,10 @@ static int selinux_path_notify(const struct path *path, u64 mask,
> > case FSNOTIFY_OBJ_TYPE_INODE:
> > perm = FILE__WATCH;
> > break;
> > + case FSNOTIFY_OBJ_TYPE_MNTNS:
> > + /* Maybe introduce FILE__WATCH_MOUNTNS? */
> > + perm = FILE__WATCH_MOUNT;
> > + break;
> > default:
> > return -EINVAL;
> > }
>
> Ignoring for a moment that this patch was merged without an explicit
> ACK for the SELinux changes, let's talk about these SELinux changes
> ...
>
> I understand that you went with the "simpler version" because you
> didn't believe the discussion was converging, which is fair, however,
> I believe Daniel's argument is convincing enough to warrant the new
> permission.
Fine, I'll work on this.
> Yes, it has taken me approximately two days to find the
> time to revisit this topic and reply with some clarity, but personally
> I feel like that is not an unreasonable period of time, especially for
> a new feature discussion occurring during the merge window.
Definitely not.
Christian is definitely very responsive and quick to queue things up,
and that can have drawbacks. In this he made it clear that he wants
to get this queued ASAP regardless of whether there's decision on the
SELinux side or not.
What I think might be a good thing if Christian could record
conditional NAKs such as this one from you, that need to be worked on
before sending a feature upstream. That would prevent wrong code
being sent upstream due to lack of attention.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-01-30 21:05 ` Paul Moore
2025-01-31 10:53 ` Miklos Szeredi
@ 2025-01-31 12:09 ` Christian Brauner
2025-01-31 14:39 ` Paul Moore
1 sibling, 1 reply; 19+ messages in thread
From: Christian Brauner @ 2025-01-31 12:09 UTC (permalink / raw)
To: Paul Moore
Cc: Miklos Szeredi, linux-fsdevel, Jan Kara, Amir Goldstein,
Karel Zak, Lennart Poettering, Ian Kent, Alexander Viro, selinux,
linux-security-module, selinux-refpolicy
On Thu, Jan 30, 2025 at 04:05:53PM -0500, Paul Moore wrote:
> On Wed, Jan 29, 2025 at 11:58 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> >
> > Add notifications for attaching and detaching mounts. The following new
> > event masks are added:
> >
> > FAN_MNT_ATTACH - Mount was attached
> > FAN_MNT_DETACH - Mount was detached
> >
> > If a mount is moved, then the event is reported with (FAN_MNT_ATTACH |
> > FAN_MNT_DETACH).
> >
> > These events add an info record of type FAN_EVENT_INFO_TYPE_MNT containing
> > these fields identifying the affected mounts:
> >
> > __u64 mnt_id - the ID of the mount (see statmount(2))
> >
> > FAN_REPORT_MNT must be supplied to fanotify_init() to receive these events
> > and no other type of event can be received with this report type.
> >
> > Marks are added with FAN_MARK_MNTNS, which records the mount namespace from
> > an nsfs file (e.g. /proc/self/ns/mnt).
> >
> > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > ---
> > fs/mount.h | 2 +
> > fs/namespace.c | 14 +++--
> > fs/notify/fanotify/fanotify.c | 38 +++++++++++--
> > fs/notify/fanotify/fanotify.h | 18 +++++++
> > fs/notify/fanotify/fanotify_user.c | 87 +++++++++++++++++++++++++-----
> > fs/notify/fdinfo.c | 5 ++
> > include/linux/fanotify.h | 12 +++--
> > include/uapi/linux/fanotify.h | 10 ++++
> > security/selinux/hooks.c | 4 ++
> > 9 files changed, 167 insertions(+), 23 deletions(-)
>
> ...
>
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 7b867dfec88b..06d073eab53c 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -3395,6 +3395,10 @@ static int selinux_path_notify(const struct path *path, u64 mask,
> > case FSNOTIFY_OBJ_TYPE_INODE:
> > perm = FILE__WATCH;
> > break;
> > + case FSNOTIFY_OBJ_TYPE_MNTNS:
> > + /* Maybe introduce FILE__WATCH_MOUNTNS? */
> > + perm = FILE__WATCH_MOUNT;
> > + break;
> > default:
> > return -EINVAL;
> > }
>
> Ignoring for a moment that this patch was merged without an explicit
> ACK for the SELinux changes, let's talk about these SELinux changes
> ...
>
> I understand that you went with the "simpler version" because you
> didn't believe the discussion was converging, which is fair, however,
> I believe Daniel's argument is convincing enough to warrant the new
> permission. Yes, it has taken me approximately two days to find the
> time to revisit this topic and reply with some clarity, but personally
> I feel like that is not an unreasonable period of time, especially for
> a new feature discussion occurring during the merge window.
>
> If you need an example on how to add a new SELinux permission, you can
> look at commit ed5d44d42c95 ("selinux: Implement userns_create hook")
> for a fairly simple example. In the watch_mountns case things are
> slightly different due to the existence of the COMMON_FILE_PERMS
> macro, but you basically want to add "watch_mountns" to the end of the
> COMMON_FILE_PERMS macro in security/selinux/include/classmap.h. Of
> course if you aren't sure about something, let me know. As a FYI, my
> network access will be spotty starting tonight and extending through
> the weekend, but I will have network/mail access at least once a day.
>
> Now back to the merge into the VFS tree ... I was very surprised to
> open this patchset and see that Christian had merged v5 after less
> than 24 hours (at least according to the email timestamps that I see)
> and without an explicit ACK for the SELinux changes. I've mentioned
> this to you before Christian, please do not merge any SELinux, LSM
> framework, or audit related patches without an explicit ACK. I
Things go into the tree for testing when the VFS side is ready for
testing. We're at v5 and the patchset has gone through four iterations
over multiple months. It will go into linux-next and fs-next now for as
much expsure as possible.
I'm not sure what the confusion between merging things into a tree and
sending things upstream is. I have explained this to you before. The
application message is also pretty clear about that.
> recognize that sometimes there are highly critical security issues
> that need immediate attention, but that is clearly not the case here,
> and there are other procedures to help deal with those emergency
> scenarios.
>
> --
> paul-moore.com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-01-31 10:53 ` Miklos Szeredi
@ 2025-01-31 14:28 ` Paul Moore
2025-02-04 10:19 ` Christian Brauner
2025-02-04 10:20 ` Christian Brauner
1 sibling, 1 reply; 19+ messages in thread
From: Paul Moore @ 2025-01-31 14:28 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Miklos Szeredi, Christian Brauner, linux-fsdevel, Jan Kara,
Amir Goldstein, Karel Zak, Lennart Poettering, Ian Kent,
Alexander Viro, selinux, linux-security-module, selinux-refpolicy
On Fri, Jan 31, 2025 at 5:53 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> On Thu, 30 Jan 2025 at 22:06, Paul Moore <paul@paul-moore.com> wrote:
> > On Wed, Jan 29, 2025 at 11:58 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> > >
> > > Add notifications for attaching and detaching mounts. The following new
> > > event masks are added:
> > >
> > > FAN_MNT_ATTACH - Mount was attached
> > > FAN_MNT_DETACH - Mount was detached
> > >
> > > If a mount is moved, then the event is reported with (FAN_MNT_ATTACH |
> > > FAN_MNT_DETACH).
> > >
> > > These events add an info record of type FAN_EVENT_INFO_TYPE_MNT containing
> > > these fields identifying the affected mounts:
> > >
> > > __u64 mnt_id - the ID of the mount (see statmount(2))
> > >
> > > FAN_REPORT_MNT must be supplied to fanotify_init() to receive these events
> > > and no other type of event can be received with this report type.
> > >
> > > Marks are added with FAN_MARK_MNTNS, which records the mount namespace from
> > > an nsfs file (e.g. /proc/self/ns/mnt).
> > >
> > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > > ---
> > > fs/mount.h | 2 +
> > > fs/namespace.c | 14 +++--
> > > fs/notify/fanotify/fanotify.c | 38 +++++++++++--
> > > fs/notify/fanotify/fanotify.h | 18 +++++++
> > > fs/notify/fanotify/fanotify_user.c | 87 +++++++++++++++++++++++++-----
> > > fs/notify/fdinfo.c | 5 ++
> > > include/linux/fanotify.h | 12 +++--
> > > include/uapi/linux/fanotify.h | 10 ++++
> > > security/selinux/hooks.c | 4 ++
> > > 9 files changed, 167 insertions(+), 23 deletions(-)
> >
> > ...
> >
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index 7b867dfec88b..06d073eab53c 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -3395,6 +3395,10 @@ static int selinux_path_notify(const struct path *path, u64 mask,
> > > case FSNOTIFY_OBJ_TYPE_INODE:
> > > perm = FILE__WATCH;
> > > break;
> > > + case FSNOTIFY_OBJ_TYPE_MNTNS:
> > > + /* Maybe introduce FILE__WATCH_MOUNTNS? */
> > > + perm = FILE__WATCH_MOUNT;
> > > + break;
> > > default:
> > > return -EINVAL;
> > > }
> >
> > Ignoring for a moment that this patch was merged without an explicit
> > ACK for the SELinux changes, let's talk about these SELinux changes
> > ...
> >
> > I understand that you went with the "simpler version" because you
> > didn't believe the discussion was converging, which is fair, however,
> > I believe Daniel's argument is convincing enough to warrant the new
> > permission.
>
> Fine, I'll work on this.
Great, thanks.
> > Yes, it has taken me approximately two days to find the
> > time to revisit this topic and reply with some clarity, but personally
> > I feel like that is not an unreasonable period of time, especially for
> > a new feature discussion occurring during the merge window.
>
> Definitely not.
>
> Christian is definitely very responsive and quick to queue things up,
> and that can have drawbacks. In this he made it clear that he wants
> to get this queued ASAP regardless of whether there's decision on the
> SELinux side or not.
When one merges code that affects another subsystem without an
explicit ACK from the affected subsystem when the maintainer has asked
for others to clear the code change with an ACK, it's hard to see that
as anything but bad behavior at its best and reckless behavior at its
worst. It is doubly troubling in cases like this where the code
change is user visible.
> What I think might be a good thing if Christian could record
> conditional NAKs such as this one from you, that need to be worked on
> before sending a feature upstream. That would prevent wrong code
> being sent upstream due to lack of attention.
Christian's merge notification email already has this section:
"Please report any outstanding bugs that were missed
during review in a new review to the original patch series
allowing us to drop it."
... and to be fair, the vfs-6.15.mount branch mentioned in the
notification does appear to be gone.
--
paul-moore.com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-01-31 12:09 ` Christian Brauner
@ 2025-01-31 14:39 ` Paul Moore
2025-02-04 10:07 ` Christian Brauner
0 siblings, 1 reply; 19+ messages in thread
From: Paul Moore @ 2025-01-31 14:39 UTC (permalink / raw)
To: Christian Brauner
Cc: Miklos Szeredi, linux-fsdevel, Jan Kara, Amir Goldstein,
Karel Zak, Lennart Poettering, Ian Kent, Alexander Viro, selinux,
linux-security-module, selinux-refpolicy
On Fri, Jan 31, 2025 at 7:09 AM Christian Brauner <brauner@kernel.org> wrote:
> On Thu, Jan 30, 2025 at 04:05:53PM -0500, Paul Moore wrote:
> >
> > Now back to the merge into the VFS tree ... I was very surprised to
> > open this patchset and see that Christian had merged v5 after less
> > than 24 hours (at least according to the email timestamps that I see)
> > and without an explicit ACK for the SELinux changes. I've mentioned
> > this to you before Christian, please do not merge any SELinux, LSM
> > framework, or audit related patches without an explicit ACK. I
>
> Things go into the tree for testing when the VFS side is ready for
> testing. We're at v5 and the patchset has gone through four iterations
> over multiple months. It will go into linux-next and fs-next now for as
> much expsure as possible.
>
> I'm not sure what the confusion between merging things into a tree and
> sending things upstream is. I have explained this to you before. The
> application message is also pretty clear about that.
I'm not sure what the confusion is around my explicit request that you
refrain from merging anything that touches the LSM framework, SELinux,
or the audit subsystem without an explicit ACK. I have explained this
to you before.
For the record, your application/merge email makes no statement about
only sending patches to Linus that have been ACK'd by all relevant
parties. The only statement I can see in your email that remotely
relates to ACKs is this:
"It's encouraged to provide Acked-bys and Reviewed-bys
even though the patch has now been applied. If possible
patch trailers will be updated."
... which once again makes no claims about holding back changes that
have not been properly ACK'd.
--
paul-moore.com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-01-31 14:39 ` Paul Moore
@ 2025-02-04 10:07 ` Christian Brauner
2025-02-04 23:52 ` Paul Moore
0 siblings, 1 reply; 19+ messages in thread
From: Christian Brauner @ 2025-02-04 10:07 UTC (permalink / raw)
To: Paul Moore
Cc: Miklos Szeredi, linux-fsdevel, Jan Kara, Amir Goldstein,
Karel Zak, Lennart Poettering, Ian Kent, Alexander Viro, selinux,
linux-security-module, selinux-refpolicy
On Fri, Jan 31, 2025 at 09:39:31AM -0500, Paul Moore wrote:
> On Fri, Jan 31, 2025 at 7:09 AM Christian Brauner <brauner@kernel.org> wrote:
> > On Thu, Jan 30, 2025 at 04:05:53PM -0500, Paul Moore wrote:
> > >
> > > Now back to the merge into the VFS tree ... I was very surprised to
> > > open this patchset and see that Christian had merged v5 after less
> > > than 24 hours (at least according to the email timestamps that I see)
> > > and without an explicit ACK for the SELinux changes. I've mentioned
> > > this to you before Christian, please do not merge any SELinux, LSM
> > > framework, or audit related patches without an explicit ACK. I
> >
> > Things go into the tree for testing when the VFS side is ready for
> > testing. We're at v5 and the patchset has gone through four iterations
> > over multiple months. It will go into linux-next and fs-next now for as
> > much expsure as possible.
> >
> > I'm not sure what the confusion between merging things into a tree and
> > sending things upstream is. I have explained this to you before. The
> > application message is also pretty clear about that.
>
> I'm not sure what the confusion is around my explicit request that you
> refrain from merging anything that touches the LSM framework, SELinux,
> or the audit subsystem without an explicit ACK. I have explained this
> to you before.
>
> For the record, your application/merge email makes no statement about
> only sending patches to Linus that have been ACK'd by all relevant
> parties. The only statement I can see in your email that remotely
> relates to ACKs is this:
>
> "It's encouraged to provide Acked-bys and Reviewed-bys
> even though the patch has now been applied. If possible
> patch trailers will be updated."
>
> ... which once again makes no claims about holding back changes that
> have not been properly ACK'd.
If seems you're having difficulties understanding that included patches
are subject to be updated from this content. You might want to remember
that this is similar for the various mm trees where this isn't even
mentioned. In other words this isn't a novel concept.
Anyway, VFS patch series will continue to appear in testing trees when
they are ready from the VFS side.
Going forward it might be best to add the required LSM integration via
the LSM subsystem.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-01-31 14:28 ` Paul Moore
@ 2025-02-04 10:19 ` Christian Brauner
0 siblings, 0 replies; 19+ messages in thread
From: Christian Brauner @ 2025-02-04 10:19 UTC (permalink / raw)
To: Paul Moore
Cc: Miklos Szeredi, Miklos Szeredi, linux-fsdevel, Jan Kara,
Amir Goldstein, Karel Zak, Lennart Poettering, Ian Kent,
Alexander Viro, selinux, linux-security-module, selinux-refpolicy
On Fri, Jan 31, 2025 at 09:28:31AM -0500, Paul Moore wrote:
> On Fri, Jan 31, 2025 at 5:53 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > On Thu, 30 Jan 2025 at 22:06, Paul Moore <paul@paul-moore.com> wrote:
> > > On Wed, Jan 29, 2025 at 11:58 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> > > >
> > > > Add notifications for attaching and detaching mounts. The following new
> > > > event masks are added:
> > > >
> > > > FAN_MNT_ATTACH - Mount was attached
> > > > FAN_MNT_DETACH - Mount was detached
> > > >
> > > > If a mount is moved, then the event is reported with (FAN_MNT_ATTACH |
> > > > FAN_MNT_DETACH).
> > > >
> > > > These events add an info record of type FAN_EVENT_INFO_TYPE_MNT containing
> > > > these fields identifying the affected mounts:
> > > >
> > > > __u64 mnt_id - the ID of the mount (see statmount(2))
> > > >
> > > > FAN_REPORT_MNT must be supplied to fanotify_init() to receive these events
> > > > and no other type of event can be received with this report type.
> > > >
> > > > Marks are added with FAN_MARK_MNTNS, which records the mount namespace from
> > > > an nsfs file (e.g. /proc/self/ns/mnt).
> > > >
> > > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > > > ---
> > > > fs/mount.h | 2 +
> > > > fs/namespace.c | 14 +++--
> > > > fs/notify/fanotify/fanotify.c | 38 +++++++++++--
> > > > fs/notify/fanotify/fanotify.h | 18 +++++++
> > > > fs/notify/fanotify/fanotify_user.c | 87 +++++++++++++++++++++++++-----
> > > > fs/notify/fdinfo.c | 5 ++
> > > > include/linux/fanotify.h | 12 +++--
> > > > include/uapi/linux/fanotify.h | 10 ++++
> > > > security/selinux/hooks.c | 4 ++
> > > > 9 files changed, 167 insertions(+), 23 deletions(-)
> > >
> > > ...
> > >
> > > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > > index 7b867dfec88b..06d073eab53c 100644
> > > > --- a/security/selinux/hooks.c
> > > > +++ b/security/selinux/hooks.c
> > > > @@ -3395,6 +3395,10 @@ static int selinux_path_notify(const struct path *path, u64 mask,
> > > > case FSNOTIFY_OBJ_TYPE_INODE:
> > > > perm = FILE__WATCH;
> > > > break;
> > > > + case FSNOTIFY_OBJ_TYPE_MNTNS:
> > > > + /* Maybe introduce FILE__WATCH_MOUNTNS? */
> > > > + perm = FILE__WATCH_MOUNT;
> > > > + break;
> > > > default:
> > > > return -EINVAL;
> > > > }
> > >
> > > Ignoring for a moment that this patch was merged without an explicit
> > > ACK for the SELinux changes, let's talk about these SELinux changes
> > > ...
> > >
> > > I understand that you went with the "simpler version" because you
> > > didn't believe the discussion was converging, which is fair, however,
> > > I believe Daniel's argument is convincing enough to warrant the new
> > > permission.
> >
> > Fine, I'll work on this.
>
> Great, thanks.
>
> > > Yes, it has taken me approximately two days to find the
> > > time to revisit this topic and reply with some clarity, but personally
> > > I feel like that is not an unreasonable period of time, especially for
> > > a new feature discussion occurring during the merge window.
> >
> > Definitely not.
> >
> > Christian is definitely very responsive and quick to queue things up,
> > and that can have drawbacks. In this he made it clear that he wants
> > to get this queued ASAP regardless of whether there's decision on the
> > SELinux side or not.
>
> When one merges code that affects another subsystem without an
> explicit ACK from the affected subsystem when the maintainer has asked
> for others to clear the code change with an ACK, it's hard to see that
> as anything but bad behavior at its best and reckless behavior at its
> worst. It is doubly troubling in cases like this where the code
> change is user visible.
>
> > What I think might be a good thing if Christian could record
> > conditional NAKs such as this one from you, that need to be worked on
> > before sending a feature upstream. That would prevent wrong code
> > being sent upstream due to lack of attention.
>
> Christian's merge notification email already has this section:
>
> "Please report any outstanding bugs that were missed
> during review in a new review to the original patch series
> allowing us to drop it."
>
> ... and to be fair, the vfs-6.15.mount branch mentioned in the
> notification does appear to be gone.
The branch is very much alive but it has never been public for very
obvious reasons: new patches don't appear in -next or elsewhere until
the merge window is closed. They will be pushed out today.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-01-31 10:53 ` Miklos Szeredi
2025-01-31 14:28 ` Paul Moore
@ 2025-02-04 10:20 ` Christian Brauner
1 sibling, 0 replies; 19+ messages in thread
From: Christian Brauner @ 2025-02-04 10:20 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Paul Moore, Miklos Szeredi, linux-fsdevel, Jan Kara,
Amir Goldstein, Karel Zak, Lennart Poettering, Ian Kent,
Alexander Viro, selinux, linux-security-module, selinux-refpolicy
On Fri, Jan 31, 2025 at 11:53:33AM +0100, Miklos Szeredi wrote:
> On Thu, 30 Jan 2025 at 22:06, Paul Moore <paul@paul-moore.com> wrote:
> >
> > On Wed, Jan 29, 2025 at 11:58 AM Miklos Szeredi <mszeredi@redhat.com> wrote:
> > >
> > > Add notifications for attaching and detaching mounts. The following new
> > > event masks are added:
> > >
> > > FAN_MNT_ATTACH - Mount was attached
> > > FAN_MNT_DETACH - Mount was detached
> > >
> > > If a mount is moved, then the event is reported with (FAN_MNT_ATTACH |
> > > FAN_MNT_DETACH).
> > >
> > > These events add an info record of type FAN_EVENT_INFO_TYPE_MNT containing
> > > these fields identifying the affected mounts:
> > >
> > > __u64 mnt_id - the ID of the mount (see statmount(2))
> > >
> > > FAN_REPORT_MNT must be supplied to fanotify_init() to receive these events
> > > and no other type of event can be received with this report type.
> > >
> > > Marks are added with FAN_MARK_MNTNS, which records the mount namespace from
> > > an nsfs file (e.g. /proc/self/ns/mnt).
> > >
> > > Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> > > ---
> > > fs/mount.h | 2 +
> > > fs/namespace.c | 14 +++--
> > > fs/notify/fanotify/fanotify.c | 38 +++++++++++--
> > > fs/notify/fanotify/fanotify.h | 18 +++++++
> > > fs/notify/fanotify/fanotify_user.c | 87 +++++++++++++++++++++++++-----
> > > fs/notify/fdinfo.c | 5 ++
> > > include/linux/fanotify.h | 12 +++--
> > > include/uapi/linux/fanotify.h | 10 ++++
> > > security/selinux/hooks.c | 4 ++
> > > 9 files changed, 167 insertions(+), 23 deletions(-)
> >
> > ...
> >
> > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > > index 7b867dfec88b..06d073eab53c 100644
> > > --- a/security/selinux/hooks.c
> > > +++ b/security/selinux/hooks.c
> > > @@ -3395,6 +3395,10 @@ static int selinux_path_notify(const struct path *path, u64 mask,
> > > case FSNOTIFY_OBJ_TYPE_INODE:
> > > perm = FILE__WATCH;
> > > break;
> > > + case FSNOTIFY_OBJ_TYPE_MNTNS:
> > > + /* Maybe introduce FILE__WATCH_MOUNTNS? */
> > > + perm = FILE__WATCH_MOUNT;
> > > + break;
> > > default:
> > > return -EINVAL;
> > > }
> >
> > Ignoring for a moment that this patch was merged without an explicit
> > ACK for the SELinux changes, let's talk about these SELinux changes
> > ...
> >
> > I understand that you went with the "simpler version" because you
> > didn't believe the discussion was converging, which is fair, however,
> > I believe Daniel's argument is convincing enough to warrant the new
> > permission.
>
> Fine, I'll work on this.
Make it separate patches please. All LSM changes have been dropped.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-02-04 10:07 ` Christian Brauner
@ 2025-02-04 23:52 ` Paul Moore
0 siblings, 0 replies; 19+ messages in thread
From: Paul Moore @ 2025-02-04 23:52 UTC (permalink / raw)
To: Christian Brauner
Cc: Miklos Szeredi, linux-fsdevel, Jan Kara, Amir Goldstein,
Karel Zak, Lennart Poettering, Ian Kent, Alexander Viro, selinux,
linux-security-module, selinux-refpolicy
On Tue, Feb 4, 2025 at 5:07 AM Christian Brauner <brauner@kernel.org> wrote:
> On Fri, Jan 31, 2025 at 09:39:31AM -0500, Paul Moore wrote:
> > On Fri, Jan 31, 2025 at 7:09 AM Christian Brauner <brauner@kernel.org> wrote:
> > > On Thu, Jan 30, 2025 at 04:05:53PM -0500, Paul Moore wrote:
> > > >
> > > > Now back to the merge into the VFS tree ... I was very surprised to
> > > > open this patchset and see that Christian had merged v5 after less
> > > > than 24 hours (at least according to the email timestamps that I see)
> > > > and without an explicit ACK for the SELinux changes. I've mentioned
> > > > this to you before Christian, please do not merge any SELinux, LSM
> > > > framework, or audit related patches without an explicit ACK. I
> > >
> > > Things go into the tree for testing when the VFS side is ready for
> > > testing. We're at v5 and the patchset has gone through four iterations
> > > over multiple months. It will go into linux-next and fs-next now for as
> > > much expsure as possible.
> > >
> > > I'm not sure what the confusion between merging things into a tree and
> > > sending things upstream is. I have explained this to you before. The
> > > application message is also pretty clear about that.
> >
> > I'm not sure what the confusion is around my explicit request that you
> > refrain from merging anything that touches the LSM framework, SELinux,
> > or the audit subsystem without an explicit ACK. I have explained this
> > to you before.
> >
> > For the record, your application/merge email makes no statement about
> > only sending patches to Linus that have been ACK'd by all relevant
> > parties. The only statement I can see in your email that remotely
> > relates to ACKs is this:
> >
> > "It's encouraged to provide Acked-bys and Reviewed-bys
> > even though the patch has now been applied. If possible
> > patch trailers will be updated."
> >
> > ... which once again makes no claims about holding back changes that
> > have not been properly ACK'd.
>
> If seems you're having difficulties understanding that included patches
> are subject to be updated from this content.
I'm having difficulties reconciling the inconsistencies between what
you've said here (which is presumably your actual policy/behavior?)
and what you've said in your merge emails.
--
paul-moore.com
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 3/3] vfs: add notifications for mount attach and detach
2025-01-29 16:58 ` [PATCH v5 3/3] vfs: add notifications for " Miklos Szeredi
@ 2025-02-11 13:04 ` Jan Kara
0 siblings, 0 replies; 19+ messages in thread
From: Jan Kara @ 2025-02-11 13:04 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, Christian Brauner, Jan Kara, Amir Goldstein,
Karel Zak, Lennart Poettering, Ian Kent, Alexander Viro,
Paul Moore, selinux, linux-security-module, selinux-refpolicy
On Wed 29-01-25 17:58:01, Miklos Szeredi wrote:
> Add notifications for attaching and detaching mounts to fs/namespace.c
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/mount.h | 20 +++++++++++++
> fs/namespace.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++-
> fs/pnode.c | 4 ++-
> 3 files changed, 101 insertions(+), 2 deletions(-)
>
> diff --git a/fs/mount.h b/fs/mount.h
> index 5324a931b403..946dc8b792d7 100644
> --- a/fs/mount.h
> +++ b/fs/mount.h
> @@ -5,6 +5,8 @@
> #include <linux/ns_common.h>
> #include <linux/fs_pin.h>
>
> +extern struct list_head notify_list;
> +
> struct mnt_namespace {
> struct ns_common ns;
> struct mount * root;
> @@ -80,6 +82,8 @@ struct mount {
> #ifdef CONFIG_FSNOTIFY
> struct fsnotify_mark_connector __rcu *mnt_fsnotify_marks;
> __u32 mnt_fsnotify_mask;
> + struct list_head to_notify; /* need to queue notification */
> + struct mnt_namespace *prev_ns; /* previous namespace (NULL if none) */
> #endif
> int mnt_id; /* mount identifier, reused */
> u64 mnt_id_unique; /* mount ID unique until reboot */
> @@ -182,4 +186,20 @@ static inline struct mnt_namespace *to_mnt_ns(struct ns_common *ns)
> return container_of(ns, struct mnt_namespace, ns);
> }
>
> +#ifdef CONFIG_FSNOTIFY
> +static inline void mnt_notify_add(struct mount *m)
> +{
> + /* Optimize the case where there are no watches */
> + if ((m->mnt_ns && m->mnt_ns->n_fsnotify_marks) ||
> + (m->prev_ns && m->prev_ns->n_fsnotify_marks))
> + list_add_tail(&m->to_notify, ¬ify_list);
> + else
> + m->prev_ns = m->mnt_ns;
> +}
> +#else
> +static inline void mnt_notify_add(struct mount *m)
> +{
> +}
> +#endif
> +
> struct mnt_namespace *mnt_ns_from_dentry(struct dentry *dentry);
> diff --git a/fs/namespace.c b/fs/namespace.c
> index d8d70da56e7b..1e964b646509 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -81,6 +81,9 @@ static HLIST_HEAD(unmounted); /* protected by namespace_sem */
> static LIST_HEAD(ex_mountpoints); /* protected by namespace_sem */
> static DEFINE_SEQLOCK(mnt_ns_tree_lock);
>
> +#ifdef CONFIG_FSNOTIFY
> +LIST_HEAD(notify_list); /* protected by namespace_sem */
> +#endif
> static struct rb_root mnt_ns_tree = RB_ROOT; /* protected by mnt_ns_tree_lock */
> static LIST_HEAD(mnt_ns_list); /* protected by mnt_ns_tree_lock */
>
> @@ -163,6 +166,7 @@ static void mnt_ns_release(struct mnt_namespace *ns)
> {
> /* keep alive for {list,stat}mount() */
> if (refcount_dec_and_test(&ns->passive)) {
> + fsnotify_mntns_delete(ns);
> put_user_ns(ns->user_ns);
> kfree(ns);
> }
> @@ -1176,6 +1180,8 @@ static void mnt_add_to_ns(struct mnt_namespace *ns, struct mount *mnt)
> ns->mnt_first_node = &mnt->mnt_node;
> rb_link_node(&mnt->mnt_node, parent, link);
> rb_insert_color(&mnt->mnt_node, &ns->mounts);
> +
> + mnt_notify_add(mnt);
> }
>
> /*
> @@ -1723,6 +1729,50 @@ int may_umount(struct vfsmount *mnt)
>
> EXPORT_SYMBOL(may_umount);
>
> +#ifdef CONFIG_FSNOTIFY
> +static void mnt_notify(struct mount *p)
> +{
> + if (!p->prev_ns && p->mnt_ns) {
> + fsnotify_mnt_attach(p->mnt_ns, &p->mnt);
> + } else if (p->prev_ns && !p->mnt_ns) {
> + fsnotify_mnt_detach(p->prev_ns, &p->mnt);
> + } else if (p->prev_ns == p->mnt_ns) {
> + fsnotify_mnt_move(p->mnt_ns, &p->mnt);
> + } else {
> + fsnotify_mnt_detach(p->prev_ns, &p->mnt);
> + fsnotify_mnt_attach(p->mnt_ns, &p->mnt);
> + }
> + p->prev_ns = p->mnt_ns;
> +}
> +
> +static void notify_mnt_list(void)
> +{
> + struct mount *m, *tmp;
> + /*
> + * Notify about mounts that were added/reparented/detached/remain
> + * connected after unmount.
> + */
> + list_for_each_entry_safe(m, tmp, ¬ify_list, to_notify) {
> + mnt_notify(m);
> + list_del_init(&m->to_notify);
> + }
> +}
> +
> +static bool need_notify_mnt_list(void)
> +{
> + return !list_empty(¬ify_list);
> +}
> +#else
> +static void notify_mnt_list(void)
> +{
> +}
> +
> +static bool need_notify_mnt_list(void)
> +{
> + return false;
> +}
> +#endif
> +
> static void namespace_unlock(void)
> {
> struct hlist_head head;
> @@ -1733,7 +1783,18 @@ static void namespace_unlock(void)
> hlist_move_list(&unmounted, &head);
> list_splice_init(&ex_mountpoints, &list);
>
> - up_write(&namespace_sem);
> + if (need_notify_mnt_list()) {
> + /*
> + * No point blocking out concurrent readers while notifications
> + * are sent. This will also allow statmount()/listmount() to run
> + * concurrently.
> + */
> + downgrade_write(&namespace_sem);
> + notify_mnt_list();
> + up_read(&namespace_sem);
> + } else {
> + up_write(&namespace_sem);
> + }
>
> shrink_dentry_list(&list);
>
> @@ -1846,6 +1907,19 @@ static void umount_tree(struct mount *mnt, enum umount_tree_flags how)
> change_mnt_propagation(p, MS_PRIVATE);
> if (disconnect)
> hlist_add_head(&p->mnt_umount, &unmounted);
> +
> + /*
> + * At this point p->mnt_ns is NULL, notification will be queued
> + * only if
> + *
> + * - p->prev_ns is non-NULL *and*
> + * - p->prev_ns->n_fsnotify_marks is non-NULL
> + *
> + * This will preclude queuing the mount if this is a cleanup
> + * after a failed copy_tree() or destruction of an anonymous
> + * namespace, etc.
> + */
> + mnt_notify_add(p);
> }
> }
>
> @@ -2555,6 +2629,7 @@ static int attach_recursive_mnt(struct mount *source_mnt,
> dest_mp = smp;
> unhash_mnt(source_mnt);
> attach_mnt(source_mnt, top_mnt, dest_mp, beneath);
> + mnt_notify_add(source_mnt);
> touch_mnt_namespace(source_mnt->mnt_ns);
> } else {
> if (source_mnt->mnt_ns) {
> @@ -4476,6 +4551,8 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root,
> list_del_init(&new_mnt->mnt_expire);
> put_mountpoint(root_mp);
> unlock_mount_hash();
> + mnt_notify_add(root_mnt);
> + mnt_notify_add(new_mnt);
> chroot_fs_refs(&root, &new);
> error = 0;
> out4:
> diff --git a/fs/pnode.c b/fs/pnode.c
> index ef048f008bdd..82d809c785ec 100644
> --- a/fs/pnode.c
> +++ b/fs/pnode.c
> @@ -549,8 +549,10 @@ static void restore_mounts(struct list_head *to_restore)
> mp = parent->mnt_mp;
> parent = parent->mnt_parent;
> }
> - if (parent != mnt->mnt_parent)
> + if (parent != mnt->mnt_parent) {
> mnt_change_mountpoint(parent, mp, mnt);
> + mnt_notify_add(mnt);
> + }
> }
> }
>
> --
> 2.48.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 1/3] fsnotify: add mount notification infrastructure
2025-01-29 16:57 ` [PATCH v5 1/3] fsnotify: add mount notification infrastructure Miklos Szeredi
@ 2025-02-11 13:05 ` Jan Kara
0 siblings, 0 replies; 19+ messages in thread
From: Jan Kara @ 2025-02-11 13:05 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, Christian Brauner, Jan Kara, Amir Goldstein,
Karel Zak, Lennart Poettering, Ian Kent, Alexander Viro,
Paul Moore, selinux, linux-security-module, selinux-refpolicy
On Wed 29-01-25 17:57:59, Miklos Szeredi wrote:
> This is just the plumbing between the event source (fs/namespace.c) and the
> event consumer (fanotify). In itself it does nothing.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/mount.h | 4 +++
> fs/notify/fsnotify.c | 47 +++++++++++++++++++++++++++-----
> fs/notify/fsnotify.h | 11 ++++++++
> fs/notify/mark.c | 14 ++++++++--
> include/linux/fsnotify.h | 20 ++++++++++++++
> include/linux/fsnotify_backend.h | 42 ++++++++++++++++++++++++++++
> 6 files changed, 128 insertions(+), 10 deletions(-)
>
> diff --git a/fs/mount.h b/fs/mount.h
> index ffb613cdfeee..82aa3bad7cf5 100644
> --- a/fs/mount.h
> +++ b/fs/mount.h
> @@ -21,6 +21,10 @@ struct mnt_namespace {
> struct rcu_head mnt_ns_rcu;
> };
> u64 event;
> +#ifdef CONFIG_FSNOTIFY
> + __u32 n_fsnotify_mask;
> + struct fsnotify_mark_connector __rcu *n_fsnotify_marks;
> +#endif
> unsigned int nr_mounts; /* # of mounts in the namespace */
> unsigned int pending_mounts;
> struct rb_node mnt_ns_tree_node; /* node in the mnt_ns_tree */
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 8ee495a58d0a..c64b95cf50c7 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -28,6 +28,11 @@ void __fsnotify_vfsmount_delete(struct vfsmount *mnt)
> fsnotify_clear_marks_by_mount(mnt);
> }
>
> +void __fsnotify_mntns_delete(struct mnt_namespace *mntns)
> +{
> + fsnotify_clear_marks_by_mntns(mntns);
> +}
> +
> /**
> * fsnotify_unmount_inodes - an sb is unmounting. handle any watched inodes.
> * @sb: superblock being unmounted.
> @@ -420,7 +425,7 @@ static int send_to_group(__u32 mask, const void *data, int data_type,
> file_name, cookie, iter_info);
> }
>
> -static struct fsnotify_mark *fsnotify_first_mark(struct fsnotify_mark_connector **connp)
> +static struct fsnotify_mark *fsnotify_first_mark(struct fsnotify_mark_connector *const *connp)
> {
> struct fsnotify_mark_connector *conn;
> struct hlist_node *node = NULL;
> @@ -538,14 +543,15 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
> {
> const struct path *path = fsnotify_data_path(data, data_type);
> struct super_block *sb = fsnotify_data_sb(data, data_type);
> - struct fsnotify_sb_info *sbinfo = fsnotify_sb_info(sb);
> + const struct fsnotify_mnt *mnt_data = fsnotify_data_mnt(data, data_type);
> + struct fsnotify_sb_info *sbinfo = sb ? fsnotify_sb_info(sb) : NULL;
> struct fsnotify_iter_info iter_info = {};
> struct mount *mnt = NULL;
> struct inode *inode2 = NULL;
> struct dentry *moved;
> int inode2_type;
> int ret = 0;
> - __u32 test_mask, marks_mask;
> + __u32 test_mask, marks_mask = 0;
>
> if (path)
> mnt = real_mount(path->mnt);
> @@ -578,17 +584,20 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
> if ((!sbinfo || !sbinfo->sb_marks) &&
> (!mnt || !mnt->mnt_fsnotify_marks) &&
> (!inode || !inode->i_fsnotify_marks) &&
> - (!inode2 || !inode2->i_fsnotify_marks))
> + (!inode2 || !inode2->i_fsnotify_marks) &&
> + (!mnt_data || !mnt_data->ns->n_fsnotify_marks))
> return 0;
>
> - marks_mask = READ_ONCE(sb->s_fsnotify_mask);
> + if (sb)
> + marks_mask |= READ_ONCE(sb->s_fsnotify_mask);
> if (mnt)
> marks_mask |= READ_ONCE(mnt->mnt_fsnotify_mask);
> if (inode)
> marks_mask |= READ_ONCE(inode->i_fsnotify_mask);
> if (inode2)
> marks_mask |= READ_ONCE(inode2->i_fsnotify_mask);
> -
> + if (mnt_data)
> + marks_mask |= READ_ONCE(mnt_data->ns->n_fsnotify_mask);
>
> /*
> * If this is a modify event we may need to clear some ignore masks.
> @@ -618,6 +627,10 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
> iter_info.marks[inode2_type] =
> fsnotify_first_mark(&inode2->i_fsnotify_marks);
> }
> + if (mnt_data) {
> + iter_info.marks[FSNOTIFY_ITER_TYPE_MNTNS] =
> + fsnotify_first_mark(&mnt_data->ns->n_fsnotify_marks);
> + }
>
> /*
> * We need to merge inode/vfsmount/sb mark lists so that e.g. inode mark
> @@ -702,11 +715,31 @@ void file_set_fsnotify_mode(struct file *file)
> }
> #endif
>
> +void fsnotify_mnt(__u32 mask, struct mnt_namespace *ns, struct vfsmount *mnt)
> +{
> + struct fsnotify_mnt data = {
> + .ns = ns,
> + .mnt_id = real_mount(mnt)->mnt_id_unique,
> + };
> +
> + if (WARN_ON_ONCE(!ns))
> + return;
> +
> + /*
> + * This is an optimization as well as making sure fsnotify_init() has
> + * been called.
> + */
> + if (!ns->n_fsnotify_marks)
> + return;
> +
> + fsnotify(mask, &data, FSNOTIFY_EVENT_MNT, NULL, NULL, NULL, 0);
> +}
> +
> static __init int fsnotify_init(void)
> {
> int ret;
>
> - BUILD_BUG_ON(HWEIGHT32(ALL_FSNOTIFY_BITS) != 24);
> + BUILD_BUG_ON(HWEIGHT32(ALL_FSNOTIFY_BITS) != 26);
>
> ret = init_srcu_struct(&fsnotify_mark_srcu);
> if (ret)
> diff --git a/fs/notify/fsnotify.h b/fs/notify/fsnotify.h
> index 663759ed6fbc..5950c7a67f41 100644
> --- a/fs/notify/fsnotify.h
> +++ b/fs/notify/fsnotify.h
> @@ -33,6 +33,12 @@ static inline struct super_block *fsnotify_conn_sb(
> return conn->obj;
> }
>
> +static inline struct mnt_namespace *fsnotify_conn_mntns(
> + struct fsnotify_mark_connector *conn)
> +{
> + return conn->obj;
> +}
> +
> static inline struct super_block *fsnotify_object_sb(void *obj,
> enum fsnotify_obj_type obj_type)
> {
> @@ -89,6 +95,11 @@ static inline void fsnotify_clear_marks_by_sb(struct super_block *sb)
> fsnotify_destroy_marks(fsnotify_sb_marks(sb));
> }
>
> +static inline void fsnotify_clear_marks_by_mntns(struct mnt_namespace *mntns)
> +{
> + fsnotify_destroy_marks(&mntns->n_fsnotify_marks);
> +}
> +
> /*
> * update the dentry->d_flags of all of inode's children to indicate if inode cares
> * about events that happen to its children.
> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index 4981439e6209..798340db69d7 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -107,6 +107,8 @@ static fsnotify_connp_t *fsnotify_object_connp(void *obj,
> return &real_mount(obj)->mnt_fsnotify_marks;
> case FSNOTIFY_OBJ_TYPE_SB:
> return fsnotify_sb_marks(obj);
> + case FSNOTIFY_OBJ_TYPE_MNTNS:
> + return &((struct mnt_namespace *)obj)->n_fsnotify_marks;
> default:
> return NULL;
> }
> @@ -120,6 +122,8 @@ static __u32 *fsnotify_conn_mask_p(struct fsnotify_mark_connector *conn)
> return &fsnotify_conn_mount(conn)->mnt_fsnotify_mask;
> else if (conn->type == FSNOTIFY_OBJ_TYPE_SB)
> return &fsnotify_conn_sb(conn)->s_fsnotify_mask;
> + else if (conn->type == FSNOTIFY_OBJ_TYPE_MNTNS)
> + return &fsnotify_conn_mntns(conn)->n_fsnotify_mask;
> return NULL;
> }
>
> @@ -346,12 +350,15 @@ static void *fsnotify_detach_connector_from_object(
> fsnotify_conn_mount(conn)->mnt_fsnotify_mask = 0;
> } else if (conn->type == FSNOTIFY_OBJ_TYPE_SB) {
> fsnotify_conn_sb(conn)->s_fsnotify_mask = 0;
> + } else if (conn->type == FSNOTIFY_OBJ_TYPE_MNTNS) {
> + fsnotify_conn_mntns(conn)->n_fsnotify_mask = 0;
> }
>
> rcu_assign_pointer(*connp, NULL);
> conn->obj = NULL;
> conn->type = FSNOTIFY_OBJ_TYPE_DETACHED;
> - fsnotify_update_sb_watchers(sb, conn);
> + if (sb)
> + fsnotify_update_sb_watchers(sb, conn);
>
> return inode;
> }
> @@ -724,7 +731,7 @@ static int fsnotify_add_mark_list(struct fsnotify_mark *mark, void *obj,
> * Attach the sb info before attaching a connector to any object on sb.
> * The sb info will remain attached as long as sb lives.
> */
> - if (!fsnotify_sb_info(sb)) {
> + if (sb && !fsnotify_sb_info(sb)) {
> err = fsnotify_attach_info_to_sb(sb);
> if (err)
> return err;
> @@ -770,7 +777,8 @@ static int fsnotify_add_mark_list(struct fsnotify_mark *mark, void *obj,
> /* mark should be the last entry. last is the current last entry */
> hlist_add_behind_rcu(&mark->obj_list, &last->obj_list);
> added:
> - fsnotify_update_sb_watchers(sb, conn);
> + if (sb)
> + fsnotify_update_sb_watchers(sb, conn);
> /*
> * Since connector is attached to object using cmpxchg() we are
> * guaranteed that connector initialization is fully visible by anyone
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 1a9ef8f6784d..589e274adc7d 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -299,6 +299,11 @@ static inline void fsnotify_vfsmount_delete(struct vfsmount *mnt)
> __fsnotify_vfsmount_delete(mnt);
> }
>
> +static inline void fsnotify_mntns_delete(struct mnt_namespace *mntns)
> +{
> + __fsnotify_mntns_delete(mntns);
> +}
> +
> /*
> * fsnotify_inoderemove - an inode is going away
> */
> @@ -507,4 +512,19 @@ static inline int fsnotify_sb_error(struct super_block *sb, struct inode *inode,
> NULL, NULL, NULL, 0);
> }
>
> +static inline void fsnotify_mnt_attach(struct mnt_namespace *ns, struct vfsmount *mnt)
> +{
> + fsnotify_mnt(FS_MNT_ATTACH, ns, mnt);
> +}
> +
> +static inline void fsnotify_mnt_detach(struct mnt_namespace *ns, struct vfsmount *mnt)
> +{
> + fsnotify_mnt(FS_MNT_DETACH, ns, mnt);
> +}
> +
> +static inline void fsnotify_mnt_move(struct mnt_namespace *ns, struct vfsmount *mnt)
> +{
> + fsnotify_mnt(FS_MNT_MOVE, ns, mnt);
> +}
> +
> #endif /* _LINUX_FS_NOTIFY_H */
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index 0d24a21a8e60..6cd8d1d28b8b 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -59,6 +59,10 @@
>
> #define FS_PRE_ACCESS 0x00100000 /* Pre-content access hook */
>
> +#define FS_MNT_ATTACH 0x01000000 /* Mount was attached */
> +#define FS_MNT_DETACH 0x02000000 /* Mount was detached */
> +#define FS_MNT_MOVE (FS_MNT_ATTACH | FS_MNT_DETACH)
> +
> /*
> * Set on inode mark that cares about things that happen to its children.
> * Always set for dnotify and inotify.
> @@ -80,6 +84,9 @@
> */
> #define ALL_FSNOTIFY_DIRENT_EVENTS (FS_CREATE | FS_DELETE | FS_MOVE | FS_RENAME)
>
> +/* Mount namespace events */
> +#define FSNOTIFY_MNT_EVENTS (FS_MNT_ATTACH | FS_MNT_DETACH)
> +
> /* Content events can be used to inspect file content */
> #define FSNOTIFY_CONTENT_PERM_EVENTS (FS_OPEN_PERM | FS_OPEN_EXEC_PERM | \
> FS_ACCESS_PERM)
> @@ -108,6 +115,7 @@
>
> /* Events that can be reported to backends */
> #define ALL_FSNOTIFY_EVENTS (ALL_FSNOTIFY_DIRENT_EVENTS | \
> + FSNOTIFY_MNT_EVENTS | \
> FS_EVENTS_POSS_ON_CHILD | \
> FS_DELETE_SELF | FS_MOVE_SELF | \
> FS_UNMOUNT | FS_Q_OVERFLOW | FS_IN_IGNORED | \
> @@ -298,6 +306,7 @@ enum fsnotify_data_type {
> FSNOTIFY_EVENT_PATH,
> FSNOTIFY_EVENT_INODE,
> FSNOTIFY_EVENT_DENTRY,
> + FSNOTIFY_EVENT_MNT,
> FSNOTIFY_EVENT_ERROR,
> };
>
> @@ -318,6 +327,11 @@ static inline const struct path *file_range_path(const struct file_range *range)
> return range->path;
> }
>
> +struct fsnotify_mnt {
> + const struct mnt_namespace *ns;
> + u64 mnt_id;
> +};
> +
> static inline struct inode *fsnotify_data_inode(const void *data, int data_type)
> {
> switch (data_type) {
> @@ -383,6 +397,24 @@ static inline struct super_block *fsnotify_data_sb(const void *data,
> }
> }
>
> +static inline const struct fsnotify_mnt *fsnotify_data_mnt(const void *data,
> + int data_type)
> +{
> + switch (data_type) {
> + case FSNOTIFY_EVENT_MNT:
> + return data;
> + default:
> + return NULL;
> + }
> +}
> +
> +static inline u64 fsnotify_data_mnt_id(const void *data, int data_type)
> +{
> + const struct fsnotify_mnt *mnt_data = fsnotify_data_mnt(data, data_type);
> +
> + return mnt_data ? mnt_data->mnt_id : 0;
> +}
> +
> static inline struct fs_error_report *fsnotify_data_error_report(
> const void *data,
> int data_type)
> @@ -420,6 +452,7 @@ enum fsnotify_iter_type {
> FSNOTIFY_ITER_TYPE_SB,
> FSNOTIFY_ITER_TYPE_PARENT,
> FSNOTIFY_ITER_TYPE_INODE2,
> + FSNOTIFY_ITER_TYPE_MNTNS,
> FSNOTIFY_ITER_TYPE_COUNT
> };
>
> @@ -429,6 +462,7 @@ enum fsnotify_obj_type {
> FSNOTIFY_OBJ_TYPE_INODE,
> FSNOTIFY_OBJ_TYPE_VFSMOUNT,
> FSNOTIFY_OBJ_TYPE_SB,
> + FSNOTIFY_OBJ_TYPE_MNTNS,
> FSNOTIFY_OBJ_TYPE_COUNT,
> FSNOTIFY_OBJ_TYPE_DETACHED = FSNOTIFY_OBJ_TYPE_COUNT
> };
> @@ -613,8 +647,10 @@ extern int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data
> extern void __fsnotify_inode_delete(struct inode *inode);
> extern void __fsnotify_vfsmount_delete(struct vfsmount *mnt);
> extern void fsnotify_sb_delete(struct super_block *sb);
> +extern void __fsnotify_mntns_delete(struct mnt_namespace *mntns);
> extern void fsnotify_sb_free(struct super_block *sb);
> extern u32 fsnotify_get_cookie(void);
> +extern void fsnotify_mnt(__u32 mask, struct mnt_namespace *ns, struct vfsmount *mnt);
>
> static inline __u32 fsnotify_parent_needed_mask(__u32 mask)
> {
> @@ -928,6 +964,9 @@ static inline void __fsnotify_vfsmount_delete(struct vfsmount *mnt)
> static inline void fsnotify_sb_delete(struct super_block *sb)
> {}
>
> +static inline void __fsnotify_mntns_delete(struct mnt_namespace *mntns)
> +{}
> +
> static inline void fsnotify_sb_free(struct super_block *sb)
> {}
>
> @@ -942,6 +981,9 @@ static inline u32 fsnotify_get_cookie(void)
> static inline void fsnotify_unmount_inodes(struct super_block *sb)
> {}
>
> +static inline void fsnotify_mnt(__u32 mask, struct mnt_namespace *ns, struct vfsmount *mnt)
> +{}
> +
> #endif /* CONFIG_FSNOTIFY */
>
> #endif /* __KERNEL __ */
> --
> 2.48.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-01-29 16:58 ` [PATCH v5 2/3] fanotify: notify on mount attach and detach Miklos Szeredi
2025-01-30 21:05 ` Paul Moore
@ 2025-02-11 13:32 ` Jan Kara
2025-02-13 11:59 ` Miklos Szeredi
1 sibling, 1 reply; 19+ messages in thread
From: Jan Kara @ 2025-02-11 13:32 UTC (permalink / raw)
To: Miklos Szeredi
Cc: linux-fsdevel, Christian Brauner, Jan Kara, Amir Goldstein,
Karel Zak, Lennart Poettering, Ian Kent, Alexander Viro,
Paul Moore, selinux, linux-security-module, selinux-refpolicy
On Wed 29-01-25 17:58:00, Miklos Szeredi wrote:
> Add notifications for attaching and detaching mounts. The following new
> event masks are added:
>
> FAN_MNT_ATTACH - Mount was attached
> FAN_MNT_DETACH - Mount was detached
>
> If a mount is moved, then the event is reported with (FAN_MNT_ATTACH |
> FAN_MNT_DETACH).
>
> These events add an info record of type FAN_EVENT_INFO_TYPE_MNT containing
> these fields identifying the affected mounts:
>
> __u64 mnt_id - the ID of the mount (see statmount(2))
>
> FAN_REPORT_MNT must be supplied to fanotify_init() to receive these events
> and no other type of event can be received with this report type.
>
> Marks are added with FAN_MARK_MNTNS, which records the mount namespace from
> an nsfs file (e.g. /proc/self/ns/mnt).
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Just one small comment below. Otherwise feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
> @@ -1847,6 +1890,19 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
> return -EINVAL;
> group = fd_file(f)->private_data;
>
> + /* Only report mount events on mnt namespace */
> + if (FAN_GROUP_FLAG(group, FAN_REPORT_MNT)) {
> + if (mask & ~FANOTIFY_MOUNT_EVENTS)
> + return -EINVAL;
> + if (mark_type != FAN_MARK_MNTNS)
> + return -EINVAL;
> + } else {
> + if (mask & FANOTIFY_MOUNT_EVENTS)
> + return -EINVAL;
> + if (mark_type == FAN_MARK_MNTNS)
> + return -EINVAL;
> + }
> +
> /*
> * An unprivileged user is not allowed to setup mount nor filesystem
> * marks. This also includes setting up such marks by a group that
> @@ -1888,7 +1944,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
> * point.
> */
> fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
> - if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_EVENT_FLAGS) &&
> + if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_MOUNT_EVENTS|FANOTIFY_EVENT_FLAGS) &&
I understand why you need this but the condition is really hard to
understand now and the comment above it becomes out of date. Perhaps I'd
move this and the following two checks for FAN_RENAME and
FANOTIFY_PRE_CONTENT_EVENTS into !FAN_GROUP_FLAG(group, FAN_REPORT_MNT)
branch to make things more obvious?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-02-11 13:32 ` Jan Kara
@ 2025-02-13 11:59 ` Miklos Szeredi
2025-02-13 13:08 ` Amir Goldstein
0 siblings, 1 reply; 19+ messages in thread
From: Miklos Szeredi @ 2025-02-13 11:59 UTC (permalink / raw)
To: Jan Kara
Cc: Miklos Szeredi, linux-fsdevel, Christian Brauner, Amir Goldstein,
Karel Zak, Lennart Poettering, Ian Kent, Alexander Viro,
Paul Moore, selinux, linux-security-module, selinux-refpolicy
On Tue, 11 Feb 2025 at 16:50, Jan Kara <jack@suse.cz> wrote:
>
> On Wed 29-01-25 17:58:00, Miklos Szeredi wrote:
> > fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
> > - if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_EVENT_FLAGS) &&
> > + if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_MOUNT_EVENTS|FANOTIFY_EVENT_FLAGS) &&
>
> I understand why you need this but the condition is really hard to
> understand now and the comment above it becomes out of date. Perhaps I'd
> move this and the following two checks for FAN_RENAME and
> FANOTIFY_PRE_CONTENT_EVENTS into !FAN_GROUP_FLAG(group, FAN_REPORT_MNT)
> branch to make things more obvious?
Okay. git diff -w below.
Thanks,
Miklos
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1936,6 +1936,8 @@ static int do_fanotify_mark(int fanotify_fd,
unsigned int flags, __u64 mask,
mark_type != FAN_MARK_INODE)
return -EINVAL;
+ /* The following checks are not relevant to mount events */
+ if (!FAN_GROUP_FLAG(group, FAN_REPORT_MNT)) {
/*
* Events that do not carry enough information to report
* event->fd require a group that supports reporting fid. Those
@@ -1944,21 +1946,25 @@ static int do_fanotify_mark(int fanotify_fd,
unsigned int flags, __u64 mask,
* point.
*/
fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
- if (mask &
~(FANOTIFY_FD_EVENTS|FANOTIFY_MOUNT_EVENTS|FANOTIFY_EVENT_FLAGS) &&
+ if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_EVENT_FLAGS) &&
(!fid_mode || mark_type == FAN_MARK_MOUNT))
return -EINVAL;
/*
- * FAN_RENAME uses special info type records to report the old and
- * new parent+name. Reporting only old and new parent id is less
- * useful and was not implemented.
+ * FAN_RENAME uses special info type records to report the old
+ * and new parent+name. Reporting only old and new parent id is
+ * less useful and was not implemented.
*/
if (mask & FAN_RENAME && !(fid_mode & FAN_REPORT_NAME))
return -EINVAL;
- /* Pre-content events are not currently generated for directories. */
+ /*
+ * 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) {
if (mark_type == FAN_MARK_MOUNT)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v5 2/3] fanotify: notify on mount attach and detach
2025-02-13 11:59 ` Miklos Szeredi
@ 2025-02-13 13:08 ` Amir Goldstein
0 siblings, 0 replies; 19+ messages in thread
From: Amir Goldstein @ 2025-02-13 13:08 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Jan Kara, Miklos Szeredi, linux-fsdevel, Christian Brauner,
Karel Zak, Lennart Poettering, Ian Kent, Alexander Viro,
Paul Moore, selinux, linux-security-module, selinux-refpolicy
On Thu, Feb 13, 2025 at 1:00 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 11 Feb 2025 at 16:50, Jan Kara <jack@suse.cz> wrote:
> >
> > On Wed 29-01-25 17:58:00, Miklos Szeredi wrote:
>
> > > fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
> > > - if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_EVENT_FLAGS) &&
> > > + if (mask & ~(FANOTIFY_FD_EVENTS|FANOTIFY_MOUNT_EVENTS|FANOTIFY_EVENT_FLAGS) &&
> >
> > I understand why you need this but the condition is really hard to
> > understand now and the comment above it becomes out of date. Perhaps I'd
> > move this and the following two checks for FAN_RENAME and
> > FANOTIFY_PRE_CONTENT_EVENTS into !FAN_GROUP_FLAG(group, FAN_REPORT_MNT)
> > branch to make things more obvious?
>
> Okay. git diff -w below.
>
> Thanks,
> Miklos
>
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1936,6 +1936,8 @@ static int do_fanotify_mark(int fanotify_fd,
> unsigned int flags, __u64 mask,
> mark_type != FAN_MARK_INODE)
> return -EINVAL;
>
> + /* The following checks are not relevant to mount events */
> + if (!FAN_GROUP_FLAG(group, FAN_REPORT_MNT)) {
Sorry for nit picking, but you already have this !FAN_REPORT_MNT
branch above:
+ /* Only report mount events on mnt namespace */
+ if (FAN_GROUP_FLAG(group, FAN_REPORT_MNT)) {
+ if (mask & ~FANOTIFY_MOUNT_EVENTS)
+ return -EINVAL;
...
+ } else {
+ if (mask & FANOTIFY_MOUNT_EVENTS)
Which can be easily moved down here and then we get in one place:
if (FAN_REPORT_MNT) {
/* event rules for FAN_REPORT_MNT */
} else {
/* event rules for !FAN_REPORT_MNT */
}
TBH, with the check for (mask & ~FANOTIFY_MOUNT_EVENTS)
I personally wouldn't mind leaving checks for FAN_RENAME and
FANOTIFY_PRE_CONTENT_EVENTS outside of the else branch,
but I don't have a strong objection to including them in the else branch.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2025-02-13 13:08 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-29 16:57 [PATCH v5 0/3] mount notification Miklos Szeredi
2025-01-29 16:57 ` [PATCH v5 1/3] fsnotify: add mount notification infrastructure Miklos Szeredi
2025-02-11 13:05 ` Jan Kara
2025-01-29 16:58 ` [PATCH v5 2/3] fanotify: notify on mount attach and detach Miklos Szeredi
2025-01-30 21:05 ` Paul Moore
2025-01-31 10:53 ` Miklos Szeredi
2025-01-31 14:28 ` Paul Moore
2025-02-04 10:19 ` Christian Brauner
2025-02-04 10:20 ` Christian Brauner
2025-01-31 12:09 ` Christian Brauner
2025-01-31 14:39 ` Paul Moore
2025-02-04 10:07 ` Christian Brauner
2025-02-04 23:52 ` Paul Moore
2025-02-11 13:32 ` Jan Kara
2025-02-13 11:59 ` Miklos Szeredi
2025-02-13 13:08 ` Amir Goldstein
2025-01-29 16:58 ` [PATCH v5 3/3] vfs: add notifications for " Miklos Szeredi
2025-02-11 13:04 ` Jan Kara
2025-01-30 16:07 ` [PATCH v5 0/3] mount notification Christian Brauner
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).