diff mbox series

fanotify support save thread id

Message ID 20180917091602.21531-1-nixiaoming@huawei.com (mailing list archive)
State New, archived
Headers show
Series fanotify support save thread id | expand

Commit Message

Xiaoming Ni Sept. 17, 2018, 9:16 a.m. UTC
Added FAN_EVENT_INFO_TID to select the thread id of the event trigger

Signed-off-by: nixiaoming <nixiaoming@huawei.com>
---
 fs/notify/fanotify/fanotify.c      | 66 +++++++++++++++++++++++++-------------
 fs/notify/fanotify/fanotify.h      |  2 +-
 fs/notify/fanotify/fanotify_user.c |  4 +--
 include/uapi/linux/fanotify.h      |  1 +
 4 files changed, 48 insertions(+), 25 deletions(-)

Comments

Amir Goldstein Sept. 17, 2018, 10:53 a.m. UTC | #1
On Mon, Sep 17, 2018 at 1:02 PM nixiaoming <nixiaoming@huawei.com> wrote:
>
> Added FAN_EVENT_INFO_TID to select the thread id of the event trigger
>

Maybe "to report the thread id of the task that caused the event".
Also in commit title, I think "reporting thread id" is more to the point
than "save thread id".


> Signed-off-by: nixiaoming <nixiaoming@huawei.com>
> ---
> --- a/include/uapi/linux/fanotify.h
> +++ b/include/uapi/linux/fanotify.h
> @@ -18,6 +18,7 @@
>
>  #define FAN_ONDIR              0x40000000      /* event occurred against dir */
>
> +#define FAN_EVENT_INFO_TID     0x02000000      /* event save thread id replace tgid */
>  #define FAN_EVENT_ON_CHILD     0x08000000      /* interested in child events */
>

nixiaoming,

You misunderstood me.

I meant that you should add support for flag FAN_EVENT_INFO_TID
for the fanotify_init() syscall, not the fanotify_mark() syscall.

See this commit from Steve Grubb for example of adding new opt-in
behavior to fanotify:
de8cd83e91bc audit: Record fanotify access control decisions

BTW, Steve, did you ever follow up with a man-pages patch?

Thanks,
Amir.
Xiaoming Ni Sept. 17, 2018, 2:57 p.m. UTC | #2
On Mon, Sep 17, 2018 6:54 PM Amir Goldstein <amir73il@gmail.com> wrote:
>On Mon, Sep 17, 2018 at 1:02 PM nixiaoming <nixiaoming@huawei.com> wrote:
>>
>> Added FAN_EVENT_INFO_TID to select the thread id of the event trigger
>>
>
>Maybe "to report the thread id of the task that caused the event".
>Also in commit title, I think "reporting thread id" is more to the point
>than "save thread id".
>
Very good advice, I will update the patch description based on your suggestions.

>
>> Signed-off-by: nixiaoming <nixiaoming@huawei.com>
>> ---
>> --- a/include/uapi/linux/fanotify.h
>> +++ b/include/uapi/linux/fanotify.h
>> @@ -18,6 +18,7 @@
>>
>>  #define FAN_ONDIR              0x40000000      /* event occurred against dir */
>>
>> +#define FAN_EVENT_INFO_TID     0x02000000      /* event save thread id replace tgid */
>>  #define FAN_EVENT_ON_CHILD     0x08000000      /* interested in child events */
>>
>
>nixiaoming,
>
>You misunderstood me.
>
>I meant that you should add support for flag FAN_EVENT_INFO_TID
>for the fanotify_init() syscall, not the fanotify_mark() syscall.
>
I mistakenly thought that adding a tag to group->fanotify_data in the fanotify_init system call would affect the system ABI. 
local tests found that Module.symvers did not change before and after the modification.

According to your opinion, I have re-modified a version of fanotify_init
I will send the patch later.

>See this commit from Steve Grubb for example of adding new opt-in
>behavior to fanotify:
>de8cd83e91bc audit: Record fanotify access control decisions
>
>BTW, Steve, did you ever follow up with a man-pages patch?
>
>Thanks,
>Amir.
>

thanks
diff mbox series

Patch

diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index 94b5215..cf6ff4e 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -89,14 +89,36 @@  static int fanotify_get_response(struct fsnotify_group *group,
 	return ret;
 }
 
+static void fanotify_iter_mask(struct fsnotify_iter_info *iter_info,
+		  u32 event_mask, __u32 *marks_mask, __u32 *marks_ignored_mask)
+{
+	int type;
+	struct fsnotify_mark *mark;
+
+	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
+		 * events on the child, don't send it!
+		 */
+		if (type == FSNOTIFY_OBJ_TYPE_INODE &&
+		    (event_mask & FS_EVENT_ON_CHILD) &&
+		    !(mark->mask & FS_EVENT_ON_CHILD))
+			continue;
+
+		*marks_mask |= mark->mask;
+		*marks_ignored_mask |= mark->ignored_mask;
+	}
+}
+
 static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
 				       u32 event_mask, const void *data,
 				       int data_type)
 {
 	__u32 marks_mask = 0, marks_ignored_mask = 0;
 	const struct path *path = data;
-	struct fsnotify_mark *mark;
-	int type;
 
 	pr_debug("%s: report_mask=%x mask=%x data=%p data_type=%d\n",
 		 __func__, iter_info->report_mask, event_mask, data, data_type);
@@ -110,23 +132,7 @@  static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
 	    !d_can_lookup(path->dentry))
 		return false;
 
-	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
-		 * events on the child, don't send it!
-		 */
-		if (type == FSNOTIFY_OBJ_TYPE_INODE &&
-		    (event_mask & FS_EVENT_ON_CHILD) &&
-		    !(mark->mask & FS_EVENT_ON_CHILD))
-			continue;
-
-		marks_mask |= mark->mask;
-		marks_ignored_mask |= mark->ignored_mask;
-	}
-
+	fanotify_iter_mask(iter_info, event_mask, &marks_mask, &marks_ignored_mask);
 	if (d_is_dir(path->dentry) &&
 	    !(marks_mask & FS_ISDIR & ~marks_ignored_mask))
 		return false;
@@ -138,9 +144,20 @@  static bool fanotify_should_send_event(struct fsnotify_iter_info *iter_info,
 	return false;
 }
 
+static bool fanotify_should_save_tid(struct fsnotify_iter_info *iter_info,
+		 u32 event_mask)
+{
+	__u32 marks_mask = 0, marks_ignored_mask = 0;
+
+	fanotify_iter_mask(iter_info, event_mask, &marks_mask, &marks_ignored_mask);
+	if (marks_mask & FAN_EVENT_INFO_TID)
+		return true;
+	return false;
+}
+
 struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
 						 struct inode *inode, u32 mask,
-						 const struct path *path)
+						 const struct path *path, bool should_save_tid)
 {
 	struct fanotify_event_info *event = NULL;
 	gfp_t gfp = GFP_KERNEL_ACCOUNT;
@@ -171,7 +188,10 @@  struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
 		goto out;
 init: __maybe_unused
 	fsnotify_init_event(&event->fse, inode, mask);
-	event->tgid = get_pid(task_tgid(current));
+	if (should_save_tid)
+		event->tgid = get_pid(task_pid(current));
+	else
+		event->tgid = get_pid(task_tgid(current));
 	if (path) {
 		event->path = *path;
 		path_get(&event->path);
@@ -193,6 +213,7 @@  static int fanotify_handle_event(struct fsnotify_group *group,
 	int ret = 0;
 	struct fanotify_event_info *event;
 	struct fsnotify_event *fsn_event;
+	bool should_save_tid;
 
 	BUILD_BUG_ON(FAN_ACCESS != FS_ACCESS);
 	BUILD_BUG_ON(FAN_MODIFY != FS_MODIFY);
@@ -219,8 +240,9 @@  static int fanotify_handle_event(struct fsnotify_group *group,
 		if (!fsnotify_prepare_user_wait(iter_info))
 			return 0;
 	}
+	should_save_tid = fanotify_should_save_tid(iter_info, mask);
 
-	event = fanotify_alloc_event(group, inode, mask, data);
+	event = fanotify_alloc_event(group, inode, mask, data, should_save_tid);
 	ret = -ENOMEM;
 	if (unlikely(!event)) {
 		/*
diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h
index 8609ba0..c303c574 100644
--- a/fs/notify/fanotify/fanotify.h
+++ b/fs/notify/fanotify/fanotify.h
@@ -54,4 +54,4 @@  static inline struct fanotify_event_info *FANOTIFY_E(struct fsnotify_event *fse)
 
 struct fanotify_event_info *fanotify_alloc_event(struct fsnotify_group *group,
 						 struct inode *inode, u32 mask,
-						 const struct path *path);
+						 const struct path *path, bool should_save_tid);
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 6905488..99d431b 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -734,7 +734,7 @@  SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	atomic_inc(&user->fanotify_listeners);
 	group->memcg = get_mem_cgroup_from_mm(current->mm);
 
-	oevent = fanotify_alloc_event(group, NULL, FS_Q_OVERFLOW, NULL);
+	oevent = fanotify_alloc_event(group, NULL, FS_Q_OVERFLOW, NULL, false);
 	if (unlikely(!oevent)) {
 		fd = -ENOMEM;
 		goto out_destroy_group;
@@ -805,7 +805,7 @@  static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 	struct fsnotify_group *group;
 	struct fd f;
 	struct path path;
-	u32 valid_mask = FAN_ALL_EVENTS | FAN_EVENT_ON_CHILD;
+	u32 valid_mask = FAN_ALL_EVENTS | FAN_EVENT_ON_CHILD | FAN_EVENT_INFO_TID;
 	int ret;
 
 	pr_debug("%s: fanotify_fd=%d flags=%x dfd=%d pathname=%p mask=%llx\n",
diff --git a/include/uapi/linux/fanotify.h b/include/uapi/linux/fanotify.h
index 7424791..d4fa336 100644
--- a/include/uapi/linux/fanotify.h
+++ b/include/uapi/linux/fanotify.h
@@ -18,6 +18,7 @@ 
 
 #define FAN_ONDIR		0x40000000	/* event occurred against dir */
 
+#define FAN_EVENT_INFO_TID	0x02000000	/* event save thread id replace tgid */
 #define FAN_EVENT_ON_CHILD	0x08000000	/* interested in child events */
 
 /* helper events */