[RFC][PATCH 0/7] fanotify: add support for more events

[RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

Hi all,

This series is a prep work for using fanotify to monitor all
events in a file system with a single watch.

The end result is indented to be an alternative to the recursive
inotify watches scheme, which has its problems.

This first part adds support for most inotify events to fanotify
when watching a directory.

The next part will add support for watching a super block,
which is not the same as watching a mount point.

I am posting this WIP to get feedback on the idea and to find
out if there are any users out there interested in the improved
fanotify capabilities and/or in the super block monitoring
use case.

Amir Goldstein (7):
  fsnotify: pass dentry instead of inode when available
  fsnotify: annotate filename events
  fanotify: new init flag FAN_EVENT_INFO_PARENT
  fanotify: store mount point from which an inode watch was added
  fanotify: support events with data type FSNOTIFY_EVENT_DENTRY
  fanotify: add support for create/attrib/rename/delete events
  fanotify: pass filename info for filename events

 fs/notify/fanotify/fanotify.c      | 85 +++++++++++++++++++++++++++++++----
 fs/notify/fanotify/fanotify.h      | 24 +++++++++-
 fs/notify/fanotify/fanotify_user.c | 92 ++++++++++++++++++++++++++++++++++----
 fs/notify/fdinfo.c                 |  4 +-
 fs/notify/fsnotify.c               |  2 +-
 fs/notify/inode_mark.c             |  1 +
 fs/notify/mark.c                   | 15 +++++--
 include/linux/fsnotify.h           | 46 ++++++++++++++-----
 include/linux/fsnotify_backend.h   | 24 +++++++---
 include/uapi/linux/fanotify.h      | 41 ++++++++++++++---
 10 files changed, 287 insertions(+), 47 deletions(-)

-- 
2.7.4

[RFC][PATCH 1/7] fsnotify: pass dentry instead of inode when available

[Amir Goldstein]

Define a new data type to pass for event FSNOTIFY_EVENT_DENTRY
and use it whenever a dentry is available instead of passing
it's ->d_inode as data type FSNOTIFY_EVENT_INODE.

None of the current fsnotify backends make use of the inode data
with data type FSNOTIFY_EVENT_INODE - only the data of type
FSNOTIFY_EVENT_PATH is ever used, so this change has no immediate
consequences.

Soon, we are going to use the dentry data type to support more
events with fanotify backcend.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fsnotify.c             |  2 +-
 include/linux/fsnotify.h         | 15 +++++++--------
 include/linux/fsnotify_backend.h |  3 ++-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index db39de2..3ba0e4a 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -112,7 +112,7 @@ int __fsnotify_parent(struct path *path, struct dentry *dentry, __u32 mask)
 			ret = fsnotify(p_inode, mask, path, FSNOTIFY_EVENT_PATH,
 				       dentry->d_name.name, 0);
 		else
-			ret = fsnotify(p_inode, mask, dentry->d_inode, FSNOTIFY_EVENT_INODE,
+			ret = fsnotify(p_inode, mask, dentry, FSNOTIFY_EVENT_DENTRY,
 				       dentry->d_name.name, 0);
 	}
 
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index eed9e85..4cd3101 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -82,14 +82,14 @@ static inline void fsnotify_move(struct inode *old_dir, struct inode *new_dir,
 
 	fsnotify(old_dir, old_dir_mask, source, FSNOTIFY_EVENT_INODE, old_name,
 		 fs_cookie);
-	fsnotify(new_dir, new_dir_mask, source, FSNOTIFY_EVENT_INODE, new_name,
+	fsnotify(new_dir, new_dir_mask, moved, FSNOTIFY_EVENT_DENTRY, new_name,
 		 fs_cookie);
 
 	if (target)
 		fsnotify_link_count(target);
 
 	if (source)
-		fsnotify(source, FS_MOVE_SELF, moved->d_inode, FSNOTIFY_EVENT_INODE, NULL, 0);
+		fsnotify(source, FS_MOVE_SELF, moved, FSNOTIFY_EVENT_DENTRY, NULL, 0);
 	audit_inode_child(new_dir, moved, AUDIT_TYPE_CHILD_CREATE);
 }
 
@@ -138,7 +138,7 @@ static inline void fsnotify_create(struct inode *inode, struct dentry *dentry)
 {
 	audit_inode_child(inode, dentry, AUDIT_TYPE_CHILD_CREATE);
 
-	fsnotify(inode, FS_CREATE, dentry->d_inode, FSNOTIFY_EVENT_INODE, dentry->d_name.name, 0);
+	fsnotify(inode, FS_CREATE, dentry, FSNOTIFY_EVENT_DENTRY, dentry->d_name.name, 0);
 }
 
 /*
@@ -151,7 +151,7 @@ static inline void fsnotify_link(struct inode *dir, struct inode *inode, struct
 	fsnotify_link_count(inode);
 	audit_inode_child(dir, new_dentry, AUDIT_TYPE_CHILD_CREATE);
 
-	fsnotify(dir, FS_CREATE, inode, FSNOTIFY_EVENT_INODE, new_dentry->d_name.name, 0);
+	fsnotify(dir, FS_CREATE, new_dentry, FSNOTIFY_EVENT_DENTRY, new_dentry->d_name.name, 0);
 }
 
 /*
@@ -160,11 +160,10 @@ static inline void fsnotify_link(struct inode *dir, struct inode *inode, struct
 static inline void fsnotify_mkdir(struct inode *inode, struct dentry *dentry)
 {
 	__u32 mask = (FS_CREATE | FS_ISDIR);
-	struct inode *d_inode = dentry->d_inode;
 
 	audit_inode_child(inode, dentry, AUDIT_TYPE_CHILD_CREATE);
 
-	fsnotify(inode, mask, d_inode, FSNOTIFY_EVENT_INODE, dentry->d_name.name, 0);
+	fsnotify(inode, mask, dentry, FSNOTIFY_EVENT_DENTRY, dentry->d_name.name, 0);
 }
 
 /*
@@ -250,7 +249,7 @@ static inline void fsnotify_xattr(struct dentry *dentry)
 		mask |= FS_ISDIR;
 
 	fsnotify_parent(NULL, dentry, mask);
-	fsnotify(inode, mask, inode, FSNOTIFY_EVENT_INODE, NULL, 0);
+	fsnotify(inode, mask, dentry, FSNOTIFY_EVENT_DENTRY, NULL, 0);
 }
 
 /*
@@ -285,7 +284,7 @@ static inline void fsnotify_change(struct dentry *dentry, unsigned int ia_valid)
 			mask |= FS_ISDIR;
 
 		fsnotify_parent(NULL, dentry, mask);
-		fsnotify(inode, mask, inode, FSNOTIFY_EVENT_INODE, NULL, 0);
+		fsnotify(inode, mask, dentry, FSNOTIFY_EVENT_DENTRY, NULL, 0);
 	}
 }
 
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 58205f3..3830d5a 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -189,10 +189,11 @@ struct fsnotify_group {
 	};
 };
 
-/* when calling fsnotify tell it if the data is a path or inode */
+/* when calling fsnotify tell it if the data is a path or inode or dentry */
 #define FSNOTIFY_EVENT_NONE	0
 #define FSNOTIFY_EVENT_PATH	1
 #define FSNOTIFY_EVENT_INODE	2
+#define FSNOTIFY_EVENT_DENTRY	3
 
 /*
  * A mark is simply an object attached to an in core inode which allows an
-- 
2.7.4

[RFC][PATCH 2/7] fsnotify: annotate filename events

[Amir Goldstein]

Filename events are referring to events that modify directory entries,
such as create,delete,rename. Those events should always be reported
on a watched directory, regardless if FS_EVENT_ON_CHILD is set
on the watch mask.

fsnotify_nameremove() and fsnotify_move() were modified to no longer
set the FS_EVENT_ON_CHILD event bit. This is a semantic change to
align with the filename event definition. It has no effect on any
existing backend, because dnotify and inotify always requets the
child events and fanotify does not get the delete,rename events.

The fsnotify_filename() helper is used to report all the filename
events. It gets a reference on parent dentry and passes it as the
data for the event along with the filename.

fsnotify_filename() is different from fsnotify_parent().
fsnotify_parent() is intended to report any events that happened on
child inodes when FS_EVENT_ON_CHILD is requested.
fsnotify_filename() is intended to report only filename events,
such as create,mkdir,link. Those events must always be reported
on a watched directory, regardless if FS_EVENT_ON_CHILD was requested.

fsnotify_d_name() is a helper for the common case where the
filename to pass is dentry->d_name.name.

It is safe to use these helpers with negative or not instantiated
dentries, such as the case with fsnotify_link() and
fsnotify_nameremove().

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 include/linux/fsnotify.h | 39 +++++++++++++++++++++++++++++++--------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 4cd3101..85dd120 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -25,6 +25,30 @@ static inline int fsnotify_parent(struct path *path, struct dentry *dentry, __u3
 	return __fsnotify_parent(path, dentry, mask);
 }
 
+/*
+ * Notify this dentry's parent about a filename event (create,delete,rename).
+ * Unlike fsnotify_parent(), the event will be reported regardless of the
+ * FS_EVENT_ON_CHILD mask on the parent inode
+ */
+static inline int fsnotify_filename(struct dentry *dentry, __u32 mask,
+				    const unsigned char *file_name, u32 cookie)
+{
+	struct dentry *parent = dget_parent(dentry);
+	struct inode *p_inode = parent->d_inode;
+	int ret;
+
+	ret = fsnotify(p_inode, mask, parent, FSNOTIFY_EVENT_DENTRY,
+		       file_name, cookie);
+
+	dput(parent);
+	return ret;
+}
+
+static inline int fsnotify_d_name(struct dentry *dentry, __u32 mask)
+{
+	return fsnotify_filename(dentry, mask, dentry->d_name.name, 0);
+}
+
 /* simple call site for access decisions */
 static inline int fsnotify_perm(struct file *file, int mask)
 {
@@ -68,8 +92,8 @@ static inline void fsnotify_move(struct inode *old_dir, struct inode *new_dir,
 {
 	struct inode *source = moved->d_inode;
 	u32 fs_cookie = fsnotify_get_cookie();
-	__u32 old_dir_mask = (FS_EVENT_ON_CHILD | FS_MOVED_FROM);
-	__u32 new_dir_mask = (FS_EVENT_ON_CHILD | FS_MOVED_TO);
+	__u32 old_dir_mask = FS_MOVED_FROM;
+	__u32 new_dir_mask = FS_MOVED_TO;
 	const unsigned char *new_name = moved->d_name.name;
 
 	if (old_dir == new_dir)
@@ -82,8 +106,7 @@ static inline void fsnotify_move(struct inode *old_dir, struct inode *new_dir,
 
 	fsnotify(old_dir, old_dir_mask, source, FSNOTIFY_EVENT_INODE, old_name,
 		 fs_cookie);
-	fsnotify(new_dir, new_dir_mask, moved, FSNOTIFY_EVENT_DENTRY, new_name,
-		 fs_cookie);
+	fsnotify_filename(moved, new_dir_mask, new_name, fs_cookie);
 
 	if (target)
 		fsnotify_link_count(target);
@@ -119,7 +142,7 @@ static inline void fsnotify_nameremove(struct dentry *dentry, int isdir)
 	if (isdir)
 		mask |= FS_ISDIR;
 
-	fsnotify_parent(NULL, dentry, mask);
+	fsnotify_d_name(dentry, mask);
 }
 
 /*
@@ -138,7 +161,7 @@ static inline void fsnotify_create(struct inode *inode, struct dentry *dentry)
 {
 	audit_inode_child(inode, dentry, AUDIT_TYPE_CHILD_CREATE);
 
-	fsnotify(inode, FS_CREATE, dentry, FSNOTIFY_EVENT_DENTRY, dentry->d_name.name, 0);
+	fsnotify_d_name(dentry, FS_CREATE);
 }
 
 /*
@@ -151,7 +174,7 @@ static inline void fsnotify_link(struct inode *dir, struct inode *inode, struct
 	fsnotify_link_count(inode);
 	audit_inode_child(dir, new_dentry, AUDIT_TYPE_CHILD_CREATE);
 
-	fsnotify(dir, FS_CREATE, new_dentry, FSNOTIFY_EVENT_DENTRY, new_dentry->d_name.name, 0);
+	fsnotify_d_name(new_dentry, FS_CREATE);
 }
 
 /*
@@ -163,7 +186,7 @@ static inline void fsnotify_mkdir(struct inode *inode, struct dentry *dentry)
 
 	audit_inode_child(inode, dentry, AUDIT_TYPE_CHILD_CREATE);
 
-	fsnotify(inode, mask, dentry, FSNOTIFY_EVENT_DENTRY, dentry->d_name.name, 0);
+	fsnotify_d_name(dentry, mask);
 }
 
 /*
-- 
2.7.4

[RFC][PATCH 3/7] fanotify: new init flag FAN_EVENT_INFO_PARENT

[Amir Goldstein]

The new init flag FAN_EVENT_INFO_PARENT indicates that the
event fd may point to the parent, depending on the event.

FAN_EVENT_INFO_PARENT will also be used to add support for more
events, so it is stored in a new 'flags' field in the group's
private data for future use.

FAN_EVENT_INFO_PARENT can only be set along with the default
notification class.

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

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 8e8e6bc..5f038f4 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -710,6 +710,14 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	if (event_f_flags & ~FANOTIFY_INIT_ALL_EVENT_F_BITS)
 		return -EINVAL;
 
+	/*
+	 * New event format info bits can only be set for the default
+	 * notification class
+	 */
+	if ((flags & FAN_ALL_EVENT_INFO_BITS) &&
+	    (flags & FAN_ALL_CLASS_BITS) != FAN_CLASS_NOTIF)
+		return -EINVAL;
+
 	switch (event_f_flags & O_ACCMODE) {
 	case O_RDONLY:
 	case O_RDWR:
@@ -794,6 +802,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	if (fd < 0)
 		goto out_destroy_group;
 
+	group->fanotify_data.flags = flags;
 	return fd;
 
 out_destroy_group:
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 3830d5a..5722e4f 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -181,7 +181,8 @@ struct fsnotify_group {
 			wait_queue_head_t access_waitq;
 			atomic_t bypass_perm;
 #endif /* CONFIG_FANOTIFY_ACCESS_PERMISSIONS */
-			int f_flags;
+			int flags;           /* flags from fanotify_init() */
+			int f_flags; /* event_f_flags from fanotify_init() */
 			unsigned int max_marks;
 			struct user_struct *user;
 		} fanotify_data;
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index 030508d..8e58765 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -36,9 +36,14 @@
 #define FAN_UNLIMITED_QUEUE	0x00000010
 #define FAN_UNLIMITED_MARKS	0x00000020
 
+/* These bits determine the format of the reported events */
+#define FAN_EVENT_INFO_PARENT	0x00000100	/* Event fd maybe of parent */
+#define FAN_ALL_EVENT_INFO_BITS (FAN_EVENT_INFO_PARENT)
+
 #define FAN_ALL_INIT_FLAGS	(FAN_CLOEXEC | FAN_NONBLOCK | \
-				 FAN_ALL_CLASS_BITS | FAN_UNLIMITED_QUEUE |\
-				 FAN_UNLIMITED_MARKS)
+				 FAN_ALL_CLASS_BITS | \
+				 FAN_ALL_EVENT_INFO_BITS | \
+				 FAN_UNLIMITED_QUEUE | FAN_UNLIMITED_MARKS)
 
 /* flags used for fanotify_modify_mark() */
 #define FAN_MARK_ADD		0x00000001
-- 
2.7.4

[RFC][PATCH 4/7] fanotify: store mount point from which an inode watch was added

[Amir Goldstein]

When adding an inode watch, and when FAN_EVENT_INFO_PARENT flag
was requested at fanotify_init(), store the mount point from which
the watch was added.

Events reported for this inode mark may report the path of this
mount point, even if the events actually happened on this inode,
or its children, but from another mount point.

An inode mark needs to have both a reference to the watched
inode and a reference to the original mount point.
For this purpose, the fsnotify_mark fields .inode and .mnt are now
independent memebers and not a union. This has a side effect of extending
the size of struct fsnotify_mark by another pointer, but this effect
is insignificant compared to the fact that every inode mark pins a whole
struct inode.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify_user.c | 14 +++++++++++---
 fs/notify/fdinfo.c                 |  4 ++--
 fs/notify/inode_mark.c             |  1 +
 fs/notify/mark.c                   | 15 ++++++++++++---
 include/linux/fsnotify_backend.h   | 18 ++++++++++++++----
 5 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 5f038f4..0696d46 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -653,6 +653,7 @@ static int fanotify_add_vfsmount_mark(struct fsnotify_group *group,
 }
 
 static int fanotify_add_inode_mark(struct fsnotify_group *group,
+				   struct vfsmount *mnt,
 				   struct inode *inode, __u32 mask,
 				   unsigned int flags)
 {
@@ -674,7 +675,7 @@ static int fanotify_add_inode_mark(struct fsnotify_group *group,
 	mutex_lock(&group->mark_mutex);
 	fsn_mark = fsnotify_find_inode_mark(group, inode);
 	if (!fsn_mark) {
-		fsn_mark = fanotify_add_new_mark(group, inode, NULL);
+		fsn_mark = fanotify_add_new_mark(group, inode, mnt);
 		if (IS_ERR(fsn_mark)) {
 			mutex_unlock(&group->mark_mutex);
 			return PTR_ERR(fsn_mark);
@@ -891,7 +892,14 @@ SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
 	/* inode held in place by reference to path; group by fget on fd */
 	if (!(flags & FAN_MARK_MOUNT))
 		inode = path.dentry->d_inode;
-	else
+
+	/*
+	 * Store the mount point from which the watch was added also for
+	 * inode marks. This mount point may be used to report dentry events,
+	 * even if the events happened on another mount point.
+	 */
+	if ((flags & FAN_MARK_MOUNT) ||
+	    group->fanotify_data.flags & FAN_EVENT_INFO_PARENT)
 		mnt = path.mnt;
 
 	/* create/update an inode mark */
@@ -900,7 +908,7 @@ SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
 		if (flags & FAN_MARK_MOUNT)
 			ret = fanotify_add_vfsmount_mark(group, mnt, mask, flags);
 		else
-			ret = fanotify_add_inode_mark(group, inode, mask, flags);
+			ret = fanotify_add_inode_mark(group, mnt, inode, mask, flags);
 		break;
 	case FAN_MARK_REMOVE:
 		if (flags & FAN_MARK_MOUNT)
diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
index fd98e51..6dd776b 100644
--- a/fs/notify/fdinfo.c
+++ b/fs/notify/fdinfo.c
@@ -119,7 +119,7 @@ static void fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
 	if (mark->flags & FSNOTIFY_MARK_FLAG_IGNORED_SURV_MODIFY)
 		mflags |= FAN_MARK_IGNORED_SURV_MODIFY;
 
-	if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) {
+	if (FSNOTIFY_IS_INODE_MARK(mark)) {
 		inode = igrab(mark->inode);
 		if (!inode)
 			return;
@@ -129,7 +129,7 @@ static void fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
 		show_mark_fhandle(m, inode);
 		seq_putc(m, '\n');
 		iput(inode);
-	} else if (mark->flags & FSNOTIFY_MARK_FLAG_VFSMOUNT) {
+	} else if (FSNOTIFY_IS_VFSMOUNT_MARK(mark)) {
 		struct mount *mnt = real_mount(mark->mnt);
 
 		seq_printf(m, "fanotify mnt_id:%x mflags:%x mask:%x ignored_mask:%x\n",
diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c
index 741077d..2471229 100644
--- a/fs/notify/inode_mark.c
+++ b/fs/notify/inode_mark.c
@@ -54,6 +54,7 @@ void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark)
 
 	hlist_del_init_rcu(&mark->obj_list);
 	mark->inode = NULL;
+	mark->mnt = NULL;
 
 	/*
 	 * this mark is now off the inode->i_fsnotify_marks list and we
diff --git a/fs/notify/mark.c b/fs/notify/mark.c
index d3fea0b..2ebabb4 100644
--- a/fs/notify/mark.c
+++ b/fs/notify/mark.c
@@ -148,10 +148,10 @@ void fsnotify_detach_mark(struct fsnotify_mark *mark)
 
 	mark->flags &= ~FSNOTIFY_MARK_FLAG_ATTACHED;
 
-	if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) {
+	if (FSNOTIFY_IS_INODE_MARK(mark)) {
 		inode = mark->inode;
 		fsnotify_destroy_inode_mark(mark);
-	} else if (mark->flags & FSNOTIFY_MARK_FLAG_VFSMOUNT)
+	} else if (FSNOTIFY_IS_VFSMOUNT_MARK(mark))
 		fsnotify_destroy_vfsmount_mark(mark);
 	else
 		BUG();
@@ -352,6 +352,11 @@ int fsnotify_add_mark_list(struct hlist_head *head, struct fsnotify_mark *mark,
  * Attach an initialized mark to a given group and fs object.
  * These marks may be used for the fsnotify backend to determine which
  * event types should be delivered to which group.
+ *
+ * Either @inode or @mnt MUST be non NULL.
+ * @inode may be NULL implying this is a mount watch.
+ * @inode and @mnt both non NULL imply this is an inode watch
+ * and @mnt is the mount point from which the watch was added.
  */
 int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
 			     struct fsnotify_group *group, struct inode *inode,
@@ -359,7 +364,6 @@ int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
 {
 	int ret = 0;
 
-	BUG_ON(inode && mnt);
 	BUG_ON(!inode && !mnt);
 	BUG_ON(!mutex_is_locked(&group->mark_mutex));
 
@@ -382,6 +386,11 @@ int fsnotify_add_mark_locked(struct fsnotify_mark *mark,
 		ret = fsnotify_add_inode_mark(mark, group, inode, allow_dups);
 		if (ret)
 			goto err;
+		/* Store the mount point from which the inode watch was added */
+		if (mnt) {
+			mark->mnt = mnt;
+			mark->flags |= FSNOTIFY_MARK_FLAG_VFSMOUNT;
+		}
 	} else if (mnt) {
 		ret = fsnotify_add_vfsmount_mark(mark, group, mnt, allow_dups);
 		if (ret)
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 5722e4f..c36e8e5 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -227,10 +227,9 @@ struct fsnotify_mark {
 	spinlock_t lock;
 	/* List of marks for inode / vfsmount [obj_lock] */
 	struct hlist_node obj_list;
-	union {	/* Object pointer [mark->lock, group->mark_mutex] */
-		struct inode *inode;	/* inode this mark is associated with */
-		struct vfsmount *mnt;	/* vfsmount this mark is associated with */
-	};
+	/* Object pointer [mark->lock, group->mark_mutex] */
+	struct inode *inode;	/* inode this mark is associated with */
+	struct vfsmount *mnt;	/* vfsmount this mark is associated with */
 	/* Events types to ignore [mark->lock, group->mark_mutex] */
 	__u32 ignored_mask;
 #define FSNOTIFY_MARK_FLAG_INODE		0x01
@@ -243,6 +242,17 @@ struct fsnotify_mark {
 	void (*free_mark)(struct fsnotify_mark *mark); /* called on final put+free */
 };
 
+/*
+ * A mark may have a reference to both an inode and a vfsmount,
+ * indicating that this is an inode mark with a reference to
+ * the mount point from which that inode mark was added
+ */
+#define FSNOTIFY_MARK_HAS_INODE(m)    ((m)->flags & FSNOTIFY_MARK_FLAG_INODE)
+#define FSNOTIFY_MARK_HAS_VFSMOUNT(m) ((m)->flags & FSNOTIFY_MARK_FLAG_VFSMOUNT)
+#define FSNOTIFY_IS_INODE_MARK(m)     (FSNOTIFY_MARK_HAS_INODE(m))
+#define FSNOTIFY_IS_VFSMOUNT_MARK(m)  (!FSNOTIFY_MARK_HAS_INODE(m) && \
+					FSNOTIFY_MARK_HAS_VFSMOUNT(m))
+
 #ifdef CONFIG_FSNOTIFY
 
 /* called from the vfs helpers */
-- 
2.7.4

[RFC][PATCH 5/7] fanotify: support events with data type FSNOTIFY_EVENT_DENTRY

[Amir Goldstein]

When event data type is FSNOTIFY_EVENT_DENTRY, store the
provided dentry in the event queue along with the mount point
that was used to setup the watch.

This will enable the watcher to read the path of the passed
dentry from /proc/self/fd/X, for example on FAN_ATTRIB events.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify.c      | 39 +++++++++++++++++++++++++++++++-------
 fs/notify/fanotify/fanotify_user.c |  9 +++++++--
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index d2f97ec..fb2b852 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -105,18 +105,28 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
 {
 	__u32 marks_mask, marks_ignored_mask;
 	struct path *path = data;
+	struct dentry *dentry;
 
 	pr_debug("%s: inode_mark=%p vfsmnt_mark=%p mask=%x data=%p"
 		 " data_type=%d\n", __func__, inode_mark, vfsmnt_mark,
 		 event_mask, data, data_type);
 
 	/* if we don't have enough info to send an event to userspace say no */
-	if (data_type != FSNOTIFY_EVENT_PATH)
+	if (data_type != FSNOTIFY_EVENT_PATH &&
+	    data_type != FSNOTIFY_EVENT_DENTRY)
 		return false;
 
-	/* sorry, fanotify only gives a damn about files and dirs */
-	if (!d_is_reg(path->dentry) &&
-	    !d_can_lookup(path->dentry))
+	dentry = path->dentry;
+	pr_debug("%s: dentry=%p d_flags=%x inode=%p\n",  __func__,
+		 dentry, dentry->d_flags, dentry->d_inode);
+
+	if (d_is_negative(dentry) || d_really_is_negative(dentry))
+		return false;
+
+	/* sorry, fanotify only gives a damn about files and dirs
+	 * FIXME: can this be removed? */
+	if (data_type == FSNOTIFY_EVENT_PATH &&
+	    !d_is_reg(dentry) && !d_can_lookup(dentry))
 		return false;
 
 	if (inode_mark && vfsmnt_mark) {
@@ -139,7 +149,7 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
 		BUG();
 	}
 
-	if (d_is_dir(path->dentry) &&
+	if (d_is_dir(dentry) &&
 	    !(marks_mask & FS_ISDIR & ~marks_ignored_mask))
 		return false;
 
@@ -177,6 +187,10 @@ init: __maybe_unused
 	if (path) {
 		event->path = *path;
 		path_get(&event->path);
+		pr_debug("%s: mnt=%p, dentry=%p parent=%p d_flags=%x\n",
+				__func__, event->path.mnt, event->path.dentry,
+				event->path.dentry->d_parent,
+				event->path.dentry->d_flags);
 	} else {
 		event->path.mnt = NULL;
 		event->path.dentry = NULL;
@@ -194,6 +208,7 @@ static int fanotify_handle_event(struct fsnotify_group *group,
 	int ret = 0;
 	struct fanotify_event_info *event;
 	struct fsnotify_event *fsn_event;
+	struct path path;
 
 	BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);
 	BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);
@@ -206,14 +221,24 @@ static int fanotify_handle_event(struct fsnotify_group *group,
 	BUILD_BUG_ON(FAN_ACCESS_PERM != FS_ACCESS_PERM);
 	BUILD_BUG_ON(FAN_ONDIR != FS_ISDIR);
 
-	if (!fanotify_should_send_event(inode_mark, fanotify_mark, mask, data,
+	if (data_type == FSNOTIFY_EVENT_PATH) {
+		path = *(struct path *)data;
+	} else if (data_type == FSNOTIFY_EVENT_DENTRY) {
+		path.dentry = (struct dentry *)data;
+		path.mnt = (inode_mark ? inode_mark->mnt : fanotify_mark->mnt);
+	} else {
+		path.mnt = NULL;
+		path.dentry = NULL;
+	}
+
+	if (!fanotify_should_send_event(inode_mark, fanotify_mark, mask, &path,
 					data_type))
 		return 0;
 
 	pr_debug("%s: group=%p inode=%p mask=%x\n", __func__, group, inode,
 		 mask);
 
-	event = fanotify_alloc_event(inode, mask, data);
+	event = fanotify_alloc_event(inode, mask, &path);
 	if (unlikely(!event))
 		return -ENOMEM;
 
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 0696d46..789962c 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -88,12 +88,17 @@ static int create_fd(struct fsnotify_group *group,
 	 */
 	/* it's possible this event was an overflow event.  in that case dentry and mnt
 	 * are NULL;  That's fine, just don't call dentry open */
-	if (event->path.dentry && event->path.mnt)
+	if (event->path.dentry && event->path.mnt) {
+		pr_debug("%s: mnt=%p, dentry=%p parent=%p d_flags=%x\n",
+				__func__, event->path.mnt, event->path.dentry,
+				event->path.dentry->d_parent,
+				event->path.dentry->d_flags);
 		new_file = dentry_open(&event->path,
 				       group->fanotify_data.f_flags | FMODE_NONOTIFY,
 				       current_cred());
-	else
+	} else {
 		new_file = ERR_PTR(-EOVERFLOW);
+	}
 	if (IS_ERR(new_file)) {
 		/*
 		 * we still send an event even if we can't open the file.  this
-- 
2.7.4

[RFC][PATCH 6/7] fanotify: add support for create/attrib/rename/delete events

[Amir Goldstein]

Add support for create/attrib/rename/delete events with data type
FSNOTIFY_EVENT_DENTRY, which can be reported on a watched
directory inode.

New dentry events may pass parent directory's path on event fd,
which may break old programs that request FAN_ALL_EVENTS.
Ignore dentry events unless user explicitly set the new
FAN_EVENT_INFO_PARENT flag to fanotify_init().

A mount watch cannot get dentry events, because the mount point
from which those events were created is unavailable inforamtion.

Legacy inotify events that are still not supported are
DELETE_SELF and MOVE_FROM, whose event data type is still
FSNOTIFY_EVENT_INODE.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify.c      |  7 +++++++
 fs/notify/fanotify/fanotify_user.c | 12 ++++++++++++
 include/uapi/linux/fanotify.h      | 26 ++++++++++++++++++++++----
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index fb2b852..378101c 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -212,9 +212,16 @@ static int fanotify_handle_event(struct fsnotify_group *group,
 
 	BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);
 	BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);
+	BUILD_BUG_ON(FAN_ATTRIB != FS_ATTRIB);
 	BUILD_BUG_ON(FAN_CLOSE_NOWRITE != FS_CLOSE_NOWRITE);
 	BUILD_BUG_ON(FAN_CLOSE_WRITE != FS_CLOSE_WRITE);
 	BUILD_BUG_ON(FAN_OPEN != FS_OPEN);
+	BUILD_BUG_ON(FAN_MOVED_TO != FS_MOVED_TO);
+	BUILD_BUG_ON(FAN_MOVED_FROM != FS_MOVED_FROM);
+	BUILD_BUG_ON(FAN_CREATE != FS_CREATE);
+	BUILD_BUG_ON(FAN_DELETE != FS_DELETE);
+	BUILD_BUG_ON(FAN_DELETE_SELF != FS_DELETE_SELF);
+	BUILD_BUG_ON(FAN_MOVE_SELF != FS_MOVE_SELF);
 	BUILD_BUG_ON(FAN_EVENT_ON_CHILD != FS_EVENT_ON_CHILD);
 	BUILD_BUG_ON(FAN_Q_OVERFLOW != FS_Q_OVERFLOW);
 	BUILD_BUG_ON(FAN_OPEN_PERM != FS_OPEN_PERM);
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 789962c..616769a 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -907,6 +907,18 @@ SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
 	    group->fanotify_data.flags & FAN_EVENT_INFO_PARENT)
 		mnt = path.mnt;
 
+	/*
+	 * New dentry events may pass parent directory's path on event fd,
+	 * which may break old programs that request FAN_ALL_EVENTS.
+	 * Ignore dentry events unless user explicitly set the new
+	 * FAN_EVENT_INFO_PARENT flag to fanotify_init().
+	 * Mount watch cannot get dentry events, because the mount point
+	 * from which those events were created is unavailable inforamtion.
+	 */
+	if ((flags & FAN_MARK_MOUNT) ||
+	    !(group->fanotify_data.flags & FAN_EVENT_INFO_PARENT))
+		mask &= ~FAN_DENTRY_EVENTS;
+
 	/* create/update an inode mark */
 	switch (flags & (FAN_MARK_ADD | FAN_MARK_REMOVE)) {
 	case FAN_MARK_ADD:
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index 8e58765..3389da0 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -6,9 +6,17 @@
 /* the following events that user-space can register for */
 #define FAN_ACCESS		0x00000001	/* File was accessed */
 #define FAN_MODIFY		0x00000002	/* File was modified */
+#define FAN_ATTRIB		0x00000004	/* Metadata changed */
 #define FAN_CLOSE_WRITE		0x00000008	/* Writtable file closed */
 #define FAN_CLOSE_NOWRITE	0x00000010	/* Unwrittable file closed */
 #define FAN_OPEN		0x00000020	/* File was opened */
+#define FAN_MOVED_FROM		0x00000040	/* File was moved from X */
+#define FAN_MOVED_TO		0x00000080	/* File was moved to Y */
+#define FAN_CREATE		0x00000100	/* Subfile was created */
+#define FAN_DELETE		0x00000200	/* Subfile was deleted */
+#define FAN_DELETE_SELF		0x00000400	/* Self was deleted */
+#define FAN_MOVE_SELF		0x00000800	/* Self was moved */
+
 
 #define FAN_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
 
@@ -21,6 +29,7 @@
 
 /* helper events */
 #define FAN_CLOSE		(FAN_CLOSE_WRITE | FAN_CLOSE_NOWRITE) /* close */
+#define FAN_MOVE		(FAN_MOVED_FROM | FAN_MOVED_TO) /* moves */
 
 /* flags used for fanotify_init() */
 #define FAN_CLOEXEC		0x00000001
@@ -69,10 +78,19 @@
  * the future and not break backward compatibility.  Apps will get only the
  * events that they originally wanted.  Be sure to add new events here!
  */
-#define FAN_ALL_EVENTS (FAN_ACCESS |\
-			FAN_MODIFY |\
-			FAN_CLOSE |\
-			FAN_OPEN)
+
+/* Events reported with data type FSNOTIFY_EVENT_PATH */
+#define FAN_PATH_EVENTS (FAN_ACCESS |\
+			 FAN_MODIFY |\
+			 FAN_CLOSE |\
+			 FAN_OPEN)
+
+/* Events reported with data type FSNOTIFY_EVENT_DENTRY */
+#define FAN_DENTRY_EVENTS (FAN_ATTRIB |\
+			   FAN_MOVED_TO | FAN_MOVE_SELF |\
+			   FAN_CREATE | FAN_DELETE)
+
+#define FAN_ALL_EVENTS (FAN_PATH_EVENTS | FAN_DENTRY_EVENTS)
 
 /*
  * All events which require a permission response from userspace
-- 
2.7.4

[RFC][PATCH 7/7] fanotify: pass filename info for filename events

[Amir Goldstein]

On filename events, allocate a variable length
fanotify_file_event_info struct and store the filename,
along with the parent directory path struct.

When filename exists, The fanotify_event_metadata.event_len
field includes the size of the metadata header and the size
of the padded filename.

User programs that use the fanotify macros FAN_EVENT_NEXT()
and FAN_EVENT_OK() will not experience any breakage and new
user programs can read the filename as a null terminated string
from the event buffer at offset FAN_EVENT_METADATA_LEN.

Nevertheless, programs that want to get the filename info
are required to explicitly request it via the FAN_EVENT_INFO_NAME
flag to fanotify_init(). This flag is only allowed to be set
along with the default FAN_CLASS_NOTIF notification class.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify.c      | 41 +++++++++++++++++++++++++++++++--
 fs/notify/fanotify/fanotify.h      | 24 +++++++++++++++++++-
 fs/notify/fanotify/fanotify_user.c | 46 ++++++++++++++++++++++++++++++++++++--
 include/uapi/linux/fanotify.h      |  8 ++++++-
 4 files changed, 113 insertions(+), 6 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 378101c..469c0d4 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -45,6 +45,12 @@ static int fanotify_merge(struct list_head *list, struct fsnotify_event *event)
 		return 0;
 #endif
 
+	/*
+	 * Don't merge a filename event with any other event
+	 */
+	if (event->mask & FAN_FILENAME_EVENTS)
+		return 0;
+
 	list_for_each_entry_reverse(test_event, list, list) {
 		if (should_merge(test_event, event)) {
 			do_merge = true;
@@ -161,7 +167,8 @@ static bool fanotify_should_send_event(struct fsnotify_mark *inode_mark,
 }
 
 struct fanotify_event_info *fanotify_alloc_event(struct inode *inode, u32 mask,
-						 struct path *path)
+						 struct path *path,
+						 const char *file_name)
 {
 	struct fanotify_event_info *event;
 
@@ -178,6 +185,31 @@ struct fanotify_event_info *fanotify_alloc_event(struct inode *inode, u32 mask,
 		goto init;
 	}
 #endif
+
+	/*
+	 * For filename events (create,delete,rename), path points to the
+	 * directory and name holds the entry name, so allocate a variable
+	 * length fanotify_file_event_info struct.
+	 */
+	if (mask & FAN_FILENAME_EVENTS) {
+		struct fanotify_file_event_info *ffe;
+		int alloc_len = sizeof(*ffe);
+		int len = 0;
+
+		if ((mask & FAN_FILENAME_EVENTS) && file_name) {
+			len = strlen(file_name);
+			alloc_len += len + 1;
+		}
+		ffe = kmalloc(alloc_len, GFP_KERNEL);
+		if (!ffe)
+			return NULL;
+		event = &ffe->fae;
+		ffe->name_len = len;
+		if (len)
+			strcpy(ffe->name, file_name);
+		goto init;
+	}
+
 	event = kmem_cache_alloc(fanotify_event_cachep, GFP_KERNEL);
 	if (!event)
 		return NULL;
@@ -245,7 +277,7 @@ static int fanotify_handle_event(struct fsnotify_group *group,
 	pr_debug("%s: group=%p inode=%p mask=%x\n", __func__, group, inode,
 		 mask);
 
-	event = fanotify_alloc_event(inode, mask, &path);
+	event = fanotify_alloc_event(inode, mask, &path, file_name);
 	if (unlikely(!event))
 		return -ENOMEM;
 
@@ -292,6 +324,11 @@ static void fanotify_free_event(struct fsnotify_event *fsn_event)
 		return;
 	}
 #endif
+	if (fsn_event->mask & FAN_FILENAME_EVENTS) {
+		kfree(FANOTIFY_FE(fsn_event));
+		return;
+	}
+
 	kmem_cache_free(fanotify_event_cachep, event);
 }
 
diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
index 2a5fb14..1cbf409 100644
--- a/fs/notify/fanotify/fanotify.h
+++ b/fs/notify/fanotify/fanotify.h
@@ -20,6 +20,21 @@ struct fanotify_event_info {
 	struct pid *tgid;
 };
 
+/*
+ * Structure for fanotify events with variable length data.
+ * It gets allocated in fanotify_handle_event() and freed
+ * when the information is retrieved by userspace
+ */
+struct fanotify_file_event_info {
+	struct fanotify_event_info fae;
+	/*
+	 * For filename events (create,delete,rename), path points to the
+	 * directory and name holds the entry name
+	 */
+	int name_len;
+	char name[];
+};
+
 #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
 /*
  * Structure for permission fanotify events. It gets allocated and freed in
@@ -41,10 +56,17 @@ FANOTIFY_PE(struct fsnotify_event *fse)
 }
 #endif
 
+static inline struct fanotify_file_event_info *
+FANOTIFY_FE(struct fsnotify_event *fse)
+{
+	return container_of(fse, struct fanotify_file_event_info, fae.fse);
+}
+
 static inline struct fanotify_event_info *FANOTIFY_E(struct fsnotify_event *fse)
 {
 	return container_of(fse, struct fanotify_event_info, fse);
 }
 
 struct fanotify_event_info *fanotify_alloc_event(struct inode *inode, u32 mask,
-						 struct path *path);
+						 struct path *path,
+						 const char *file_name);
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 616769a..03fcf38 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -205,13 +205,22 @@ static int process_access_response(struct fsnotify_group *group,
 }
 #endif
 
+static int round_event_name_len(struct fanotify_file_event_info *event)
+{
+	if (!event->name_len)
+		return 0;
+	return roundup(event->name_len + 1, FAN_EVENT_METADATA_LEN);
+}
+
 static ssize_t copy_event_to_user(struct fsnotify_group *group,
 				  struct fsnotify_event *event,
 				  char __user *buf)
 {
 	struct fanotify_event_metadata fanotify_event_metadata;
+	struct fanotify_file_event_info *ffe = NULL;
 	struct file *f;
 	int fd, ret;
+	size_t pad_name_len = 0;
 
 	pr_debug("%s: group=%p event=%p\n", __func__, group, event);
 
@@ -219,12 +228,37 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
 	if (ret < 0)
 		return ret;
 
+	if ((event->mask & FAN_FILENAME_EVENTS) &&
+	    (group->fanotify_data.flags & FAN_EVENT_INFO_NAME)) {
+		ffe = FANOTIFY_FE(event);
+		pad_name_len = round_event_name_len(ffe);
+		fanotify_event_metadata.event_len += pad_name_len;
+	}
+
 	fd = fanotify_event_metadata.fd;
 	ret = -EFAULT;
 	if (copy_to_user(buf, &fanotify_event_metadata,
-			 fanotify_event_metadata.event_len))
+			 FAN_EVENT_METADATA_LEN))
 		goto out_close_fd;
 
+	buf += FAN_EVENT_METADATA_LEN;
+
+	/*
+	 * send the filename and pad to a multiple of FAN_EVENT_METADATA_LEN
+	 * with zeros.
+	 */
+	ret = -EFAULT;
+	if (ffe && pad_name_len) {
+		/* copy the filename */
+		if (copy_to_user(buf, ffe->name, ffe->name_len))
+			goto out_close_fd;
+		buf += ffe->name_len;
+
+		/* fill userspace with 0's */
+		if (clear_user(buf, pad_name_len - ffe->name_len))
+			goto out_close_fd;
+	}
+
 #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
 	if (event->mask & FAN_ALL_PERM_EVENTS)
 		FANOTIFY_PE(event)->fd = fd;
@@ -755,7 +789,7 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	group->fanotify_data.user = user;
 	atomic_inc(&user->fanotify_listeners);
 
-	oevent = fanotify_alloc_event(NULL, FS_Q_OVERFLOW, NULL);
+	oevent = fanotify_alloc_event(NULL, FS_Q_OVERFLOW, NULL, NULL);
 	if (unlikely(!oevent)) {
 		fd = -ENOMEM;
 		goto out_destroy_group;
@@ -919,6 +953,14 @@ SYSCALL_DEFINE5(fanotify_mark, int, fanotify_fd, unsigned int, flags,
 	    !(group->fanotify_data.flags & FAN_EVENT_INFO_PARENT))
 		mask &= ~FAN_DENTRY_EVENTS;
 
+	/*
+	 * Filename events are not interesting without the name inforamtion.
+	 * Ignore filename events unless user explicitly set the new
+	 * FAN_EVENT_INFO_NAME flag to fanotify_init().
+	 */
+	if (!(group->fanotify_data.flags & FAN_EVENT_INFO_NAME))
+		mask &= ~FAN_FILENAME_EVENTS;
+
 	/* create/update an inode mark */
 	switch (flags & (FAN_MARK_ADD | FAN_MARK_REMOVE)) {
 	case FAN_MARK_ADD:
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index 3389da0..83f26d9 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -47,7 +47,9 @@
 
 /* These bits determine the format of the reported events */
 #define FAN_EVENT_INFO_PARENT	0x00000100	/* Event fd maybe of parent */
-#define FAN_ALL_EVENT_INFO_BITS (FAN_EVENT_INFO_PARENT)
+#define FAN_EVENT_INFO_NAME	0x00000200	/* Event data has filename */
+#define FAN_ALL_EVENT_INFO_BITS (FAN_EVENT_INFO_PARENT | \
+				 FAN_EVENT_INFO_NAME)
 
 #define FAN_ALL_INIT_FLAGS	(FAN_CLOEXEC | FAN_NONBLOCK | \
 				 FAN_ALL_CLASS_BITS | \
@@ -98,6 +100,10 @@
 #define FAN_ALL_PERM_EVENTS (FAN_OPEN_PERM |\
 			     FAN_ACCESS_PERM)
 
+/* Events on directory requiring to pass filename */
+#define FAN_FILENAME_EVENTS (FAN_MOVED_FROM | FAN_MOVED_TO |\
+			     FAN_CREATE | FAN_DELETE)
+
 #define FAN_ALL_OUTGOING_EVENTS	(FAN_ALL_EVENTS |\
 				 FAN_ALL_PERM_EVENTS |\
 				 FAN_Q_OVERFLOW)
-- 
2.7.4

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

Hi Aleksander,

I dug up this thread from Tracker ml:
https://mail.gnome.org/archives/tracker-list/2011-September/msg00089.html

Linking to this posting on LKML:
https://lkml.org/lkml/2009/3/27/166

Is the Tracker project interested is a solution for efficient
monitoring of entire file system?
If only interested in unprivileged monitoring and/or recursive
directory monitoring,
the work could be extended in this direction (see details below)

Cheers,
Amir.


On Mon, Oct 10, 2016 at 10:12 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> Hi all,
>
> This series is a prep work for using fanotify to monitor all
> events in a file system with a single watch.
>
> The end result is indented to be an alternative to the recursive
> inotify watches scheme, which has its problems.
>
> This first part adds support for most inotify events to fanotify
> when watching a directory.
>

Please note that in my current implementation of directory watch, you
get the directory fd
for create/move/delete events and the filename information is optional
data to event by an opt-in flag to fanotify_init.
Without filename data, those events can be more compact and easily merged,
in case user is only interested in coalesced "something changed here" event.

> The next part will add support for watching a super block,
> which is not the same as watching a mount point.
>

FYI, I also intend to (optionally) replace the fd information with
file handle for a super block watch.
That has a few benefits:
- Avoids the need to take references on path/dentries in the event queue
- Could pave the path to relaxing CAP_ADMIN requirement
- It serves as a replacement the rename cookie (and it persists across
any number of renames)

> I am posting this WIP to get feedback on the idea and to find
> out if there are any users out there interested in the improved
> fanotify capabilities and/or in the super block monitoring
> use case.
>
> Amir Goldstein (7):
>   fsnotify: pass dentry instead of inode when available
>   fsnotify: annotate filename events
>   fanotify: new init flag FAN_EVENT_INFO_PARENT
>   fanotify: store mount point from which an inode watch was added
>   fanotify: support events with data type FSNOTIFY_EVENT_DENTRY
>   fanotify: add support for create/attrib/rename/delete events
>   fanotify: pass filename info for filename events
>
>  fs/notify/fanotify/fanotify.c      | 85 +++++++++++++++++++++++++++++++----
>  fs/notify/fanotify/fanotify.h      | 24 +++++++++-
>  fs/notify/fanotify/fanotify_user.c | 92 ++++++++++++++++++++++++++++++++++----
>  fs/notify/fdinfo.c                 |  4 +-
>  fs/notify/fsnotify.c               |  2 +-
>  fs/notify/inode_mark.c             |  1 +
>  fs/notify/mark.c                   | 15 +++++--
>  include/linux/fsnotify.h           | 46 ++++++++++++++-----
>  include/linux/fsnotify_backend.h   | 24 +++++++---
>  include/uapi/linux/fanotify.h      | 41 ++++++++++++++---
>  10 files changed, 287 insertions(+), 47 deletions(-)
>
> --
> 2.7.4
>

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Jan Kara]

Hi,

On Mon 10-10-16 22:12:57, Amir Goldstein wrote:
> This series is a prep work for using fanotify to monitor all
> events in a file system with a single watch.
> 
> The end result is indented to be an alternative to the recursive
> inotify watches scheme, which has its problems.
> 
> This first part adds support for most inotify events to fanotify
> when watching a directory.

Well, and how is this better than what inotify supports? The proclaimed
advantage of fanotify() was that you get open file handle in the event
which prevents races when directory tree changes before you manage to
process inotify event. However with directory changes, you are again
starting to report names and so things become racy again. So I don't quite
see a value of reimplementing this in fanotify. The only benefit I see is
that you can watch the whole mountpoint with one watch instead of having to
add watch to every directory. Is that such a huge win? What is a use case
for that?

> The next part will add support for watching a super block,
> which is not the same as watching a mount point.

Careful here. In the world of user namespaces and containers you have to be
really careful so that events from one container don't leak into another
container despite they live in the same physical filesystem, just a
different bind mount. I believe chroot / bind mounts was one of the reasons
why fanotify ended with mountpoints and not with superblocks. But I guess
Eric or Al remember this better.

> I am posting this WIP to get feedback on the idea and to find
> out if there are any users out there interested in the improved
> fanotify capabilities and/or in the super block monitoring
> use case.

Well, I hope you have some usecases in mind when you propose this ;) I've
been asked about fanotify for superblocks but so far I think that doing it
in kernel would be a headache (mostly security-wise) and doing it in
userspace - watch every mountpoint in /proc/mounts - may be less
error-prone.

								Honza

> 
> Amir Goldstein (7):
>   fsnotify: pass dentry instead of inode when available
>   fsnotify: annotate filename events
>   fanotify: new init flag FAN_EVENT_INFO_PARENT
>   fanotify: store mount point from which an inode watch was added
>   fanotify: support events with data type FSNOTIFY_EVENT_DENTRY
>   fanotify: add support for create/attrib/rename/delete events
>   fanotify: pass filename info for filename events
> 
>  fs/notify/fanotify/fanotify.c      | 85 +++++++++++++++++++++++++++++++----
>  fs/notify/fanotify/fanotify.h      | 24 +++++++++-
>  fs/notify/fanotify/fanotify_user.c | 92 ++++++++++++++++++++++++++++++++++----
>  fs/notify/fdinfo.c                 |  4 +-
>  fs/notify/fsnotify.c               |  2 +-
>  fs/notify/inode_mark.c             |  1 +
>  fs/notify/mark.c                   | 15 +++++--
>  include/linux/fsnotify.h           | 46 ++++++++++++++-----
>  include/linux/fsnotify_backend.h   | 24 +++++++---
>  include/uapi/linux/fanotify.h      | 41 ++++++++++++++---
>  10 files changed, 287 insertions(+), 47 deletions(-)
> 
> -- 
> 2.7.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

On Oct 11, 2016 9:42 PM, "Jan Kara" <jack@suse.cz> wrote:
>
> Hi,
>
> On Mon 10-10-16 22:12:57, Amir Goldstein wrote:
> > This series is a prep work for using fanotify to monitor all
> > events in a file system with a single watch.
> >
> > The end result is indented to be an alternative to the recursive
> > inotify watches scheme, which has its problems.
> >
> > This first part adds support for most inotify events to fanotify
> > when watching a directory.
>
> Well, and how is this better than what inotify supports? The proclaimed
> advantage of fanotify() was that you get open file handle in the event
> which prevents races when directory tree changes before you manage to
> process inotify event. However with directory changes, you are again
> starting to report names and so things become racy again. So I don't quite
> see a value of reimplementing this in fanotify.

For this part, I did not intend to add any benefit compared to inotify.
This is prep work to align fanotify with inotify feature set before I begin to
enhance fanotify to surpass inotify feature set. Besides, inotify was deemed
a bad API (rightfully IMO), and since my work requires adding more info to
the event data I expected that any attempt to modify the inotify API
would be rejected and re-directed towards using the newer and more flexible
fanotify API instead.

Also, please note that there is still a subtle and important difference between
inotify dir watch and fanotify dir watch presented by this work.
The child file changes are still reported with fd of the accessed
files themselves.
The dir entry name changes are reported with fd of the dir so there is
not really
any race issues as far as I can tell, only a recorded history of entry
name changes.
In fact, it is easy to report fd of the file in case of create and
move-to events,
but I chose the semantics of identifying the directory for all entry
name changes.

> The only benefit I see is
> that you can watch the whole mountpoint with one watch instead of having to
> add watch to every directory. Is that such a huge win? What is a use case
> for that?
>

Our use case is watching over 10M directories for continuous backup.
Maybe much more and for a workload where most of those directories are
seldom accessed. Inotify pins the directory inode for every watched directory.
This is not very scalable in terms of memory usage.

> > The next part will add support for watching a super block,
> > which is not the same as watching a mount point.
>
> Careful here. In the world of user namespaces and containers you have to be
> really careful so that events from one container don't leak into another
> container despite they live in the same physical filesystem, just a
> different bind mount. I believe chroot / bind mounts was one of the reasons
> why fanotify ended with mountpoints and not with superblocks. But I guess
> Eric or Al remember this better.
>

[cc ebiederman]

There are 2 sides to this argument. On the one hand, leaking file access
information (pid info and read/perm access events for that matter) across
namespaces should be done carefully or not done at all.
On the other hand, no namespace has exclusive ownership of the file system.
If a directory is accessible from several mount points be it different
namespaces or not, file system modifications affect all mounts from which
this directory is visible, so it makes sense to notify listeners from any mount
at least about MODIFY|ATTRIB|CREATE|MOVE|DELETE events in that directory.

Further more, fanotify currently requires CAP_ADMIN and I intend to enforce the
same capabilties required to mount a super block for watching a super block.
So containers with user namespace or with no CAP_ADMIN won't be able to use this
feature, but the container daemon will be able to use it to monitor
all files accessible
to the host. In my eyes this is a usability improvement, not an
information leak.

> > I am posting this WIP to get feedback on the idea and to find
> > out if there are any users out there interested in the improved
> > fanotify capabilities and/or in the super block monitoring
> > use case.
>
> Well, I hope you have some usecases in mind when you propose this ;) I've
> been asked about fanotify for superblocks

For file access events only? or for directory changes as well?
Because that feature is not avaialbe with fanotify mount watch
(neither with my work).

> but so far I think that doing it
> in kernel would be a headache (mostly security-wise) and doing it in
> userspace - watch every mountpoint in /proc/mounts - may be less
> error-prone.

Hmm, so I am all for implementing in userspace and monitoring mounts
would have been much better than monitoring super block.
In fact, our use case is actually recursive directory subtree watch and
super block watch is a compromise, because it requires more filtering
in userspace.
The reason I am aiming for super block watch is actually technical -
all of the directory fsnotify hooks do not have the 'file' or 'path' reference,
only the 'dentry' (and some only have the 'inode'), because they are buried deep
inside vfs helpers. I considered the option of propagating the dir
file reference
into the vfs helpers, but that seems unnecessarily complicated. Especially,
in light of the fact that I believe in the right of all mount namespaces to get
notified of changes in fs visible from their mounts.

Let me state my roadmap, because I realize it is not clear from this WIP:
1. fanotify supports directory events
2. fanotify supports super block watch
3. fanotify optionally reports file handles instead of file descriptors
4. fanotify filters out events on dentries that are not accessible
from the mount
    from which the watch was added

The implication of step #4 above it that the fsnotify infrastructure will report
any event on the super block to the fanotify backend and fanotify will
filter every reported dentry by walking up to root (at least for slow path),
looking for the root dentry of the mount from which the inode mark was added.

For some workloads it will make more sense to do recursive directory watch
the old inotify way and for some workloads and it would make sense to do
a filtered root watch. In either case, vfs performance does not suffer any
degradation outside the scope of the watch (either single dir or sb root).

Does this plan make sense?

Cheers,
Amir.


>
>                                                                 Honza
>
> >
> > Amir Goldstein (7):
> >   fsnotify: pass dentry instead of inode when available
> >   fsnotify: annotate filename events
> >   fanotify: new init flag FAN_EVENT_INFO_PARENT
> >   fanotify: store mount point from which an inode watch was added
> >   fanotify: support events with data type FSNOTIFY_EVENT_DENTRY
> >   fanotify: add support for create/attrib/rename/delete events
> >   fanotify: pass filename info for filename events
> >
> >  fs/notify/fanotify/fanotify.c      | 85 +++++++++++++++++++++++++++++++----
> >  fs/notify/fanotify/fanotify.h      | 24 +++++++++-
> >  fs/notify/fanotify/fanotify_user.c | 92 ++++++++++++++++++++++++++++++++++----
> >  fs/notify/fdinfo.c                 |  4 +-
> >  fs/notify/fsnotify.c               |  2 +-
> >  fs/notify/inode_mark.c             |  1 +
> >  fs/notify/mark.c                   | 15 +++++--
> >  include/linux/fsnotify.h           | 46 ++++++++++++++-----
> >  include/linux/fsnotify_backend.h   | 24 +++++++---
> >  include/uapi/linux/fanotify.h      | 41 ++++++++++++++---
> >  10 files changed, 287 insertions(+), 47 deletions(-)
> >
> > --
> > 2.7.4
> >
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Marko Rauhamaa]

Amir Goldstein <amir73il@gmail.com>:

> Marko,
>
> The super block watch patches are available on my git tree at:
> https://github.com/amir73il/linux/commits/fanotify_sb-4.9
>
> I won't bother to post them to the list unless someone shows interest.

Thank you, Amir! I will investigate your changes next week.


Marko

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

On Thu, Oct 13, 2016 at 9:42 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Thu, Oct 13, 2016 at 8:35 PM, Marko Rauhamaa
> <marko.rauhamaa@f-secure.com> wrote:
>>
>> Amir Goldstein:
>>> This series is a prep work for using fanotify to monitor all events in
>>> a file system with a single watch.
>>>
>>> [...]
>>>
>>> I am posting this WIP to get feedback on the idea and to find out if
>>> there are any users out there interested in the improved fanotify
>>> capabilities and/or in the super block monitoring use case.
>>
>> My employer certainly is in need of monitoring a whole filesystem. We
>> have noticed that namespaces evade monitoring via FAN_MARK_MOUNT. I was
>> thinking something like a FAN_MARK_FILESYSTEM would be needed.
>>
>
> I have a POC of monitoring entire file system, while filtering to namespace
> only the events that should be visible to its mounts.
> I need to get the patches into shape and shake them a bit, then
> I will post them and I am hoping that others could test them for their
> use case as well.
>

Marko,

The super block watch patches are available on my git tree at:
https://github.com/amir73il/linux/commits/fanotify_sb-4.9

I won't bother to post them to the list unless someone shows interest.
Note that beyond the ability to watch a file system, they also
provide all the legacy intotify events (create/remove), so maybe
that's also useful to you guys.

Anyway, if you are interested and willing to give them a try
please send me feedback.

Thanks,
Amir.

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Marko Rauhamaa]

Amir Goldstein <amir73il@gmail.com>:
> Let me rephrse my question - please argue why monitoring a file system
> instead of a mount point is important for your use case and more impotantly,
> please argue why you cannot achive the same result by monitoring all the
> relevant mount points from user space.

We are already doing that and missing fanotify events that are shrouded
by namespaces. Namespaces are popular through the use of containers, but
not only that. Distros are using namespaces to protect the private files
of services:

   <URL: http://fedoraproject.org/wiki/Features/ServicesPrivateTmp>

   <URL: https://access.redhat.com/blogs/766093/posts/1976243>.

> For example, the argument I used against the legacy recursive intotify
> watch of all directories in the treee is the poor ability to scale
> well over millions of directories.

Sure, that would be untenable. I'd argue the namespace issue is at least
equally tough because you'd have to keep monitoring namespaces deep
under the /proc hierarchy where processes come and go, there is no
epollable notification scheme available (to my knowledge) and race
conditions are inevitable. It would be virtually impossible for a
top-level fanotify monitor to keep track of what is going on.


Marko

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

On Fri, Oct 14, 2016 at 11:28 AM, Marko Rauhamaa
<marko.rauhamaa@f-secure.com> wrote:
> Amir Goldstein <amir73il@gmail.com>:
>
>> On Thu, Oct 13, 2016 at 8:35 PM, Marko Rauhamaa
>>> My employer certainly is in need of monitoring a whole filesystem. We
>>> have noticed that namespaces evade monitoring via FAN_MARK_MOUNT. I
>>> was thinking something like a FAN_MARK_FILESYSTEM would be needed.
>>
>> [...]
>>
>> I keep hearing about people that wanted that feature, but those people will
>> need to come forward and voice their use cases.
>
> Well, F-Secure's Linux Security product monitors files to detect
> malware. Files are analyzed for viruses and unexpected modifications to
> system files are flagged.
>

This I could have guessed :-)

Let me rephrse my question - please argue why monitoring a file system
instead of a mount point is important for your use case and more impotantly,
please argue why you cannot achive the same result by monitoring all the
relevant mount points from user space.

For example, the argument I used against the legacy recursive intotify watch
of all directories in the treee is the poor ability to scale well over
millions of directories.


>
> Other fanotify deficiencies include:
>
>  * the offending process can die without leaving a trace because
>    FAN_CLOSE_WRITE events do not block (instead of blocking, it would be
>    enough for the /proc/$PID directory to stay available while the
>    related fanotify fd is open)
>
>  * the (e)uid and (e)gid of the offending process are not conveyed in
>    the fanotify event
>
>  * the FAN_OPEN_PERM event does not carry the mode so write access
>    cannot be denied
>
>  * there is no (PERM or non-PERM) event generated by the first
>    modification (FAN_MODIFY generates a flurry of events;
>    FAN_CLOSE_WRITE does not get generated unless the file is closed)
>
>

I am not sure I will be able help you wrt those extra requirements.

Amir.

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Marko Rauhamaa]

Amir Goldstein <amir73il@gmail.com>:

> On Thu, Oct 13, 2016 at 8:35 PM, Marko Rauhamaa
>> My employer certainly is in need of monitoring a whole filesystem. We
>> have noticed that namespaces evade monitoring via FAN_MARK_MOUNT. I
>> was thinking something like a FAN_MARK_FILESYSTEM would be needed.
>
> [...]
>
> I keep hearing about people that wanted that feature, but those people will
> need to come forward and voice their use cases.

Well, F-Secure's Linux Security product monitors files to detect
malware. Files are analyzed for viruses and unexpected modifications to
system files are flagged.


Other fanotify deficiencies include:

 * the offending process can die without leaving a trace because
   FAN_CLOSE_WRITE events do not block (instead of blocking, it would be
   enough for the /proc/$PID directory to stay available while the
   related fanotify fd is open)

 * the (e)uid and (e)gid of the offending process are not conveyed in
   the fanotify event

 * the FAN_OPEN_PERM event does not carry the mode so write access
   cannot be denied

 * there is no (PERM or non-PERM) event generated by the first
   modification (FAN_MODIFY generates a flurry of events;
   FAN_CLOSE_WRITE does not get generated unless the file is closed)


Marko

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Amir Goldstein]

On Thu, Oct 13, 2016 at 8:35 PM, Marko Rauhamaa
<marko.rauhamaa@f-secure.com> wrote:
>
> Amir Goldstein:
>> This series is a prep work for using fanotify to monitor all events in
>> a file system with a single watch.
>>
>> [...]
>>
>> I am posting this WIP to get feedback on the idea and to find out if
>> there are any users out there interested in the improved fanotify
>> capabilities and/or in the super block monitoring use case.
>
> My employer certainly is in need of monitoring a whole filesystem. We
> have noticed that namespaces evade monitoring via FAN_MARK_MOUNT. I was
> thinking something like a FAN_MARK_FILESYSTEM would be needed.
>

I have a POC of monitoring entire file system, while filtering to namespace
only the events that should be visible to its mounts.
I need to get the patches into shape and shake them a bit, then
I will post them and I am hoping that others could test them for their
use case as well.

I keep hearing about people that wanted that feature, but those people will
need to come forward and voice their use cases.

> (There are some other needed features but filesystem monitoring is the
> most pressing one.)
>
>
> Jan Kara:
>> Careful here. In the world of user namespaces and containers you have
>> to be really careful so that events from one container don't leak into
>> another container despite they live in the same physical filesystem,
>> just a different bind mount.
>
> Obviously, proper care needs to be taken, but a namespace should not be
> able smuggle filesystem events past fanotify monitoring.
>

I agree.

Cheers,
Amir.

Re: [RFC][PATCH 0/7] fanotify: add support for more events

[Marko Rauhamaa]

Amir Goldstein:
> This series is a prep work for using fanotify to monitor all events in
> a file system with a single watch.
>
> [...]
>
> I am posting this WIP to get feedback on the idea and to find out if
> there are any users out there interested in the improved fanotify
> capabilities and/or in the super block monitoring use case.

My employer certainly is in need of monitoring a whole filesystem. We
have noticed that namespaces evade monitoring via FAN_MARK_MOUNT. I was
thinking something like a FAN_MARK_FILESYSTEM would be needed.

(There are some other needed features but filesystem monitoring is the
most pressing one.)


Jan Kara:
> Careful here. In the world of user namespaces and containers you have
> to be really careful so that events from one container don't leak into
> another container despite they live in the same physical filesystem,
> just a different bind mount.

Obviously, proper care needs to be taken, but a namespace should not be
able smuggle filesystem events past fanotify monitoring.


Marko