linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Is it possible to add pid and comm members to the event structure to increase the display of user and thread information?
@ 2018-09-11  6:51 Nixiaoming
  2018-09-11 15:11 ` Amir Goldstein
  0 siblings, 1 reply; 5+ messages in thread
From: Nixiaoming @ 2018-09-11  6:51 UTC (permalink / raw)
  To: Eric Paris, rlove, john, amir73il, jack, viro; +Cc: linux-kernel, linux-fsdevel

Inotify api cannot display information about users and processes.
That is, you can only know that the file event is generated, but you don't know who triggered the event, which is not conducive to fault location.
Is it possible to add pid and comm members to the event structure to increase the display of user and thread information?

Example:
diff --git a/fs/notify/inotify/inotify.h b/fs/notify/inotify/inotify.h
index 7e4578d..be91844 100644
--- a/fs/notify/inotify/inotify.h
+++ b/fs/notify/inotify/inotify.h
@@ -7,6 +7,8 @@ struct inotify_event_info {
        struct fsnotify_event fse;
        int wd;
        u32 sync_cookie;
+ int pid;
+ char comm[TASK_COMM_LEN];
        int name_len;
        char name[];
 };
diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index f4184b4..f7ad298 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -117,6 +117,8 @@ int inotify_handle_event(struct fsnotify_group *group,
        fsnotify_init_event(fsn_event, inode, mask);
        event->wd = i_mark->wd;
        event->sync_cookie = cookie;
+ event->pid = current->pid;
+ strncpy(event->comm, current->comm, TASK_COMM_LEN);
        event->name_len = len;
        if (len)
                strcpy(event->name, file_name);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Is it possible to add pid and comm members to the event structure to increase the display of user and thread information?
  2018-09-11  6:51 Is it possible to add pid and comm members to the event structure to increase the display of user and thread information? Nixiaoming
@ 2018-09-11 15:11 ` Amir Goldstein
  2018-09-13 11:25   ` Nixiaoming
  0 siblings, 1 reply; 5+ messages in thread
From: Amir Goldstein @ 2018-09-11 15:11 UTC (permalink / raw)
  To: nixiaoming
  Cc: Eric Paris, Robert Love, John McCutchan, Jan Kara, Al Viro,
	linux-kernel, linux-fsdevel

On Tue, Sep 11, 2018 at 9:51 AM Nixiaoming <nixiaoming@huawei.com> wrote:
>
> Inotify api cannot display information about users and processes.
> That is, you can only know that the file event is generated, but you don't know who triggered the event, which is not conducive to fault location.
> Is it possible to add pid and comm members to the event structure to increase the display of user and thread information?
>

"Is it possible?" is not the only relevant question.
I suppose your patch can sort of works, but it exposes information to
potentially unpriveleged
processes, even exposes pid values outside of the process pid namespace.

While those issues could be addressed, you can't change the format
struct inotify_event
without breaking existing applications.

I guess you are not using fanotify API, which already provides pid
information (albiet tgid),
because it lacks other functionality that you need? Which
functionality might that be?
Is it directory modification events?
If so than you might be interested in my effort to add support for
those events to fanotify:
https://github.com/amir73il/fsnotify-utils/wiki/Super-block-root-watch

Your support, should you choose to offer it, could be in the form of
testing patches
and/or just by putting forward your use case as an example for the
need of an extended
fanotify API.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: Is it possible to add pid and comm members to the event structure to increase the display of user and thread information?
  2018-09-11 15:11 ` Amir Goldstein
@ 2018-09-13 11:25   ` Nixiaoming
  2018-09-13 12:31     ` Amir Goldstein
  0 siblings, 1 reply; 5+ messages in thread
From: Nixiaoming @ 2018-09-13 11:25 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Eric Paris, Robert Love, John McCutchan, Jan Kara, Al Viro,
	linux-kernel, linux-fsdevel

On Tue, Sep 11, 2018 at 11:12 PM Amir Goldstein <amir73il@gmail.com> wrote:
>On Tue, Sep 11, 2018 at 9:51 AM Nixiaoming <nixiaoming@huawei.com> wrote:
>>
>> Inotify api cannot display information about users and processes.
>> That is, you can only know that the file event is generated, but you don't know who triggered the event, which is not conducive to fault location.
>> Is it possible to add pid and comm members to the event structure to increase the display of user and thread information?
>>
>
>"Is it possible?" is not the only relevant question.
>I suppose your patch can sort of works, but it exposes information to
>potentially unpriveleged
>processes, even exposes pid values outside of the process pid namespace.
>
>While those issues could be addressed, you can't change the format
>struct inotify_event
>without breaking existing applications.
>
In order to improve the fault location capability, can we make incompatible interface changes?

>I guess you are not using fanotify API, which already provides pid
>information (albiet tgid),
>because it lacks other functionality that you need? Which
>functionality might that be?
>Is it directory modification events?
>If so than you might be interested in my effort to add support for
>those events to fanotify:
>https://github.com/amir73il/fsnotify-utils/wiki/Super-block-root-watch
>
The fanotify API does not support monitoring file deletion events
The fanotify API supports tgid display,
but for multi-threaded programs,
it still cannot accurately identify which thread triggered the event.
Can I modify tgid to pid?
- event->tgid = get_pid(task_tgid(current));
+ event->tgid = get_pid(task_pid(current));

>Your support, should you choose to offer it, could be in the form of
>testing patches
>and/or just by putting forward your use case as an example for the
>need of an extended
>fanotify API.
>
>Thanks,
>Amir.
>

Thanks

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Is it possible to add pid and comm members to the event structure to increase the display of user and thread information?
  2018-09-13 11:25   ` Nixiaoming
@ 2018-09-13 12:31     ` Amir Goldstein
  2018-09-15 13:13       ` Nixiaoming
  0 siblings, 1 reply; 5+ messages in thread
From: Amir Goldstein @ 2018-09-13 12:31 UTC (permalink / raw)
  To: nixiaoming
  Cc: Eric Paris, Robert Love, John McCutchan, Jan Kara, Al Viro,
	linux-kernel, linux-fsdevel

On Thu, Sep 13, 2018 at 2:25 PM Nixiaoming <nixiaoming@huawei.com> wrote:
>
> On Tue, Sep 11, 2018 at 11:12 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >On Tue, Sep 11, 2018 at 9:51 AM Nixiaoming <nixiaoming@huawei.com> wrote:
> >>
> >> Inotify api cannot display information about users and processes.
> >> That is, you can only know that the file event is generated, but you don't know who triggered the event, which is not conducive to fault location.
> >> Is it possible to add pid and comm members to the event structure to increase the display of user and thread information?
> >>
> >
> >"Is it possible?" is not the only relevant question.
> >I suppose your patch can sort of works, but it exposes information to
> >potentially unpriveleged
> >processes, even exposes pid values outside of the process pid namespace.
> >
> >While those issues could be addressed, you can't change the format
> >struct inotify_event
> >without breaking existing applications.
> >
> In order to improve the fault location capability, can we make incompatible interface changes?

Not unless application/sysadmin/distro opts-in for the incompatible change.

>
> >I guess you are not using fanotify API, which already provides pid
> >information (albiet tgid),
> >because it lacks other functionality that you need? Which
> >functionality might that be?
> >Is it directory modification events?
> >If so than you might be interested in my effort to add support for
> >those events to fanotify:
> >https://github.com/amir73il/fsnotify-utils/wiki/Super-block-root-watch
> >
> The fanotify API does not support monitoring file deletion events

Yes, I am working toward that goal.

> The fanotify API supports tgid display,
> but for multi-threaded programs,
> it still cannot accurately identify which thread triggered the event.
> Can I modify tgid to pid?
> - event->tgid = get_pid(task_tgid(current));
> + event->tgid = get_pid(task_pid(current));
>

So if you would like to change that you need to add a new flag to
fanotify_init (e.g. FAN_EVENT_INFO_TID)
new applications that would opt-in for the flag will get task_pid()
while existing application will keep getting task_tgid()
new applications will get -EINVAL when passing FAN_EVENT_INFO_TID
to fanotify_init() on an old kernel and they could then fall back to getting
tgid in events and be aware of that fact.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: Is it possible to add pid and comm members to the event structure to increase the display of user and thread information?
  2018-09-13 12:31     ` Amir Goldstein
@ 2018-09-15 13:13       ` Nixiaoming
  0 siblings, 0 replies; 5+ messages in thread
From: Nixiaoming @ 2018-09-15 13:13 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Eric Paris, Robert Love, John McCutchan, Jan Kara, Al Viro,
	linux-kernel, linux-fsdevel

on Thu, Sep 13, 2018 at 8:32 PM Amir Goldstein <amir73il@gmail.com>wrote:
>On Thu, Sep 13, 2018 at 2:25 PM Nixiaoming <nixiaoming@huawei.com> wrote:
>>
>> On Tue, Sep 11, 2018 at 11:12 PM Amir Goldstein <amir73il@gmail.com> wrote:
>> >On Tue, Sep 11, 2018 at 9:51 AM Nixiaoming <nixiaoming@huawei.com> wrote:
>> >>
>> >> Inotify api cannot display information about users and processes.
>> >> That is, you can only know that the file event is generated, but you don't know who triggered the event, which is not conducive to fault location.
>> >> Is it possible to add pid and comm members to the event structure to increase the display of user and thread information?
>> >>
>> >
>> >"Is it possible?" is not the only relevant question.
>> >I suppose your patch can sort of works, but it exposes information to
>> >potentially unpriveleged
>> >processes, even exposes pid values outside of the process pid namespace.
>> >
>> >While those issues could be addressed, you can't change the format
>> >struct inotify_event
>> >without breaking existing applications.
>> >
>> In order to improve the fault location capability, can we make incompatible interface changes?
>
>Not unless application/sysadmin/distro opts-in for the incompatible change.
>
>>
>> >I guess you are not using fanotify API, which already provides pid
>> >information (albiet tgid),
>> >because it lacks other functionality that you need? Which
>> >functionality might that be?
>> >Is it directory modification events?
>> >If so than you might be interested in my effort to add support for
>> >those events to fanotify:
>> >https://github.com/amir73il/fsnotify-utils/wiki/Super-block-root-watch
>> >
>> The fanotify API does not support monitoring file deletion events
>
>Yes, I am working toward that goal.
>
>> The fanotify API supports tgid display,
>> but for multi-threaded programs,
>> it still cannot accurately identify which thread triggered the event.
>> Can I modify tgid to pid?
>> - event->tgid = get_pid(task_tgid(current));
>> + event->tgid = get_pid(task_pid(current));
>>
>
>So if you would like to change that you need to add a new flag to
>fanotify_init (e.g. FAN_EVENT_INFO_TID)
>new applications that would opt-in for the flag will get task_pid()
>while existing application will keep getting task_tgid()
>new applications will get -EINVAL when passing FAN_EVENT_INFO_TID
>to fanotify_init() on an old kernel and they could then fall back to getting
>tgid in events and be aware of that fact.
>
>Thanks,
>Amir.
>

Thank you for your guidance
I am modifying the code test locally,
The kernel mode selects whether to display the thread id information 
according to the FAN_EVENT_INFO_TID tag when fanotify_alloc_event is called 
the user mode test program adds FAN_EVENT_INFO_TID to the fanotify_markd parameter mask,  
now, it can accurately display which thread triggered the event.

I will send the patch later, please help me to review it

thanks

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2018-09-15 18:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-11  6:51 Is it possible to add pid and comm members to the event structure to increase the display of user and thread information? Nixiaoming
2018-09-11 15:11 ` Amir Goldstein
2018-09-13 11:25   ` Nixiaoming
2018-09-13 12:31     ` Amir Goldstein
2018-09-15 13:13       ` Nixiaoming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).