Re: Question: Why is there no notification when a file is opened using filp_open()?

[Amir Goldstein]

On Wed, Sep 9, 2020 at 10:00 AM Xiaoming Ni <nixiaoming@xxxxxxxxxx> wrote:
>
> On 2020/9/9 11:44, Amir Goldstein wrote:
> > On Tue, Sep 8, 2020 at 8:19 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> >>
> >> On Tue, Sep 08, 2020 at 04:18:29PM +0300, Amir Goldstein wrote:
> >>> On Tue, Sep 8, 2020 at 3:53 PM Xiaoming Ni <nixiaoming@xxxxxxxxxx> wrote:
> >>>> For example, in fs/coredump.c, do_coredump() calls filp_open() to
> >>>> generate core files.
> >>>> In this scenario, the fsnotify_open() notification is missing.
> >>>
> >>> I am not convinced that we should generate an event.
> >>> You will have to explain in what is the real world use case that requires this
> >>> event to be generated.
> >>
> >> Take the typical usage for fsnotify of a graphical file manager.
> >> It would be nice if the file manager showed a corefile as soon as it
> >> appeared in a directory rather than waiting until some other operation
> >> in that directory caused those directory contents to be refreshed.
> >
> > fsnotify_open() is not the correct notification for file managers IMO.
> > fsnotify_create() is and it will be called in this case.
> >
> > If the reason you are interested in open events is because you want
> > to monitor the entire filesystem then welcome to the future -
> > FAN_CREATE is supported since kernel v5.1.
> >
> > Is there another real life case you have in mind where you think users
> > should be able to get an open fd for a file that the kernel has opened?
> > Because that is what FAN_OPEN will do.
> >
>
> There are also cases where file is opened in read-only mode using
> filp_open().
>
> case1: nfsd4_init_recdir() call filp_open()
> filp_open()
> nfsd4_init_recdir() fs/nfsd/nfs4recover.c#L543
>
> L70: static char user_recovery_dirname[PATH_MAX] =
> "/var/lib/nfs/v4recovery";
> L543: nn->rec_file = filp_open(user_recovery_dirname, O_RDONLY |
> O_DIRECTORY, 0);
>
>
> case2: ima_read_policy()
> filp_open()
> kernel_read_file_from_path()  fs/exec.c#L1004
> ima_read_policy()  security/integrity/ima/ima_fs.c#L286
> ima_write_policy() security/integrity/ima/ima_fs.c#L335
> ima_measure_policy_ops   security/integrity/ima/ima_fs.c#L443
> sys_write()
>
> case3: use do_file_open_root() to open file
> do_file_open_root()
> file_open_root()   fs/open.c#L1159
> kernel_read_file_from_path_initns()  fs/exec.c#L1029
> fw_get_filesystem_firmware()  drivers/base/firmware_loader/main.c#L498
>
> Do we need to add fsnotify_open() in these scenarios?

We do not *need* to add fsnotify_open() if there is no concrete use case
from real life that needs it.

Matthew gave an example of a real life use case and I explained why IMO
we don't need to add fsnotify_open() for the use case that he described.

If you want to add fsnotify_open() to any call site, please come up with
a real life use case - not a made up one, one that really exists and where
the open event is really needed.

grepping the code for callers of filp_open() is not enough.

Thanks,
Amir.