[PATCH v4 0/3] fanotify: introduce new event types FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM

[PATCH v4 0/3] fanotify: introduce new event types 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 types FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM have
been introduced in order to provide users 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.

I've also written the required updates for the man-pages project. You can
find the necessary changes for these new event types within the following
commit:

https://github.com/matthewbobrowski/man-pages/commit/d075dd8c8dfe19fccb9ea91f9550ea41b6e67334

Please note that all modifications here are based on the changes Amir has
made around deprecating some of the previously exposed UAPI constants. The
branch which my changes are based on can be found here:

https://github.com/amir73il/linux/tree/fanotify_api-v3

Lastly, thanks to both Amir and Jan for their help and feedback along the
way, truly appreciated.

---

Matthew Bobrowski (3):
  fanotify: introduce new event type FAN_OPEN_EXEC
  fanotify: return only user requested event types in event mask
  fanotify: introduce new event type FAN_OPEN_EXEC_PERM

 fs/notify/fanotify/fanotify.c    | 50 ++++++++++++++++++--------------
 fs/notify/fsnotify.c             |  2 +-
 include/linux/fanotify.h         |  5 ++--
 include/linux/fsnotify.h         | 12 +++++---
 include/linux/fsnotify_backend.h | 10 +++++--
 include/uapi/linux/fanotify.h    |  2 ++
 6 files changed, 50 insertions(+), 31 deletions(-)

-- 
2.17.2

[PATCH v4 1/3] fanotify: introduce new event type FAN_OPEN_EXEC

[Matthew Bobrowski]

A new event type 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.

The FAN_OPEN_EXEC flag is set within the fsnotify_open() hook only if a
file has the __FMODE_EXEC bit 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 2c57186caa2e..b3e92302ed84 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -207,8 +207,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);
 
 	if (!fanotify_should_send_event(iter_info, mask, data, data_type))
 		return 0;
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index a0ad06285450..051e5fc0dba1 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -402,7 +402,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 05b696b4856b..0e07d23b6c17 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -39,7 +39,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..329ac6684326 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 00b2304ed124..da278f11ab29 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

[PATCH v4 2/3] fanotify: return only user requested event types in event mask

[Matthew Bobrowski]

Modified fanotify_should_send_event() so that it now returns a mask for
a 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 | 46 ++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index b3e92302ed84..9da334d343b8 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 a 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 ecplicitly
+ * 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,21 +107,21 @@ 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))
 			continue;
 		mark = iter_info->marks[type];
 		/*
-		 * if the event is for a child and this inode doesn't care about
+		 * If the event is for a child and this inode doesn't care about
 		 * events on the child, don't send it!
 		 */
 		if (type == FSNOTIFY_OBJ_TYPE_INODE &&
@@ -129,13 +135,10 @@ static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
 
 	if (d_is_dir(path->dentry) &&
 	    !(marks_mask & FS_ISDIR & ~marks_ignored_mask))
-		return false;
-
-	if (event_mask & FANOTIFY_OUTGOING_EVENTS &
-	    marks_mask & ~marks_ignored_mask)
-		return true;
+		return 0;
 
-	return false;
+	return event_mask & FANOTIFY_OUTGOING_EVENTS & marks_mask &
+		~marks_ignored_mask;
 }
 
 struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
@@ -194,6 +197,7 @@ static int fanotify_handle_event(struct fsnotify_group *group,
 				 struct fsnotify_iter_info *iter_info)
 {
 	int ret = 0;
+	u32 event_mask = 0;
 	struct fanotify_event_info *event;
 	struct fsnotify_event *fsn_event;
 
@@ -211,13 +215,15 @@ static int fanotify_handle_event(struct fsnotify_group *group,
 
 	BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 11);
 
-	if (!fanotify_should_send_event(iter_info, mask, data, data_type))
+	event_mask = fanotify_group_event_mask(iter_info, mask, data,
+						data_type);
+	if (!event_mask)
 		return 0;
 
 	pr_debug("%s: group=%p inode=%p mask=%x\n", __func__, group, inode,
-		 mask);
+		 event_mask);
 
-	if (fanotify_is_perm_event(mask)) {
+	if (fanotify_is_perm_event(event_mask)) {
 		/*
 		 * fsnotify_prepare_user_wait() fails if we race with mark
 		 * deletion.  Just let the operation pass in that case.
@@ -226,14 +232,14 @@ static int fanotify_handle_event(struct fsnotify_group *group,
 			return 0;
 	}
 
-	event = fanotify_alloc_event(group, inode, mask, data);
+	event = fanotify_alloc_event(group, inode, event_mask, data);
 	ret = -ENOMEM;
 	if (unlikely(!event)) {
 		/*
 		 * We don't queue overflow events for permission events as
 		 * there the access is denied and so no event is in fact lost.
 		 */
-		if (!fanotify_is_perm_event(mask))
+		if (!fanotify_is_perm_event(event_mask))
 			fsnotify_queue_overflow(group);
 		goto finish;
 	}
@@ -242,18 +248,18 @@ static int fanotify_handle_event(struct fsnotify_group *group,
 	ret = fsnotify_add_event(group, fsn_event, fanotify_merge);
 	if (ret) {
 		/* Permission events shouldn't be merged */
-		BUG_ON(ret == 1 && mask & FANOTIFY_PERM_EVENTS);
+		BUG_ON(ret == 1 && event_mask & FANOTIFY_PERM_EVENTS);
 		/* Our event wasn't used in the end. Free it. */
 		fsnotify_destroy_event(group, fsn_event);
 
 		ret = 0;
-	} else if (fanotify_is_perm_event(mask)) {
+	} else if (fanotify_is_perm_event(event_mask)) {
 		ret = fanotify_get_response(group, FANOTIFY_PE(fsn_event),
 					    iter_info);
 		fsnotify_destroy_event(group, fsn_event);
 	}
 finish:
-	if (fanotify_is_perm_event(mask))
+	if (fanotify_is_perm_event(event_mask))
 		fsnotify_finish_user_wait(iter_info);
 
 	return ret;
-- 
2.17.2

[PATCH v4 3/3] fanotify: introduce new event type FAN_OPEN_EXEC_PERM

[Matthew Bobrowski]

A new event type FAN_OPEN_EXEC_PERM has been defined. This allows users
to receive and grant access to files that are intending to be opened for
execution.

This acts in the same manor as previous permission event types, meaning
that a access response is required from the application to permit
further operations on the file. The event flag is set specifically when
the __FMODE_EXEC flag is set within file->f_flags in the fsnotify_perm()
hook.

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         | 10 ++++++----
 include/linux/fsnotify_backend.h |  7 ++++---
 include/uapi/linux/fanotify.h    |  1 +
 6 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 9da334d343b8..bae9a312d772 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -212,8 +212,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);
 
 	event_mask = fanotify_group_event_mask(iter_info, mask, data,
 						data_type);
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 051e5fc0dba1..40ed97aede29 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -402,7 +402,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 0e07d23b6c17..614943153596 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -42,7 +42,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..6fb8d1fcfeaf 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -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 329ac6684326..96616651220c 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 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,7 +65,7 @@
 				   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)
 
@@ -77,7 +78,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 da278f11ab29..e2df10c61cb1 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

Re: [PATCH v4 0/3] fanotify: introduce new event types FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM

[Amir Goldstein]

On Thu, Oct 11, 2018 at 1:42 PM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> 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 types FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM have
> been introduced in order to provide users 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.
>
> I've also written the required updates for the man-pages project. You can
> find the necessary changes for these new event types within the following
> commit:
>
> https://github.com/matthewbobrowski/man-pages/commit/d075dd8c8dfe19fccb9ea91f9550ea41b6e67334
>
> Please note that all modifications here are based on the changes Amir has
> made around deprecating some of the previously exposed UAPI constants. The
> branch which my changes are based on can be found here:
>
> https://github.com/amir73il/linux/tree/fanotify_api-v3
>

There is already a newer version merge to Jan's fsnotify branch.
You should reabse on that branch, althrough I don't see any immediate
merge conflicts.

> Lastly, thanks to both Amir and Jan for their help and feedback along the
> way, truly appreciated.
>

Jan,

You may add
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

on the series.

Thanks,
Amir.

Re: [PATCH v4 2/3] fanotify: return only user requested event types in event mask

[Amir Goldstein]

On Thu, Oct 11, 2018 at 1:42 PM Matthew Bobrowski
<mbobrowski@mbobrowski.org> wrote:
>
> Modified fanotify_should_send_event() so that it now returns a mask for
> a 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>
> ---

I feel that this story could be shorter and less confusing, but
I cannot offer a better version, nor do I object to this version.
Maybe Jan can give it some final touches.

Thanks,
Amir.

Re: [PATCH v4 2/3] fanotify: return only user requested event types in event mask

[Jan Kara]

Hi,

the patch looks good. Just couple nits:

On Thu 11-10-18 21:42:41, Matthew Bobrowski wrote:
> Modified fanotify_should_send_event() so that it now returns a mask for
  ^^ Modify

> a event that contains ONLY flags for the event types that have been
  ^ an

> 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 | 46 ++++++++++++++++++++---------------
>  1 file changed, 26 insertions(+), 20 deletions(-)
> 
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index b3e92302ed84..9da334d343b8 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 a event that only contains the flags
				      ^^ an

> + * that have been specifically requested by the user. Flags that may have
> + * been included within the event mask, but have not been ecplicitly
								^^ 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)

<snip>

>  struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
> @@ -194,6 +197,7 @@ static int fanotify_handle_event(struct fsnotify_group *group,
>  				 struct fsnotify_iter_info *iter_info)
>  {
>  	int ret = 0;
> +	u32 event_mask = 0;
>  	struct fanotify_event_info *event;
>  	struct fsnotify_event *fsn_event;
>  
> @@ -211,13 +215,15 @@ static int fanotify_handle_event(struct fsnotify_group *group,
>  
>  	BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 11);
>  
> -	if (!fanotify_should_send_event(iter_info, mask, data, data_type))
> +	event_mask = fanotify_group_event_mask(iter_info, mask, data,
> +						data_type);
        ^^^ Why don't you store the result in 'mask'? You don't need
the original mask later anyway, it reduces churn and also possibility of
getting things wrong in the future...

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v4 1/3] fanotify: introduce new event type FAN_OPEN_EXEC

[Jan Kara]

On Thu 11-10-18 21:42:23, Matthew Bobrowski wrote:
> A new event type 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.

Can we perhaps expand this a bit to clarify that this means execve(),
execve_at(), and uselib() syscalls? Then the sentence below could just
clarify that feature is implemented by fanotify_open() generating
FAN_OPEN_EXEC event if __FMODE_EXEC is set within file->f_flags. Thanks!

								Honza
 
> The FAN_OPEN_EXEC flag is set within the fsnotify_open() hook only if a
> file has the __FMODE_EXEC bit 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 2c57186caa2e..b3e92302ed84 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -207,8 +207,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);
>  
>  	if (!fanotify_should_send_event(iter_info, mask, data, data_type))
>  		return 0;
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index a0ad06285450..051e5fc0dba1 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -402,7 +402,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 05b696b4856b..0e07d23b6c17 100644
> --- a/include/linux/fanotify.h
> +++ b/include/linux/fanotify.h
> @@ -39,7 +39,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..329ac6684326 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 00b2304ed124..da278f11ab29 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
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v4 3/3] fanotify: introduce new event type FAN_OPEN_EXEC_PERM

[Jan Kara]

On Thu 11-10-18 21:43:09, Matthew Bobrowski wrote:
> A new event type FAN_OPEN_EXEC_PERM has been defined. This allows users
> to receive and grant access to files that are intending to be opened for
> execution.

The same note here as for FAN_OPEN_EXEC.

> This acts in the same manor as previous permission event types, meaning
> that a access response is required from the application to permit
> further operations on the file. The event flag is set specifically when
> the __FMODE_EXEC flag is set within file->f_flags in the fsnotify_perm()
> hook.
> 
> Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>

And a general question: Do I understand right that your audit application
needs this event type?  I'm asking because so far what Steve wrote me was
about pattern matching of fanotify events to detect when something happens.
So it's not clear to me how permission event fits into that.

								Honza

> ---
>  fs/notify/fanotify/fanotify.c    |  3 ++-
>  fs/notify/fsnotify.c             |  2 +-
>  include/linux/fanotify.h         |  3 ++-
>  include/linux/fsnotify.h         | 10 ++++++----
>  include/linux/fsnotify_backend.h |  7 ++++---
>  include/uapi/linux/fanotify.h    |  1 +
>  6 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> index 9da334d343b8..bae9a312d772 100644
> --- a/fs/notify/fanotify/fanotify.c
> +++ b/fs/notify/fanotify/fanotify.c
> @@ -212,8 +212,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);
>  
>  	event_mask = fanotify_group_event_mask(iter_info, mask, data,
>  						data_type);
> diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> index 051e5fc0dba1..40ed97aede29 100644
> --- a/fs/notify/fsnotify.c
> +++ b/fs/notify/fsnotify.c
> @@ -402,7 +402,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 0e07d23b6c17..614943153596 100644
> --- a/include/linux/fanotify.h
> +++ b/include/linux/fanotify.h
> @@ -42,7 +42,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..6fb8d1fcfeaf 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -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 329ac6684326..96616651220c 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 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,7 +65,7 @@
>  				   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)
>  
> @@ -77,7 +78,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 da278f11ab29..e2df10c61cb1 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
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH v4 2/3] fanotify: return only user requested event types in event mask

[Matthew Bobrowski]

On Tue, Oct 16, 2018 at 01:52:15PM +0200, Jan Kara wrote:
> 
> Hi,
> 
> the patch looks good. Just couple nits:
> 
> On Thu 11-10-18 21:42:41, Matthew Bobrowski wrote:
> > Modified fanotify_should_send_event() so that it now returns a mask for
>   ^^ Modify

Updated.

> > a event that contains ONLY flags for the event types that have been
>   ^ an

Updated.

> > 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 | 46 ++++++++++++++++++++---------------
> >  1 file changed, 26 insertions(+), 20 deletions(-)
> > 
> > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > index b3e92302ed84..9da334d343b8 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 a event that only contains the flags
> 				      ^^ an

Updated.

> > + * that have been specifically requested by the user. Flags that may have
> > + * been included within the event mask, but have not been ecplicitly
> 								^^ explicitly

Updated.

> > + * 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)
> 
> <snip>
> 
> >  struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
> > @@ -194,6 +197,7 @@ static int fanotify_handle_event(struct fsnotify_group *group,
> >  				 struct fsnotify_iter_info *iter_info)
> >  {
> >  	int ret = 0;
> > +	u32 event_mask = 0;
> >  	struct fanotify_event_info *event;
> >  	struct fsnotify_event *fsn_event;
> >  
> > @@ -211,13 +215,15 @@ static int fanotify_handle_event(struct fsnotify_group *group,
> >  
> >  	BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 11);
> >  
> > -	if (!fanotify_should_send_event(iter_info, mask, data, data_type))
> > +	event_mask = fanotify_group_event_mask(iter_info, mask, data,
> > +						data_type);
>         ^^^ Why don't you store the result in 'mask'? You don't need
> the original mask later anyway, it reduces churn and also possibility of
> getting things wrong in the future...

Good point. There's no real reason why we can't overwrite the original
'mask' value with the returned value from fanotify_group_event_mask().
That being said, I've gone ahead and applied updates accordingly.

I will post through the updates within an updated patch series.

Re: [PATCH v4 1/3] fanotify: introduce new event type FAN_OPEN_EXEC

[Matthew Bobrowski]

On Tue, Oct 16, 2018 at 01:58:32PM +0200, Jan Kara wrote:
> On Thu 11-10-18 21:42:23, Matthew Bobrowski wrote:
> > A new event type 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.
> 
> Can we perhaps expand this a bit to clarify that this means execve(),
> execve_at(), and uselib() syscalls? Then the sentence below could just
> clarify that feature is implemented by fanotify_open() generating
> FAN_OPEN_EXEC event if __FMODE_EXEC is set within file->f_flags. Thanks!

Updated.

I will include these updates within the new patch series I'm going to post
through.

> > The FAN_OPEN_EXEC flag is set within the fsnotify_open() hook only if a
> > file has the __FMODE_EXEC bit 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 2c57186caa2e..b3e92302ed84 100644
> > --- a/fs/notify/fanotify/fanotify.c
> > +++ b/fs/notify/fanotify/fanotify.c
> > @@ -207,8 +207,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);
> >  
> >  	if (!fanotify_should_send_event(iter_info, mask, data, data_type))
> >  		return 0;
> > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> > index a0ad06285450..051e5fc0dba1 100644
> > --- a/fs/notify/fsnotify.c
> > +++ b/fs/notify/fsnotify.c
> > @@ -402,7 +402,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 05b696b4856b..0e07d23b6c17 100644
> > --- a/include/linux/fanotify.h
> > +++ b/include/linux/fanotify.h
> > @@ -39,7 +39,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..329ac6684326 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 00b2304ed124..da278f11ab29 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
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Re: [PATCH v4 3/3] fanotify: introduce new event type FAN_OPEN_EXEC_PERM

[Matthew Bobrowski]

On Tue, Oct 16, 2018 at 02:04:20PM +0200, Jan Kara wrote:
> On Thu 11-10-18 21:43:09, Matthew Bobrowski wrote:
> > A new event type FAN_OPEN_EXEC_PERM has been defined. This allows users
> > to receive and grant access to files that are intending to be opened for
> > execution.
> 
> The same note here as for FAN_OPEN_EXEC.

Updated.

I will include the updates to this within the updated patch series.

> > This acts in the same manor as previous permission event types, meaning
> > that a access response is required from the application to permit
> > further operations on the file. The event flag is set specifically when
> > the __FMODE_EXEC flag is set within file->f_flags in the fsnotify_perm()
> > hook.
> > 
> > Signed-off-by: Matthew Bobrowski <mbobrowski@mbobrowski.org>
> 
> And a general question: Do I understand right that your audit application
> needs this event type?  

Yes, your understanding is correct. The application we're developing
requires this event type.

> I'm asking because so far what Steve wrote me was about pattern matching
> of fanotify events to detect when something happens. So it's not clear
> to me how permission event fits into that.

That detection for when "something happens", is precisely to do with
understanding when a file is opened for execution. We use permission
events within the application we're developing to either ALLOW or DENY a
particular action being performed against an object. At the moment, we
don't have any way to determine what actually caused a watched object to
be opened, so this is to aid with that, and also help us make a better
judgment how we're to respond to a particular action.

> > ---
> >  fs/notify/fanotify/fanotify.c    |  3 ++-
> >  fs/notify/fsnotify.c             |  2 +-
> >  include/linux/fanotify.h         |  3 ++-
> >  include/linux/fsnotify.h         | 10 ++++++----
> >  include/linux/fsnotify_backend.h |  7 ++++---
> >  include/uapi/linux/fanotify.h    |  1 +
> >  6 files changed, 16 insertions(+), 10 deletions(-)
> > 
> > diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
> > index 9da334d343b8..bae9a312d772 100644
> > --- a/fs/notify/fanotify/fanotify.c
> > +++ b/fs/notify/fanotify/fanotify.c
> > @@ -212,8 +212,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);
> >  
> >  	event_mask = fanotify_group_event_mask(iter_info, mask, data,
> >  						data_type);
> > diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
> > index 051e5fc0dba1..40ed97aede29 100644
> > --- a/fs/notify/fsnotify.c
> > +++ b/fs/notify/fsnotify.c
> > @@ -402,7 +402,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 0e07d23b6c17..614943153596 100644
> > --- a/include/linux/fanotify.h
> > +++ b/include/linux/fanotify.h
> > @@ -42,7 +42,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..6fb8d1fcfeaf 100644
> > --- a/include/linux/fsnotify.h
> > +++ b/include/linux/fsnotify.h
> > @@ -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 329ac6684326..96616651220c 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 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,7 +65,7 @@
> >  				   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)
> >  
> > @@ -77,7 +78,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 da278f11ab29..e2df10c61cb1 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
> > 
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

Re: [PATCH v4 3/3] fanotify: introduce new event type FAN_OPEN_EXEC_PERM

[Jan Kara]

On Wed 17-10-18 17:33:18, Matthew Bobrowski wrote:
> > And a general question: Do I understand right that your audit application
> > needs this event type?  
> 
> Yes, your understanding is correct. The application we're developing
> requires this event type.
> 
> > I'm asking because so far what Steve wrote me was about pattern matching
> > of fanotify events to detect when something happens. So it's not clear
> > to me how permission event fits into that.
> 
> That detection for when "something happens", is precisely to do with
> understanding when a file is opened for execution. We use permission
> events within the application we're developing to either ALLOW or DENY a
> particular action being performed against an object. At the moment, we
> don't have any way to determine what actually caused a watched object to
> be opened, so this is to aid with that, and also help us make a better
> judgment how we're to respond to a particular action.

OK, understood. I just wanted to be sure I understand what you need
correctly. Thanks for explanation.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR