fanotify: introduce new event types FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM

PATCH Superseded Matthew Bobrowski 3 participants 12 messages Oct 17, 2018 – Oct 24, 2018

Subsystems (3)

VFS Fsnotify: filesystem notification infrastructure Fanotify

lore

Share & tools

lore

Matthew Bobrowski

author

2018-10-17 16:59 UTC #0 lore raw

v5 cover fanotify: introduce new event types FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM

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.

Changes since v4:
Thanks to both Amir and Jan for the review, much appreciated!

* Patch 1: "fanotify: introduce new event type FAN_OPEN_EXEC"
* Updated the changelog to clarify that the event type FAN_OPEN_EXEC is set only when a file is opened using the syscalls execve(), execveat() or uselib()

* Patch 2: "fanotify: return only user requested event types in event mask"
* Rectified grammar issues within commit message
* Rectified spelling and grammer issues within the function comment
* Updated fanotify_hande_event() so that the value returned from fanotify_group_event_mask() stores the result in the existing 'mask' variable as oppose to a newly created variable

* Patch 3: "fanotify: introduce new event type FAN_OPEN_EXEC_PERM"
* Updated the changelog to clarify that the event FAN_OPEN_EXEC_PERM is set only when a fie is opened using the syscalls execve(), execveat() or uselib()

I've also written the required updates for the man-pages project. It includes descriptions about the newly available event types and some additional notes around what actions will see these flags. You can find the changes here:

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

---

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    | 34 +++++++++++++++++++-------------
 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, 41 insertions(+), 24 deletions(-)

Signature · 1 line

2.17.2

Matthew Bobrowski

author

2018-10-17 16:59 UTC #1 lore raw

PATCH v5 1/3 fanotify: introduce new event type FAN_OPEN_EXEC

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. Events of FAN_OPEN_EXEC type will be generated when a file has been opened by 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 <[email protected]>

---
 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(-)

fs/notify/fanotify/fanotify.c +2 -1

index 2c57186caa2e..b3e92302ed84 100644
@@ -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;

fs/notify/fsnotify.c +1 -1

index a0ad06285450..051e5fc0dba1 100644
@@ -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)

include/linux/fanotify.h +1 -1

index 05b696b4856b..0e07d23b6c17 100644
@@ -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)

include/linux/fsnotify.h +2 -0

index fd1ce10553bf..1fe5ac93b252 100644
@@ -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);

include/linux/fsnotify_backend.h +5 -2

index 135b973e44d1..329ac6684326 100644
@@ -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 | \

include/uapi/linux/fanotify.h +1 -0

index 00b2304ed124..da278f11ab29 100644
@@ -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 */

Signature · 1 line

2.17.2

Matthew Bobrowski

author

2018-10-17 17:00 UTC #2 lore raw

PATCH v5 2/3 fanotify: return only user requested event types in event mask

In reply to 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 <[email protected]>

---
 fs/notify/fanotify/fanotify.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

fs/notify/fanotify/fanotify.c +17 ~~-13~~

index b3e92302ed84..8a49a0adab73 100644
@@ -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,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,

@@ -211,7 +214,8 @@ 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))
+	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,

Signature · 1 line

2.17.2

Matthew Bobrowski

author

2018-10-17 17:00 UTC #3 lore raw

PATCH v5 3/3 fanotify: introduce new event type FAN_OPEN_EXEC_PERM

In reply to 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. 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 manor as previous permission event types, meaning that an access response is required from the application to permit further operations on the file. This feature is implemented within the fsnotify_perm() hook by setting FAN_OPEN_EXEC_PERM event type if __FMODE_EXEC is set within file->f_flags.

Signed-off-by Matthew Bobrowski <[email protected]>

---
 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(-)

fs/notify/fanotify/fanotify.c +2 -1

index 8a49a0adab73..a427daf8d117 100644
@@ -211,8 +211,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)

fs/notify/fsnotify.c +1 -1

index 051e5fc0dba1..40ed97aede29 100644
@@ -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)

include/linux/fanotify.h +2 -1

index 0e07d23b6c17..614943153596 100644
@@ -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)

include/linux/fsnotify.h +6 -4

index 1fe5ac93b252..6fb8d1fcfeaf 100644
@@ -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)

include/linux/fsnotify_backend.h +4 -3

index 329ac6684326..96616651220c 100644
@@ -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 | \

include/uapi/linux/fanotify.h +1 -0

index da278f11ab29..e2df10c61cb1 100644
@@ -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 */

Signature · 1 line

2.17.2

Jan Kara

maintainer

2018-10-18 17:26 UTC #4 lore raw

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

include/linux/fsnotify_backend.h line 47

Quoted 10 lines · Matthew Bobrowski

include/linux/fsnotify_backend.h +1 -1

index 329ac6684326..96616651220c 100644
@@ -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 */

This hunk would belong to patch 1 instead of this one.

Quoted 6 lines

include/linux/fsnotify_backend.h +1 -0

 #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 */

include/linux/fsnotify_backend.h line 71

Quoted 8 lines

include/linux/fsnotify_backend.h +1 -1

@@ -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)

You seem to be missing an update of ALL_FSNOTIFY_PERM_EVENTS to also include FS_OPEN_EXEC_PERM.

Otherwise the patch looks good to me.

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Matthew Bobrowski

author

2018-10-18 19:56 UTC #5 lore raw

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

Quoted 9 lines · Jan Kara

Quoted 8 lines · Matthew Bobrowski

include/linux/fsnotify_backend.h +0 -0

index 329ac6684326..96616651220c 100644
@@ -44,8 +44,9 @@
 #define FS_Q_OVERFLOW		0x00004000	/* Event queued overflowed */
 #define FS_IN_IGNORED		0x00008000	/* last inotify event here */

Quoted 4 lines

include/linux/fsnotify_backend.h line 47
Quoted 2 lines
include/linux/fsnotify_backend.h +1 -1
-#define FS_OPEN_PERM		0x00010000	/* open event in an permission hook */
+#define FS_OPEN_PERM		0x00010000	/* open event in a permission hook */
This hunk would belong to patch 1 instead of this one.

Strange and totally agree. I'm quite perplexed as to why this came through in like this in this particular patch. Something really weird must've happened when I was "amending" previously recommended updates.

Quoted 13 lines

include/linux/fsnotify_backend.h +2 -1

 #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)

Quoted 5 lines

include/linux/fsnotify_backend.h line 71
Quoted 2 lines
include/linux/fsnotify_backend.h +0 -0
 #define FS_MOVE			(FS_MOVED_FROM | FS_MOVED_TO)
 
You seem to be missing an update of ALL_FSNOTIFY_PERM_EVENTS to also include FS_OPEN_EXEC_PERM.

Oh, yep! Thanks for pointing that out.

Quoted 1 line

Otherwise the patch looks good to me.

:-)

Signature · 1 line

Matthew Bobrowski <[email protected]>

Jan Kara

maintainer

2018-10-18 17:28 UTC #6 lore raw

In reply to Matthew Bobrowski

Quoted 4 lines · 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.

OK, so these patches look mostly ready (except for a small bits I've commented on patch 3). How about the LTP test to actually test them? Once that exists, I can test the patches and merge them into my tree...

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Matthew Bobrowski

author

2018-10-18 20:07 UTC #7 lore raw

Quoted 8 lines · Jan Kara

Quoted 4 lines · 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.
OK, so these patches look mostly ready (except for a small bits I've commented on patch 3).

OK, great. I can go ahead and perform the necessary updates. These are minor though, so I'm not sure whether it's appropriate to actually send through and entire new patch series that include these changes? To be fair I'm not entirely sure how you're meant to really submit through these minor updates based on previous reviews...

Quoted 2 lines

How about the LTP test to actually test them? Once that exists, I can test the patches and merge them into my tree...

Yes, I'm working on them at the moment. I've got some time coming up this weekend, so I hope to have them ready for you by then. It's a little fiddly when it comes to incorporating the exec events into previous tests. The way they're currently written doesn't really allow for them to be somewhat extensible in my opinion. I've written a new test in a completely separate test file, which is easy enough; however I think that these new event types should most definitely be part of perviously written tests i.e. fanotify03 for FAN_OPEN_EXEC_PERM. I've discussed this with Amir and he also agrees.

Signature · 1 line

Matthew Bobrowski <[email protected]>

Jan Kara

maintainer

2018-10-18 20:49 UTC #8 lore raw

Quoted 7 lines · Matthew Bobrowski

Quoted 6 lines · Jan Kara
Quoted 4 lines · 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.

Quoted 8 lines

Quoted 2 lines
OK, so these patches look mostly ready (except for a small bits I've commented on patch 3).
OK, great. I can go ahead and perform the necessary updates. These are minor though, so I'm not sure whether it's appropriate to actually send through and entire new patch series that include these changes? To be fair I'm not entirely sure how you're meant to really submit through these minor updates based on previous reviews...

I guess I can just make the changes when picking up patches.

Quoted 12 lines

Quoted 2 lines
How about the LTP test to actually test them? Once that exists, I can test the patches and merge them into my tree...
Yes, I'm working on them at the moment. I've got some time coming up this weekend, so I hope to have them ready for you by then. It's a little fiddly when it comes to incorporating the exec events into previous tests. The way they're currently written doesn't really allow for them to be somewhat extensible in my opinion. I've written a new test in a completely separate test file, which is easy enough; however I think that these new event types should most definitely be part of perviously written tests i.e. fanotify03 for FAN_OPEN_EXEC_PERM. I've discussed this with Amir and he also agrees.

Yeah, that would be nice.

Honza

Signature · 2 lines

Jan Kara <[email protected]>
SUSE Labs, CR

Matthew Bobrowski

author

2018-10-24 13:18 UTC #9 lore raw

Hi Jan,

Quoted 11 lines · Jan Kara

Quoted 10 lines · Matthew Bobrowski
Quoted 8 lines · Jan Kara
Quoted 4 lines · 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.
OK, so these patches look mostly ready (except for a small bits I've commented on patch 3).

Quoted 7 lines

Quoted 5 lines
OK, great. I can go ahead and perform the necessary updates. These are minor though, so I'm not sure whether it's appropriate to actually send through and entire new patch series that include these changes? To be fair I'm not entirely sure how you're meant to really submit through these minor updates based on previous reviews...
I guess I can just make the changes when picking up patches.

If you like, you may obtain a copy of patch 3 for this patch series from the link I've provided below. It contains the minor update around adding FS_OPEN_EXEC_PERM to ALL_FSNOTIFY_PERM_EVENTS. Saves you having to remember to do it when applying the patches to your tree.

https://mbobrowski.org/patches/kernel/0003-fanotify.patch

Signature · 1 line

Matthew Bobrowski <[email protected]>

Amir Goldstein

reviewer

2018-10-18 18:08 UTC #10 lore raw

In reply to Matthew Bobrowski

Quoted 7 lines · Matthew Bobrowski

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

Matthew,

Please note that this branch is slightly outdated.
When testing you should rebase your patches on top of
git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify

I don't think that you need to re-post unless Jan wants a re-post of patch #3 because there shouldn't be any rebase conflicts.

Thanks,
Amir.

Matthew Bobrowski

author

2018-10-18 20:13 UTC #11 lore raw

Quoted 7 lines · Amir Goldstein

Quoted 5 lines · Matthew Bobrowski
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:

Quoted 6 lines

Quoted 2 lines
https://github.com/amir73il/linux/tree/fanotify_api-v3
Please note that this branch is slightly outdated.
When testing you should rebase your patches on top of
git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify

Perfect, and will do. Thanks for sending that through.

Quoted 2 lines

I don't think that you need to re-post unless Jan wants a re-post of patch #3 because there shouldn't be any rebase conflicts.

Yeah, that's what I mentioned within my previous email. I mean, the ammendments to these files are minimal, so I don't really know whether I need to submit a completely new patch series through for each update after a review? Happy to re-post, if required.

Signature · 1 line

Matthew Bobrowski <[email protected]>