2018-09-17 10:04:16

by Xiaoming Ni

[permalink] [raw]
Subject: [PATCH] fanotify support save thread id

Added FAN_EVENT_INFO_TID to select the thread id of the event trigger

Signed-off-by: nixiaoming <[email protected]>
---
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(-)

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



2018-09-17 10:54:20

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH] fanotify support save thread id

On Mon, Sep 17, 2018 at 1:02 PM nixiaoming <[email protected]> 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 <[email protected]>
> ---
> --- 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.

2018-09-17 14:58:14

by Xiaoming Ni

[permalink] [raw]
Subject: RE: [PATCH] fanotify support save thread id


On Mon, Sep 17, 2018 6:54 PM Amir Goldstein <[email protected]> wrote:
>On Mon, Sep 17, 2018 at 1:02 PM nixiaoming <[email protected]> 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 <[email protected]>
>> ---
>> --- 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