[PATCH v6 0/4] fanotify: introduce new event masks FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM

[PATCH v6 0/4] fanotify: introduce new event masks FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM

[Matthew Bobrowski]

Currently, the fanotify API does not provide a means for user space
applications to receive events when a file has been opened specifically
for execution. New event masks FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM have
been introduced in order to provide users with this capability.

These event types, when either are explicitly requested by the user, will
be returned within the event mask when a marked file object is being
opened has __FMODE_EXEC set as one of the flags for open_flag.

Linux is used as an operating system in some products, with an environment
that can be certified under the Common Criteria Operating System
Protection Profile (OSPP). This is a formal threat model for a class of
technology. It requires specific countermeasures to mitigate threats. It
requires documentation to explain how a product implements these
countermeasures. It requires proof via a test suite to demonstrate that
the requirements are met, observed and checked by an independent qualified
third party. The latest set of requirements for OSPP v4.2 can be found
here:

https://www.niap-ccevs.org/Profile/Info.cfm?PPID=424&id=424

If you look on page 58, you will see the following requirement:

FPT_SRP_EXT.1 Software Restriction Policies FPT_SRP_EXT.1.1
administrator specified [selection:
        file path,
        file digital signature,
        version,
        hash,
        [assignment: other characteristics]
]

This patch is to help aid in meeting this requirement.

Note that this set of patches are based on Amir's fsnotify-fixes branch,
which has already been posted through for review. For those interested,
this branch can be found here:
https://github.com/amir73il/linux/commits/fsnotify-fixes

LTP tests for this feature are on my 'fanotify-exec' branch here: 
https://github.com/matthewbobrowski/ltp/commits/fanotify_exec. The files
that contains the test cases are provided below:

syscalls/fanotify03: test cases have been updated to cover
		     FAN_OPEN_EXEC_PERM events
syscalls/fanotify12: newly introduced LTP test file to cover 
		     FAN_OPEN_EXEC events
		    
The man pages updates for these newly defined event masks can be found
here:
https://github.com/matthewbobrowski/man-pages/commits/fanotify_exec.

I would like to thank both Amir and Jan for their time and help, highly
appreciated once again.

---

Matthew Bobrowski (4):
  fanotify: return only user requested event types in event mask
  fanotify: introduce new event mask FAN_OPEN_EXEC
  fanotify: introduce new event mask FAN_OPEN_EXEC_PERM
  fsnotify: don't merge events FS_OPEN_PERM and FS_OPEN_EXEC_PERM

 fs/notify/fanotify/fanotify.c    | 29 +++++++++-------
 fs/notify/fsnotify.c             |  2 +-
 include/linux/fanotify.h         |  5 +--
 include/linux/fsnotify.h         | 58 +++++++++++++++++++-------------
 include/linux/fsnotify_backend.h | 13 ++++---
 include/uapi/linux/fanotify.h    |  2 ++
 6 files changed, 66 insertions(+), 43 deletions(-)

-- 
2.17.2


-- 
Matthew Bobrowski

[PATCH v6 1/4] fanotify: return only user requested event types in event mask

[Matthew Bobrowski]

Modify fanotify_should_send_event() so that it now returns a mask for
an event that contains ONLY flags for the event types that have been
specifically requested by the user. Flags that may have been included
within the event mask, but have not been explicitly requested by the
user will not be present in the returned value.

As an example, given the situation where a user requests events of type
FAN_OPEN. Traditionally, the event mask returned within an event that
occurred on a filesystem object that has been marked for monitoring and is
opened, will only ever have the FAN_OPEN bit set. With the introduction of
the new flags like FAN_OPEN_EXEC, and perhaps any other future event
flags, there is a possibility of the returned event mask containing more
than a single bit set, despite having only requested the single event type.
Prior to these modifications performed to fanotify_should_send_event(), a
user would have received a bundled event mask containing flags FAN_OPEN
and FAN_OPEN_EXEC in the instance that a file was opened for execution via
execve(), for example. This means that a user would receive event types
in the returned event mask that have not been requested. This runs the
possibility of breaking existing systems and causing other unforeseen
issues.

To mitigate this possibility, fanotify_should_send_event() has been
modified to return the event mask containing ONLY event types explicitly
requested by the user. This means that we will NOT report events that the
user did no set a mask for, and we will NOT report events that the user
has set an ignore mask for.

The function name fanotify_should_send_event() has also been updated so
that it's more relevant to what it has been designed to do.

Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
---
 fs/notify/fanotify/fanotify.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index e08a6647267b..0a09950317dd 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -89,7 +89,13 @@ static int fanotify_get_response(struct fsnotify_group *group,
 	return ret;
 }
 
-static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
+/*
+ * This function returns a mask for an event that only contains the flags
+ * that have been specifically requested by the user. Flags that may have
+ * been included within the event mask, but have not been explicitly
+ * requested by the user, will not be present in the returned mask.
+ */
+static u32 fanotify_group_event_mask(struct fsnotify_iter_info *iter_info,
 				       u32 event_mask, const void *data,
 				       int data_type)
 {
@@ -101,14 +107,14 @@ static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
 	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 we don't have enough info to send an event to userspace say no */
+	/* If we don't have enough info to send an event to userspace say no */
 	if (data_type != FSNOTIFY_EVENT_PATH)
-		return false;
+		return 0;
 
-	/* sorry, fanotify only gives a damn about files and dirs */
+	/* Sorry, fanotify only gives a damn about files and dirs */
 	if (!d_is_reg(path->dentry) &&
 	    !d_can_lookup(path->dentry))
-		return false;
+		return 0;
 
 	fsnotify_foreach_obj_type(type) {
 		if (!fsnotify_iter_should_report_type(iter_info, type))
@@ -131,11 +137,7 @@ static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
 	    !(marks_mask & FS_ISDIR & ~marks_ignored_mask))
 		return false;
 
-	if (event_mask & FANOTIFY_OUTGOING_EVENTS &
-	    marks_mask & ~marks_ignored_mask)
-		return true;
-
-	return false;
+	return event_mask & FANOTIFY_OUTGOING_EVENTS & marks_mask;
 }
 
 struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
@@ -210,7 +212,8 @@ static int fanotify_handle_event(struct fsnotify_group *group,
 
 	BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 10);
 
-	if (!fanotify_should_send_event(iter_info, mask, data, data_type))
+	mask = fanotify_group_event_mask(iter_info, mask, data, data_type);
+	if (!mask)
 		return 0;
 
 	pr_debug("%s: group=%p inode=%p mask=%x\n", __func__, group, inode,
-- 
2.17.2


-- 
Matthew Bobrowski

[PATCH v6 2/4] fanotify: introduce new event mask FAN_OPEN_EXEC

[Matthew Bobrowski]

A new event mask FAN_OPEN_EXEC has been defined so that users have the
ability to receive events specifically when a file has been opened with
the intent to be executed. Events of FAN_OPEN_EXEC type will be
generated when a file has been opened using either execve(), execveat()
or uselib() system calls.

The feature is implemented within fsnotify_open() by generating the
FAN_OPEN_EXEC event type if __FMODE_EXEC is set within file->f_flags.

Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
---
 fs/notify/fanotify/fanotify.c    | 3 ++-
 fs/notify/fsnotify.c             | 2 +-
 include/linux/fanotify.h         | 2 +-
 include/linux/fsnotify.h         | 2 ++
 include/linux/fsnotify_backend.h | 7 +++++--
 include/uapi/linux/fanotify.h    | 1 +
 6 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 0a09950317dd..e30f3a1d9699 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -209,8 +209,9 @@ static int fanotify_handle_event(struct fsnotify_group *group,
 	BUILD_BUG_ON(FAN_OPEN_PERM != FS_OPEN_PERM);
 	BUILD_BUG_ON(FAN_ACCESS_PERM != FS_ACCESS_PERM);
 	BUILD_BUG_ON(FAN_ONDIR != FS_ISDIR);
+	BUILD_BUG_ON(FAN_OPEN_EXEC != FS_OPEN_EXEC);
 
-	BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 10);
+	BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 11);
 
 	mask = fanotify_group_event_mask(iter_info, mask, data, data_type);
 	if (!mask)
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index d2c34900ae05..b3f58f36a0ab 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -401,7 +401,7 @@ static __init int fsnotify_init(void)
 {
 	int ret;
 
-	BUILD_BUG_ON(HWEIGHT32(ALL_FSNOTIFY_BITS) != 23);
+	BUILD_BUG_ON(HWEIGHT32(ALL_FSNOTIFY_BITS) != 24);
 
 	ret = init_srcu_struct(&fsnotify_mark_srcu);
 	if (ret)
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index a5a60691e48b..c521e4264f2b 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -37,7 +37,7 @@
 
 /* Events that user can request to be notified on */
 #define FANOTIFY_EVENTS		(FAN_ACCESS | FAN_MODIFY | \
-				 FAN_CLOSE | FAN_OPEN)
+				 FAN_CLOSE | FAN_OPEN | FAN_OPEN_EXEC)
 
 /* Events that require a permission response from user */
 #define FANOTIFY_PERM_EVENTS	(FAN_OPEN_PERM | FAN_ACCESS_PERM)
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index fd1ce10553bf..1fe5ac93b252 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -215,6 +215,8 @@ static inline void fsnotify_open(struct file *file)
 
 	if (S_ISDIR(inode->i_mode))
 		mask |= FS_ISDIR;
+	if (file->f_flags & __FMODE_EXEC)
+		mask |= FS_OPEN_EXEC;
 
 	fsnotify_parent(path, NULL, mask);
 	fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 135b973e44d1..39d94e62a836 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -38,6 +38,7 @@
 #define FS_DELETE		0x00000200	/* Subfile was deleted */
 #define FS_DELETE_SELF		0x00000400	/* Self was deleted */
 #define FS_MOVE_SELF		0x00000800	/* Self was moved */
+#define FS_OPEN_EXEC		0x00001000	/* File was opened for exec */
 
 #define FS_UNMOUNT		0x00002000	/* inode on umount fs */
 #define FS_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
@@ -62,7 +63,8 @@
 #define FS_EVENTS_POSS_ON_CHILD   (FS_ACCESS | FS_MODIFY | FS_ATTRIB |\
 				   FS_CLOSE_WRITE | FS_CLOSE_NOWRITE | FS_OPEN |\
 				   FS_MOVED_FROM | FS_MOVED_TO | FS_CREATE |\
-				   FS_DELETE | FS_OPEN_PERM | FS_ACCESS_PERM)
+				   FS_DELETE | FS_OPEN_PERM | FS_ACCESS_PERM | \
+				   FS_OPEN_EXEC)
 
 #define FS_MOVE			(FS_MOVED_FROM | FS_MOVED_TO)
 
@@ -74,7 +76,8 @@
 			     FS_MOVED_FROM | FS_MOVED_TO | FS_CREATE | \
 			     FS_DELETE | FS_DELETE_SELF | FS_MOVE_SELF | \
 			     FS_UNMOUNT | FS_Q_OVERFLOW | FS_IN_IGNORED | \
-			     FS_OPEN_PERM | FS_ACCESS_PERM | FS_DN_RENAME)
+			     FS_OPEN_PERM | FS_ACCESS_PERM | FS_DN_RENAME | \
+			     FS_OPEN_EXEC)
 
 /* Extra flags that may be reported with event or control handling of events */
 #define ALL_FSNOTIFY_FLAGS  (FS_EXCL_UNLINK | FS_ISDIR | FS_IN_ONESHOT | \
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index b86740d1c50a..d9664fbc905b 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -10,6 +10,7 @@
 #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_OPEN_EXEC		0x00001000	/* File was opened for exec */
 
 #define FAN_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
 
-- 
2.17.2


-- 
Matthew Bobrowski

[PATCH v6 3/4] fanotify: introduce new event mask FAN_OPEN_EXEC_PERM

[Matthew Bobrowski]

A new event mask FAN_OPEN_EXEC_PERM has been defined. This allows users
to receive events and grant acccess to files that are intending to be
opened for execution. Events of FAN_OPEN_EXEC_PERM type will be
generated when a file has been opened by using either execve(),
execveat() or uselib() system calls.

This acts in the same manner as previous permission event types, meaning
that an access response is required from the application to permit any
further operations on the file. This feature is implemented within the
fsnotify_perm() hook by setting the FAN_OPEN_EXEC_PERM event type if
__FMODE_EXEC is set within file->f_flags.

Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
---
 fs/notify/fanotify/fanotify.c    |  3 ++-
 fs/notify/fsnotify.c             |  2 +-
 include/linux/fanotify.h         |  3 ++-
 include/linux/fsnotify.h         | 12 +++++++-----
 include/linux/fsnotify_backend.h | 10 ++++++----
 include/uapi/linux/fanotify.h    |  1 +
 6 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index e30f3a1d9699..d9aa505591eb 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -210,8 +210,9 @@ 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);
 	BUILD_BUG_ON(FAN_OPEN_EXEC != FS_OPEN_EXEC);
+	BUILD_BUG_ON(FAN_OPEN_EXEC_PERM != FS_OPEN_EXEC_PERM);
 
-	BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 11);
+	BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 12);
 
 	mask = fanotify_group_event_mask(iter_info, mask, data, data_type);
 	if (!mask)
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index b3f58f36a0ab..ecf09b6243d9 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -401,7 +401,7 @@ static __init int fsnotify_init(void)
 {
 	int ret;
 
-	BUILD_BUG_ON(HWEIGHT32(ALL_FSNOTIFY_BITS) != 24);
+	BUILD_BUG_ON(HWEIGHT32(ALL_FSNOTIFY_BITS) != 25);
 
 	ret = init_srcu_struct(&fsnotify_mark_srcu);
 	if (ret)
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index c521e4264f2b..9e2142795335 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -40,7 +40,8 @@
 				 FAN_CLOSE | FAN_OPEN | FAN_OPEN_EXEC)
 
 /* Events that require a permission response from user */
-#define FANOTIFY_PERM_EVENTS	(FAN_OPEN_PERM | FAN_ACCESS_PERM)
+#define FANOTIFY_PERM_EVENTS	(FAN_OPEN_PERM | FAN_ACCESS_PERM | \
+				 FAN_OPEN_EXEC_PERM)
 
 /* 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/linux/fsnotify.h b/include/linux/fsnotify.h
index 1fe5ac93b252..9c7b594bf540 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -26,7 +26,7 @@ static inline int fsnotify_parent(const struct path *path, struct dentry *dentry
 	return __fsnotify_parent(path, dentry, mask);
 }
 
-/* simple call site for access decisions */
+/* Simple call site for access decisions */
 static inline int fsnotify_perm(struct file *file, int mask)
 {
 	const struct path *path = &file->f_path;
@@ -38,12 +38,14 @@ static inline int fsnotify_perm(struct file *file, int mask)
 		return 0;
 	if (!(mask & (MAY_READ | MAY_OPEN)))
 		return 0;
-	if (mask & MAY_OPEN)
+	if (mask & MAY_OPEN) {
 		fsnotify_mask = FS_OPEN_PERM;
-	else if (mask & MAY_READ)
+
+		if (file->f_flags & __FMODE_EXEC)
+			fsnotify_mask |= FS_OPEN_EXEC_PERM;
+	} else if (mask & MAY_READ) {
 		fsnotify_mask = FS_ACCESS_PERM;
-	else
-		BUG();
+	}
 
 	ret = fsnotify_parent(path, NULL, fsnotify_mask);
 	if (ret)
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 39d94e62a836..150c0acb2f43 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -44,8 +44,9 @@
 #define FS_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
 #define FS_IN_IGNORED		0x00008000	/* last inotify event here */
 
-#define FS_OPEN_PERM		0x00010000	/* open event in an permission hook */
+#define FS_OPEN_PERM		0x00010000	/* open event in a permission hook */
 #define FS_ACCESS_PERM		0x00020000	/* access event in a permissions hook */
+#define FS_OPEN_EXEC_PERM	0x00040000	/* open/exec event in a permission hook */
 
 #define FS_EXCL_UNLINK		0x04000000	/* do not send events if object is unlinked */
 #define FS_ISDIR		0x40000000	/* event occurred against dir */
@@ -64,11 +65,12 @@
 				   FS_CLOSE_WRITE | FS_CLOSE_NOWRITE | FS_OPEN |\
 				   FS_MOVED_FROM | FS_MOVED_TO | FS_CREATE |\
 				   FS_DELETE | FS_OPEN_PERM | FS_ACCESS_PERM | \
-				   FS_OPEN_EXEC)
+				   FS_OPEN_EXEC | FS_OPEN_EXEC_PERM)
 
 #define FS_MOVE			(FS_MOVED_FROM | FS_MOVED_TO)
 
-#define ALL_FSNOTIFY_PERM_EVENTS (FS_OPEN_PERM | FS_ACCESS_PERM)
+#define ALL_FSNOTIFY_PERM_EVENTS (FS_OPEN_PERM | FS_ACCESS_PERM | \
+				  FS_OPEN_EXEC_PERM)
 
 /* Events that can be reported to backends */
 #define ALL_FSNOTIFY_EVENTS (FS_ACCESS | FS_MODIFY | FS_ATTRIB | \
@@ -77,7 +79,7 @@
 			     FS_DELETE | FS_DELETE_SELF | FS_MOVE_SELF | \
 			     FS_UNMOUNT | FS_Q_OVERFLOW | FS_IN_IGNORED | \
 			     FS_OPEN_PERM | FS_ACCESS_PERM | FS_DN_RENAME | \
-			     FS_OPEN_EXEC)
+			     FS_OPEN_EXEC | FS_OPEN_EXEC_PERM)
 
 /* Extra flags that may be reported with event or control handling of events */
 #define ALL_FSNOTIFY_FLAGS  (FS_EXCL_UNLINK | FS_ISDIR | FS_IN_ONESHOT | \
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index d9664fbc905b..909c98fcace2 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -16,6 +16,7 @@
 
 #define FAN_OPEN_PERM		0x00010000	/* File open in perm check */
 #define FAN_ACCESS_PERM		0x00020000	/* File accessed in perm check */
+#define FAN_OPEN_EXEC_PERM	0x00040000	/* File open/exec in perm check */
 
 #define FAN_ONDIR		0x40000000	/* event occurred against dir */
 
-- 
2.17.2


-- 
Matthew Bobrowski

[PATCH v6 4/4] fsnotify: don't merge events FS_OPEN_PERM and FS_OPEN_EXEC_PERM

[Matthew Bobrowski]

Permission events are not to be consolidated into a single event mask.
In order for this to not happen, we require additional calls to
fsnotify_parent() and fsnotify() within the fsnotify_perm() when the
conditon to set FS_OPEN_EXEC_PERM is evaluated to true.

To simplify the code that provides this functionality a simple wrapper
fsnotify_path() has been defined to keep things nice and clean. Other
functions that used the same fsnotify_parent()/fsnotify() call
combination have been updated to use the simplified fsnotify_path()
wrapper.

Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
---
 include/linux/fsnotify.h | 48 ++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 9c7b594bf540..660ffc751352 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -26,6 +26,21 @@ static inline int fsnotify_parent(const struct path *path, struct dentry *dentry
 	return __fsnotify_parent(path, dentry, mask);
 }
 
+/*
+ * Simple wrapper to consolidate calls fsnotify_parent()/fsnotify() when
+ * an event is on a path.
+ */
+static inline int fsnotify_path(struct inode *inode, const struct path *path,
+				__u32 mask)
+{
+	int ret;
+
+	ret = fsnotify_parent(path, NULL, mask);
+	if (ret) return ret;
+
+	return fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
+}
+
 /* Simple call site for access decisions */
 static inline int fsnotify_perm(struct file *file, int mask)
 {
@@ -41,17 +56,15 @@ static inline int fsnotify_perm(struct file *file, int mask)
 	if (mask & MAY_OPEN) {
 		fsnotify_mask = FS_OPEN_PERM;
 
-		if (file->f_flags & __FMODE_EXEC)
-			fsnotify_mask |= FS_OPEN_EXEC_PERM;
+		if (file->f_flags & __FMODE_EXEC) {
+			ret = fsnotify_path(inode, path, FS_OPEN_EXEC_PERM);
+			if (ret) return ret;
+		}
 	} else if (mask & MAY_READ) {
 		fsnotify_mask = FS_ACCESS_PERM;
 	}
 
-	ret = fsnotify_parent(path, NULL, fsnotify_mask);
-	if (ret)
-		return ret;
-
-	return fsnotify(inode, fsnotify_mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
+	return fsnotify_path(inode, path, fsnotify_mask);
 }
 
 /*
@@ -182,10 +195,8 @@ static inline void fsnotify_access(struct file *file)
 	if (S_ISDIR(inode->i_mode))
 		mask |= FS_ISDIR;
 
-	if (!(file->f_mode & FMODE_NONOTIFY)) {
-		fsnotify_parent(path, NULL, mask);
-		fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
-	}
+	if (!(file->f_mode & FMODE_NONOTIFY))
+		fsnotify_path(inode, path, mask);
 }
 
 /*
@@ -200,10 +211,8 @@ static inline void fsnotify_modify(struct file *file)
 	if (S_ISDIR(inode->i_mode))
 		mask |= FS_ISDIR;
 
-	if (!(file->f_mode & FMODE_NONOTIFY)) {
-		fsnotify_parent(path, NULL, mask);
-		fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
-	}
+	if (!(file->f_mode & FMODE_NONOTIFY))
+		fsnotify_path(inode, path, mask);
 }
 
 /*
@@ -220,8 +229,7 @@ static inline void fsnotify_open(struct file *file)
 	if (file->f_flags & __FMODE_EXEC)
 		mask |= FS_OPEN_EXEC;
 
-	fsnotify_parent(path, NULL, mask);
-	fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
+	fsnotify_path(inode, path, mask);
 }
 
 /*
@@ -237,10 +245,8 @@ static inline void fsnotify_close(struct file *file)
 	if (S_ISDIR(inode->i_mode))
 		mask |= FS_ISDIR;
 
-	if (!(file->f_mode & FMODE_NONOTIFY)) {
-		fsnotify_parent(path, NULL, mask);
-		fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
-	}
+	if (!(file->f_mode & FMODE_NONOTIFY))
+		fsnotify_path(inode, path, mask);
 }
 
 /*
-- 
2.17.2


-- 
Matthew Bobrowski

Re: [PATCH v6 4/4] fsnotify: don't merge events FS_OPEN_PERM and FS_OPEN_EXEC_PERM

[Amir Goldstein]

On Wed, Nov 7, 2018 at 1:18 PM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> Permission events are not to be consolidated into a single event mask.
> In order for this to not happen, we require additional calls to
> fsnotify_parent() and fsnotify() within the fsnotify_perm() when the
> conditon to set FS_OPEN_EXEC_PERM is evaluated to true.
>

That shouldn't be a separate patch. it should be squashed into the patch
introducing FS_OPEN_EXEC_PERM there is no reason to have an
interim commit where events are merged.

> To simplify the code that provides this functionality a simple wrapper
> fsnotify_path() has been defined to keep things nice and clean. Other
> functions that used the same fsnotify_parent()/fsnotify() call
> combination have been updated to use the simplified fsnotify_path()
> wrapper.
>

And this should be a separate re-factoring patch.

> Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> ---
>  include/linux/fsnotify.h | 48 ++++++++++++++++++++++------------------
>  1 file changed, 27 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 9c7b594bf540..660ffc751352 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -26,6 +26,21 @@ static inline int fsnotify_parent(const struct path *path, struct dentry *dentry
>         return __fsnotify_parent(path, dentry, mask);
>  }
>
> +/*
> + * Simple wrapper to consolidate calls fsnotify_parent()/fsnotify() when
> + * an event is on a path.
> + */
> +static inline int fsnotify_path(struct inode *inode, const struct path *path,
> +                               __u32 mask)
> +{
> +       int ret;
> +
> +       ret = fsnotify_parent(path, NULL, mask);
> +       if (ret) return ret;
> +
> +       return fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
> +}
> +
>  /* Simple call site for access decisions */
>  static inline int fsnotify_perm(struct file *file, int mask)
>  {
> @@ -41,17 +56,15 @@ static inline int fsnotify_perm(struct file *file, int mask)
>         if (mask & MAY_OPEN) {
>                 fsnotify_mask = FS_OPEN_PERM;
>
> -               if (file->f_flags & __FMODE_EXEC)
> -                       fsnotify_mask |= FS_OPEN_EXEC_PERM;
> +               if (file->f_flags & __FMODE_EXEC) {
> +                       ret = fsnotify_path(inode, path, FS_OPEN_EXEC_PERM);
> +                       if (ret) return ret;

return should be in newline - just was just me hand writing a patch in email...

After making these small fixes, you may add to patches:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

Re: [PATCH v6 4/4] fsnotify: don't merge events FS_OPEN_PERM and FS_OPEN_EXEC_PERM

[Jan Kara]

On Wed 07-11-18 13:30:55, Amir Goldstein wrote:
> On Wed, Nov 7, 2018 at 1:18 PM Matthew Bobrowski
> <mbobrowski@mbobrowski.org> wrote:
> >
> > Permission events are not to be consolidated into a single event mask.
> > In order for this to not happen, we require additional calls to
> > fsnotify_parent() and fsnotify() within the fsnotify_perm() when the
> > conditon to set FS_OPEN_EXEC_PERM is evaluated to true.
> >
> 
> That shouldn't be a separate patch. it should be squashed into the patch
> introducing FS_OPEN_EXEC_PERM there is no reason to have an
> interim commit where events are merged.

Agreed.

> > To simplify the code that provides this functionality a simple wrapper
> > fsnotify_path() has been defined to keep things nice and clean. Other
> > functions that used the same fsnotify_parent()/fsnotify() call
> > combination have been updated to use the simplified fsnotify_path()
> > wrapper.
> >
> 
> And this should be a separate re-factoring patch.

And agreed too. You can put this refactoring commit before the one
introducing FS_OPEN_EXEC_PERM to make your life simpler...

								Honza
> 
> > Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> > ---
> >  include/linux/fsnotify.h | 48 ++++++++++++++++++++++------------------
> >  1 file changed, 27 insertions(+), 21 deletions(-)
> >
> > diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> > index 9c7b594bf540..660ffc751352 100644
> > --- a/include/linux/fsnotify.h
> > +++ b/include/linux/fsnotify.h
> > @@ -26,6 +26,21 @@ static inline int fsnotify_parent(const struct path *path, struct dentry *dentry
> >         return __fsnotify_parent(path, dentry, mask);
> >  }
> >
> > +/*
> > + * Simple wrapper to consolidate calls fsnotify_parent()/fsnotify() when
> > + * an event is on a path.
> > + */
> > +static inline int fsnotify_path(struct inode *inode, const struct path *path,
> > +                               __u32 mask)
> > +{
> > +       int ret;
> > +
> > +       ret = fsnotify_parent(path, NULL, mask);
> > +       if (ret) return ret;
> > +
> > +       return fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
> > +}
> > +
> >  /* Simple call site for access decisions */
> >  static inline int fsnotify_perm(struct file *file, int mask)
> >  {
> > @@ -41,17 +56,15 @@ static inline int fsnotify_perm(struct file *file, int mask)
> >         if (mask & MAY_OPEN) {
> >                 fsnotify_mask = FS_OPEN_PERM;
> >
> > -               if (file->f_flags & __FMODE_EXEC)
> > -                       fsnotify_mask |= FS_OPEN_EXEC_PERM;
> > +               if (file->f_flags & __FMODE_EXEC) {
> > +                       ret = fsnotify_path(inode, path, FS_OPEN_EXEC_PERM);
> > +                       if (ret) return ret;
> 
> return should be in newline - just was just me hand writing a patch in email...
> 
> After making these small fixes, you may add to patches:
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> 
> Thanks,
> Amir.
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v6 4/4] fsnotify: don't merge events FS_OPEN_PERM and FS_OPEN_EXEC_PERM

[Matthew Bobrowski]

On Wed, Nov 07, 2018 at 03:15:53PM +0100, Jan Kara wrote:
> On Wed 07-11-18 13:30:55, Amir Goldstein wrote:
> > On Wed, Nov 7, 2018 at 1:18 PM Matthew Bobrowski
> > <mbobrowski@mbobrowski.org> wrote:
> > >
> > > Permission events are not to be consolidated into a single event mask.
> > > In order for this to not happen, we require additional calls to
> > > fsnotify_parent() and fsnotify() within the fsnotify_perm() when the
> > > conditon to set FS_OPEN_EXEC_PERM is evaluated to true.
> > >
> > 
> > That shouldn't be a separate patch. it should be squashed into the patch
> > introducing FS_OPEN_EXEC_PERM there is no reason to have an
> > interim commit where events are merged.
> 
> Agreed.
> 
> > > To simplify the code that provides this functionality a simple wrapper
> > > fsnotify_path() has been defined to keep things nice and clean. Other
> > > functions that used the same fsnotify_parent()/fsnotify() call
> > > combination have been updated to use the simplified fsnotify_path()
> > > wrapper.
> > >
> > 
> > And this should be a separate re-factoring patch.
> 
> And agreed too. You can put this refactoring commit before the one
> introducing FS_OPEN_EXEC_PERM to make your life simpler...

OK, no problem.

> > return should be in newline - just was just me hand writing a patch in email...
> > 
> > After making these small fixes, you may add to patches:
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks Amir!

I will send through v7 shortly.

-- 
Matthew Bobrowski <mbobrowski@mbobrowski.org>