[GIT PULL] fsnotify changes for 6.10-rc1

[GIT PULL] fsnotify changes for 6.10-rc1

[Jan Kara]

  Hello Linus,

  could you please pull from

git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify_for_v6.10-rc1

The pull contains:
  * Changes to reduce overhead of fsnotify infrastructure when no
    permission events are in use
  * A few small cleanups

Top of the tree is 795bb82d12a1. The full shortlog is:

Amir Goldstein (11):
      fsnotify: rename fsnotify_{get,put}_sb_connectors()
      fsnotify: create helpers to get sb and connp from object
      fsnotify: create a wrapper fsnotify_find_inode_mark()
      fanotify: merge two checks regarding add of ignore mark
      fsnotify: pass object pointer and type to fsnotify mark helpers
      fsnotify: create helper fsnotify_update_sb_watchers()
      fsnotify: lazy attach fsnotify_sb_info state to sb
      fsnotify: move s_fsnotify_connectors into fsnotify_sb_info
      fsnotify: use an enum for group priority constants
      fsnotify: optimize the case of no permission event watchers
      fsnotify: fix UAF from FS_ERROR event on a shutting down filesystem

Gustavo A. R. Silva (1):
      fsnotify: Avoid -Wflex-array-member-not-at-end warning

Nikita Kiryushin (1):
      fanotify: remove unneeded sub-zero check for unsigned value

The diffstat is

 fs/nfsd/filecache.c                |   4 +-
 fs/notify/dnotify/dnotify.c        |   4 +-
 fs/notify/fanotify/fanotify_user.c | 143 +++++++++---------------------
 fs/notify/fdinfo.c                 |  20 ++---
 fs/notify/fsnotify.c               |  27 ++++--
 fs/notify/fsnotify.h               |  39 ++++++---
 fs/notify/inotify/inotify_user.c   |   2 +-
 fs/notify/mark.c                   | 174 ++++++++++++++++++++++++++++---------
 fs/super.c                         |   1 +
 include/linux/fs.h                 |  14 +--
 include/linux/fsnotify.h           |  21 ++++-
 include/linux/fsnotify_backend.h   |  97 ++++++++++++++-------
 kernel/audit_tree.c                |   2 +-
 kernel/audit_watch.c               |   2 +-
 14 files changed, 334 insertions(+), 216 deletions(-)

							Thanks
								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [GIT PULL] fsnotify changes for 6.10-rc1

[Linus Torvalds]

On Mon, 20 May 2024 at 04:22, Jan Kara <jack@suse.cz> wrote:
>
>   * A few small cleanups

This IS NOT A CLEANUP! It's a huge mistake:

> Nikita Kiryushin (1):
>       fanotify: remove unneeded sub-zero check for unsigned value

The old code did

    WARN_ON_ONCE(len < 0 || len >= FANOTIFY_EVENT_ALIGN);

and that is very legible and very understandable to humans.

The new code is

    WARN_ON_ONCE(len >= FANOTIFY_EVENT_ALIGN);

and now a human that reads that line needs to go back and check what
the type of 'len' is to notice that it's unsigned. It is not at all
clear from the context, and the declaration of 'len' is literally 80
lines up. Not very close at all, in other words.

And a compiler doesn't care. A compiler will know the type, and not
generate pointless code.

So this kind of change SHOULD NOT BE DONE. This was making the code
less legible for no good reason.

The reason for this change is quoted as

    Found by Linux Verification Center (linuxtesting.org) with SVACE.

and honestly, that only means that linuxtesting.org is actively
detrimental, and whatever "SVACE" is is just incompetent garbage.

Please stop accepting these kinds of patches.

I have done this pull, but I reverted that damage.

                                   Linus

Re: [GIT PULL] fsnotify changes for 6.10-rc1

[Jan Kara]

On Mon 20-05-24 12:46:44, Linus Torvalds wrote:
> On Mon, 20 May 2024 at 04:22, Jan Kara <jack@suse.cz> wrote:
> >
> >   * A few small cleanups
> 
> This IS NOT A CLEANUP! It's a huge mistake:
> 
> > Nikita Kiryushin (1):
> >       fanotify: remove unneeded sub-zero check for unsigned value
> 
> The old code did
> 
>     WARN_ON_ONCE(len < 0 || len >= FANOTIFY_EVENT_ALIGN);
> 
> and that is very legible and very understandable to humans.
> 
> The new code is
> 
>     WARN_ON_ONCE(len >= FANOTIFY_EVENT_ALIGN);
> 
> and now a human that reads that line needs to go back and check what
> the type of 'len' is to notice that it's unsigned. It is not at all
> clear from the context, and the declaration of 'len' is literally 80
> lines up. Not very close at all, in other words.

So I was hesitating whether to take this or not because I liked the len < 0
check as well. Then I've convinced myself that the impression this check
gives that "if we miscomputed and used more than we should, the
WARN_ON_ONCE() would trigger" is incorrect because of underflow so better
delete it. But now that I've written it down and looked again that
impression is actually correct because the len >= FANOTIFY_EVENT_ALIGN
check will catch that situation instead. Long story short, I agree with
your revert.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [GIT PULL] fsnotify changes for 6.10-rc1

[pr-tracker-bot]

The pull request you sent on Mon, 20 May 2024 13:22:39 +0200:

> git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify_for_v6.10-rc1

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/5af9d1cf3906171de28f1c395264f29088bdd267

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html