On Tue 19-10-21 08:34:41, Amir Goldstein wrote:

On Tue, Oct 19, 2021 at 3:01 AM Gabriel Krisman Bertazi
[off-list ref] wrote:

quoted

FAN_FS_ERROR allows events without inodes - i.e. for file system-wide
errors.  Even though fsnotify_handle_inode_event is not currently used
by fanotify, this patch protects this path to handle this new case.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Gabriel Krisman Bertazi <redacted>
---
 fs/notify/fsnotify.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index fde3a1115a17..47f931fb571c 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c

@@ -252,6 +252,9 @@ static int fsnotify_handle_inode_event(struct fsnotify_group *group,
        if (WARN_ON_ONCE(!ops->handle_inode_event))
                return 0;

+       if (!inode)
+               return 0;
+

Sigh.. the plot thickens.
There are three in-tree backends that implement the ->handle_inode_event()
interface.

inotify and dnotify can take NULL inode and the above will make the CREATE
events on kernfs vanish, so we cannot do that.
Sorry for not noticing this earlier when I asked for this change.

nfsd_file_fsnotify_handle_event() can most certainly not take NULL inode,
but nfsd does not watch for CREATE events.

And furthermore you cannot really export kernfs :)

I think what we need to do is (Jan please correct me if you think otherwise):
1. Document the handle_inode_event() interface that either inode or dir
    must be non-NULL
2. WARN_ON_ONCE(!inode && !dir) instead of just (!inode) above

Yeah, like:
	if (WARN_ON_ONCE(!inode && !dir))
		return 0;

3. Add WARN_ON_ONCE(!inode) before trace_nfsd_file_fsnotify_handle_event()
    in nfsd_file_fsnotify_handle_event()

And:
	if (WARN_ON_ONCE(!inode))
		return 0;

Sounds like a good plan to me.

								Honza
-- 
Jan Kara [off-list ref]
SUSE Labs, CR