[PATCH] inotify: Avoid reporting event with invalid wd

[PATCH] inotify: Avoid reporting event with invalid wd

[Jan Kara]

When inotify_freeing_mark() races with inotify_handle_inode_event() it
can happen that inotify_handle_inode_event() sees that i_mark->wd got
already reset to -1 and reports this value to userspace which can
confuse the inotify listener. Avoid the problem by validating that wd is
sensible (and pretend the mark got removed before the event got
generated otherwise).

CC: stable@vger.kernel.org
Fixes: 7e790dd5fc93 ("inotify: fix error paths in inotify_update_watch")
Reported-by: syzbot+4a06d4373fd52f0b2f9c@syzkaller.appspotmail.com
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/notify/inotify/inotify_fsnotify.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

I plan to merge this fix through my tree.

diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index 49cfe2ae6d23..f86d12790cb1 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -65,7 +65,7 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
 	struct fsnotify_event *fsn_event;
 	struct fsnotify_group *group = inode_mark->group;
 	int ret;
-	int len = 0;
+	int len = 0, wd;
 	int alloc_len = sizeof(struct inotify_event_info);
 	struct mem_cgroup *old_memcg;
 
@@ -80,6 +80,13 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
 	i_mark = container_of(inode_mark, struct inotify_inode_mark,
 			      fsn_mark);
 
+	/*
+	 * We can be racing with mark being detached. Don't report event with
+ 	 * invalid wd.
+	 */
+	wd = READ_ONCE(i_mark->wd);
+	if (wd == -1)
+		return 0;
 	/*
 	 * Whoever is interested in the event, pays for the allocation. Do not
 	 * trigger OOM killer in the target monitoring memcg as it may have
@@ -110,7 +117,7 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
 	fsn_event = &event->fse;
 	fsnotify_init_event(fsn_event);
 	event->mask = mask;
-	event->wd = i_mark->wd;
+	event->wd = wd;
 	event->sync_cookie = cookie;
 	event->name_len = len;
 	if (len)
-- 
2.35.3

Re: [PATCH] inotify: Avoid reporting event with invalid wd

[Amir Goldstein]

On Mon, Apr 24, 2023 at 7:32 PM Jan Kara <jack@suse.cz> wrote:
>
> When inotify_freeing_mark() races with inotify_handle_inode_event() it
> can happen that inotify_handle_inode_event() sees that i_mark->wd got
> already reset to -1 and reports this value to userspace which can
> confuse the inotify listener. Avoid the problem by validating that wd is
> sensible (and pretend the mark got removed before the event got
> generated otherwise).
>
> CC: stable@vger.kernel.org
> Fixes: 7e790dd5fc93 ("inotify: fix error paths in inotify_update_watch")
> Reported-by: syzbot+4a06d4373fd52f0b2f9c@syzkaller.appspotmail.com
> Signed-off-by: Jan Kara <jack@suse.cz>

Makes sense.

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  fs/notify/inotify/inotify_fsnotify.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> I plan to merge this fix through my tree.
>
> diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
> index 49cfe2ae6d23..f86d12790cb1 100644
> --- a/fs/notify/inotify/inotify_fsnotify.c
> +++ b/fs/notify/inotify/inotify_fsnotify.c
> @@ -65,7 +65,7 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
>         struct fsnotify_event *fsn_event;
>         struct fsnotify_group *group = inode_mark->group;
>         int ret;
> -       int len = 0;
> +       int len = 0, wd;
>         int alloc_len = sizeof(struct inotify_event_info);
>         struct mem_cgroup *old_memcg;
>
> @@ -80,6 +80,13 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
>         i_mark = container_of(inode_mark, struct inotify_inode_mark,
>                               fsn_mark);
>
> +       /*
> +        * We can be racing with mark being detached. Don't report event with
> +        * invalid wd.
> +        */
> +       wd = READ_ONCE(i_mark->wd);
> +       if (wd == -1)
> +               return 0;
>         /*
>          * Whoever is interested in the event, pays for the allocation. Do not
>          * trigger OOM killer in the target monitoring memcg as it may have
> @@ -110,7 +117,7 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
>         fsn_event = &event->fse;
>         fsnotify_init_event(fsn_event);
>         event->mask = mask;
> -       event->wd = i_mark->wd;
> +       event->wd = wd;
>         event->sync_cookie = cookie;
>         event->name_len = len;
>         if (len)
> --
> 2.35.3
>

Re: [PATCH] inotify: Avoid reporting event with invalid wd

[Jan Kara]

On Tue 25-04-23 09:01:20, Amir Goldstein wrote:
> On Mon, Apr 24, 2023 at 7:32 PM Jan Kara <jack@suse.cz> wrote:
> >
> > When inotify_freeing_mark() races with inotify_handle_inode_event() it
> > can happen that inotify_handle_inode_event() sees that i_mark->wd got
> > already reset to -1 and reports this value to userspace which can
> > confuse the inotify listener. Avoid the problem by validating that wd is
> > sensible (and pretend the mark got removed before the event got
> > generated otherwise).
> >
> > CC: stable@vger.kernel.org
> > Fixes: 7e790dd5fc93 ("inotify: fix error paths in inotify_update_watch")
> > Reported-by: syzbot+4a06d4373fd52f0b2f9c@syzkaller.appspotmail.com
> > Signed-off-by: Jan Kara <jack@suse.cz>
> 
> Makes sense.
> 
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks. I've pulled the patch into my tree.

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