On Wed, Aug 25, 2021 at 9:40 PM Gabriel Krisman Bertazi
[off-list ref] wrote:

Amir Goldstein [off-list ref] writes:

quoted

On Fri, Aug 13, 2021 at 12:41 AM Gabriel Krisman Bertazi
[off-list ref] wrote:

quoted

Some file system events (i.e. FS_ERROR) might not be associated with an
inode.  For these, it makes sense to associate them directly with the
super block of the file system they apply to.  This patch allows the
event to be reported with a NULL inode, by recovering the superblock
directly from the data field, if needed.

Signed-off-by: Gabriel Krisman Bertazi <redacted>

--
Changes since v5:
  - add fsnotify_data_sb handle to retrieve sb from the data field. (jan)
---
 fs/notify/fsnotify.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 30d422b8c0fc..536db02cb26e 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c

@@ -98,6 +98,14 @@ void fsnotify_sb_delete(struct super_block *sb)
        fsnotify_clear_marks_by_sb(sb);
 }

+static struct super_block *fsnotify_data_sb(const void *data, int data_type)
+{
+       struct inode *inode = fsnotify_data_inode(data, data_type);
+       struct super_block *sb = inode ? inode->i_sb : NULL;
+
+       return sb;
+}
+
 /*
  * Given an inode, first check if we care what happens to our children.  Inotify
  * and dnotify both tell their parents about events.  If we care about any event

@@ -455,8 +463,10 @@ static void fsnotify_iter_next(struct fsnotify_iter_info *iter_info)
  *             @file_name is relative to
  * @file_name: optional file name associated with event
  * @inode:     optional inode associated with event -
- *             either @dir or @inode must be non-NULL.
- *             if both are non-NULL event may be reported to both.
+ *             If @dir and @inode are NULL, @data must have a type that
+ *             allows retrieving the file system associated with this

Irrelevant comment. sb must always be available from @data.

quoted

+ *             event.  if both are non-NULL event may be reported to
+ *             both.
  * @cookie:    inotify rename cookie
  */
 int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,

@@ -483,7 +493,7 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
                 */
                parent = dir;
        }
-       sb = inode->i_sb;
+       sb = inode ? inode->i_sb : fsnotify_data_sb(data, data_type);

        const struct path *path = fsnotify_data_path(data, data_type);
+       const struct super_block *sb = fsnotify_data_sb(data, data_type);

All the games with @data @inode and @dir args are irrelevant to this.
sb should always be available from @data and it does not matter
if fsnotify_data_inode() is the same as @inode, @dir or neither.
All those inodes are anyway on the same sb.

Hi Amir,

I think this is actually necessary.  I could identify at least one event
(FS_CREATE | FS_ISDIR) where fsnotify is invoked with a NULL data field.
In that case, fsnotify_dirent is called with a negative dentry from
vfs_mkdir().  I'm not sure why exactly the dentry is negative after the

That doesn't sound right at all.
Are you sure about this?
Which filesystem was this mkdir called on?

mkdir, but it would be possible we are racing with the file removal, I

No. Both vfs_mkdir() and vfs_rmdir() hold the dir inode lock (on parent).

guess?  It might be a bug in fsnotify that this case even happen, but
I'm not sure yet.

fsnotify_data_inode() should not be NULL.
fsnotify_handle_inode_event() passes fsnotify_data_inode() to
callbacks like audit_watch_handle_event() that checks
WARN_ON_ONCE(!inode)

The easiest way around it is what I proposed: just use inode->i_sb if
data is NULL.  Since, as you said, data, inode and dir are all on the
same superblock, it should work, I think.

It would be papering over another issue.
I don't mind if we use inode->i_sb as long as we understand the reason
for what you are reporting.

Please provide more information.

Thanks,
Amir.