On Mon 18-09-23 14:32:16, Max Kellermann wrote:
> This implements a missing piece in the inotify API: referring to a
> file by a directory file descriptor and a path name. This can be
> solved in userspace currently only by doing something similar to:
>
> int old = open(".");
> fchdir(dfd);
> inotify_add_watch(....);
> fchdir(old);
>
> Support for LOOKUP_EMPTY is still missing. We could add another IN_*
> flag for that (which would clutter the IN_* flags list further) or
> add a "flags" parameter to the new system call (which would however
> duplicate features already present via special IN_* flags).
>
> To: Jan Kara <[email protected]>
> Cc: Amir Goldstein <[email protected]>
> To: [email protected]
> To: [email protected]
> Signed-off-by: Max Kellermann <[email protected]>
Thanks for the patches! But generally we don't add new functionality to the
inotify API and rather steer users towards fanotify. In this particular
case fanotify_mark(2) already has the support for dirfd + name. Is there
any problem with using fanotify for you? Note that since kernel 5.13 you
don't need CAP_SYS_ADMIN capability for fanotify functionality that is
more-or-less equivalent to what inotify provides.
Honza
> ---
> fs/notify/inotify/inotify_user.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
> index b6e6f6ab21f8..8a9096c5ebb1 100644
> --- a/fs/notify/inotify/inotify_user.c
> +++ b/fs/notify/inotify/inotify_user.c
> @@ -797,6 +797,12 @@ SYSCALL_DEFINE3(inotify_add_watch, int, fd, const char __user *, pathname,
> return do_inotify_add_watch(fd, AT_FDCWD, pathname, mask);
> }
>
> +SYSCALL_DEFINE4(inotify_add_watch_at, int, fd, int, dfd, const char __user *, pathname,
> + u32, mask)
> +{
> + return do_inotify_add_watch(fd, dfd, pathname, mask);
> +}
> +
> SYSCALL_DEFINE2(inotify_rm_watch, int, fd, __s32, wd)
> {
> struct fsnotify_group *group;
> --
> 2.39.2
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Mon, Sep 18, 2023 at 2:40 PM Jan Kara <[email protected]> wrote:
> Note that since kernel 5.13 you
> don't need CAP_SYS_ADMIN capability for fanotify functionality that is
> more-or-less equivalent to what inotify provides.
Oh, I missed that change - I remember fanotify as being inaccessible
for unprivileged processes, and fanotify being designed for things
like virus scanners. Indeed I should migrate my code to fanotify.
If fanotify has now become the designated successor of inotify, that
should be hinted in the inotify manpage, and if inotify is effectively
feature-frozen, maybe that should be an extra status in the
MAINTAINERS file?
Max
On Mon, Sep 18, 2023 at 2:40 PM Jan Kara <[email protected]> wrote:
> Is there any problem with using fanotify for you?
Turns out fanotify is unusable for me, unfortunately.
I have been using inotify to get notifications of cgroup events, but
the cgroup filesystem appears to be unsupported by fanotify: all
attempts to use fanotify_mark() on cgroup event files fail with
ENODEV. I think that comes from fanotify_test_fsid(). Filesystems
without a fsid work just fine with inotify, but fail with fanotify.
Since fanotify lacks important features, is it really a good idea to
feature-freeze inotify?
(By the way, what was not documented is that fanotify_init() can only
be used by unprivileged processes if the FAN_REPORT_FID flag was
specified. I had to read the kernel sources to figure that out - I
have no idea why this limitation exists - the code comment in the
kernel source doesn't explain it.)
On Mon 18-09-23 15:57:43, Max Kellermann wrote:
> On Mon, Sep 18, 2023 at 2:40 PM Jan Kara <[email protected]> wrote:
> > Note that since kernel 5.13 you
> > don't need CAP_SYS_ADMIN capability for fanotify functionality that is
> > more-or-less equivalent to what inotify provides.
>
> Oh, I missed that change - I remember fanotify as being inaccessible
> for unprivileged processes, and fanotify being designed for things
> like virus scanners. Indeed I should migrate my code to fanotify.
>
> If fanotify has now become the designated successor of inotify, that
> should be hinted in the inotify manpage, and if inotify is effectively
> feature-frozen, maybe that should be an extra status in the
> MAINTAINERS file?
The manpage update is a good idea. I'm not sure about the MAINTAINERS
status - we do have 'Obsolete' but I'm reluctant to mark inotify as
obsolete as it's perfectly fine for existing users, we fully maintain it
and support it but we just don't want to extend the API anymore. Amir, what
are your thoughts on this?
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR