linux-kernel - [RFC 1/2] fanotify: new event FAN_MODIFY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 14 Mar 2017 00:02:20 +0100
From:   Filip Štědronský <r.lklm@...narg.cz>
To:     linux-fsdevel@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, Jan Kara <jack@...e.cz>,
        Amir Goldstein <amir73il@...il.com>,
        Alexander Viro <viro@...iv.linux.org.uk>
Subject: [RFC 1/2] fanotify: new event FAN_MODIFY_DIR

Fanotify currently does not report directory modification events
(rename, unlink, etc.). But these events are essential for about half of
concievable fanotify use cases, especially:

  - file system indexers / desktop search tools
  - file synchronization tools (like Dropbox, Nextcloud, etc.),
    online backup tools

and pretty much any app that needs to maintain and up-to-date internal
representation of current contents of the file system.

All applications of the above classes that I'm aware of currently use
recursive inotify watches, which do not scale (my home dir has ~200k
directories, creating all the watches takes ~2min and eats several tens
of MB of kernel memory).

There have been many calls for such a feature, pretty much since the
creation of fanotify in 2009:
  * By GNOME developers:
    https://wiki.gnome.org/BastienNocera/KernelWishlist#VFS.2C_filesystems
  * By KDE developers:
    http://lkml.kernel.org/r/201211011352.42476.Martin%40lichtvoll.de
    'Better support for (desktop) file search / indexing applications'
  * And others:
    http://lkml.kernel.org/r/AANLkTi=owK=WZW4oNtpm5WpAZhqCQUdTR2K5gzJ_MqZ+%40mail.gmail.com
    'Fanotify mv/rename?'
    http://lkml.kernel.org/r/1238158043.23703.20.camel%40fatty
    'Issues with using fanotify for a filesystem indexer'

Add a new fanotify event, FAN_MODIFY_DIR, that is emitted whenever the
contents of a directory change (a directory entry is added, removed or
renamed). This covers all the currently missing events: rename, unlink,
mknod, mkdir, rmdir, etc.

Included with the event is a file descriptor to the modified directory
but no further info. The userspace application is expected to rescan the
whole directory and update its model accordingly. This needs to be done
carefully to prevent race conditions. A cross-directory rename generates
two FAN_MODIFY_DIR events.

This minimalistic approach has several advantages:
  * Does not require changes to the fanotify userspace API or the
    fsnotify in-kernel framework, apart from adding a new event.
    Especially doesn't complicate it by adding string fields.
  * Has simple and clear semantics, even with multiple renames occurring
    in parallel etc. In case of any inconsistencies, one can simply wait
    for a while and rescan again.
  * FAN_MODIFY_DIR events are easily merged in case of multiple
    operations on a directory (e.g. deleting all files).

Signed-off-by: Filip Štědronský <r.lkml@...narg.cz>

---

An alternative might be to create wrapper functions like
vfs_path_(rename|unlink|...). They could also take care of calling
security_path_(rename|unlink|...), which is currently also up to
the indvidual callers (possibly with a flag because it might not
be always desired).

An alternative was proposed by Amir Goldstein in several long series of
patches that add superblock-scoped (as opposed to vfsmount-scoped)
fanotify watches and specific dentry manipulation events with filenames:

    http://lkml.kernel.org/r/1481984434-13293-1-git-send-email-amir73il%40gmail.com
    http://lkml.kernel.org/r/1482247207-4424-1-git-send-email-amir73il%40gmail.com
    http://lkml.kernel.org/r/1476126784-12520-1-git-send-email-amir73il%40gmail.com
    http://lkml.kernel.org/r/1489411223-12081-1-git-send-email-amir73il%40gmail.com

There is large but not complete overlap between that proposal and
mine (which is was originally created over a year ago, before Amir's
work, but never posted).

I think the superblock watch idea is very interesting because it might
in the future allow reporing fsnotify events from remote and virtual
filesystems. So I'm posting this more as a potential source of more
ideas for discussion, or a fallback proposal in case Amir's patches
don't make it.
---
 fs/notify/fanotify/fanotify.c    |  1 +
 include/linux/fsnotify.h         | 17 +++++++++++++++++
 include/linux/fsnotify_backend.h |  1 +
 include/uapi/linux/fanotify.h    |  5 ++++-
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index bbc175d4213d..5178b06c338c 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -186,6 +186,7 @@ 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_MODIFY_DIR != FS_MODIFY_DIR);
 	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);
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index b43d3f5bd9ea..00fb87c975d6 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -208,6 +208,23 @@ static inline void fsnotify_modify(struct file *file)
 }
 
 /*
+ * fsnotify_modifydir - directory contents were changed
+ * (as a result of rename, creat, unlink, etc.)
+ */
+static inline void fsnotify_modify_dir(struct path *path)
+{
+	struct inode *inode = path->dentry->d_inode;
+	__u32 mask = FS_MODIFY_DIR;
+
+	if (S_ISDIR(inode->i_mode))
+		mask |= FS_ISDIR;
+	else
+		return;
+
+	fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
+}
+
+/*
  * fsnotify_open - file was opened
  */
 static inline void fsnotify_open(struct file *file)
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 487246546ebe..7751b337ec31 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -42,6 +42,7 @@
 
 #define FS_OPEN_PERM		0x00010000	/* open event in an permission hook */
 #define FS_ACCESS_PERM		0x00020000	/* access event in a permissions hook */
+#define FS_MODIFY_DIR		0x00040000	/* directory changed (rename/unlink/...) */
 
 #define FS_EXCL_UNLINK		0x04000000	/* do not send events if object is unlinked */
 #define FS_ISDIR		0x40000000	/* event occurred against dir */
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index 030508d195d3..f14e048d492a 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -15,6 +15,8 @@
 #define FAN_OPEN_PERM		0x00010000	/* File open in perm check */
 #define FAN_ACCESS_PERM		0x00020000	/* File accessed in perm check */
 
+#define FAN_MODIFY_DIR		0x00040000	/* directory changed (rename/unlink/...) */
+
 #define FAN_ONDIR		0x40000000	/* event occurred against dir */
 
 #define FAN_EVENT_ON_CHILD	0x08000000	/* interested in child events */
@@ -67,7 +69,8 @@
 #define FAN_ALL_EVENTS (FAN_ACCESS |\
 			FAN_MODIFY |\
 			FAN_CLOSE |\
-			FAN_OPEN)
+			FAN_OPEN |\
+			FAN_MODIFY_DIR)
 
 /*
  * All events which require a permission response from userspace
-- 
2.11.1