Re: [PATCH 14/17] fsnotify: detach the connector before destroying its marks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] fsnotify: NULL pointer dereference in lockless fdinfo readers
--

commit e6a7d29740f3498c210e8d2cef8ba5c5a72ce862
Author: Christian Brauner <brauner@kernel.org>

fsnotify: detach the connector before destroying its marks

This commit moves the detachment of the fsnotify connector from the object
to the beginning of the mark destruction process. This prevents new marks
from being added to the connector during the destruction walk.

> diff --git a/fs/notify/mark.c b/fs/notify/mark.c
> index b2640d836a712..f6891d39e42fe 100644
> --- a/fs/notify/mark.c
> +++ b/fs/notify/mark.c
> @@ -1112,6 +1112,16 @@ void fsnotify_destroy_marks(fsnotify_connp_t *connp)
>  	conn = fsnotify_grab_connector(connp);
>  	if (!conn)
>  		return;
> +	/*
> +	 * Detach the connector from the object first. Once conn->lock is
> +	 * dropped a mark could be added in front of the one we're at and the
> +	 * walk would miss it. fsnotify_grab_connector() refuses a detached
> +	 * connector so any mark added from now on gets a connector of its
> +	 * own. This also stops pinning the inode until all mark references
> +	 * get dropped. It would lead to strange results such as delaying
> +	 * inode deletion or blocking unmount.
> +	 */
> +	objp = fsnotify_detach_connector_from_object(conn, &type);

[Severity: Critical]
Will this early detachment expose a NULL pointer to concurrent lockless
readers?

By moving fsnotify_detach_connector_from_object() to the beginning of
fsnotify_destroy_marks(), the connector's object pointer (conn->obj) is set
to NULL before the marks are removed from the group's list.

Concurrent lockless readers iterating the group list might observe the mark,
check the connector type, and attempt to read conn->obj. Because there is
no lock synchronization or memory barrier enforcing the visibility of the
detached state before conn->obj is cleared, a reader could see a valid
type but a NULL object.

This regression might be triggered if file unlinks race with reads of
/proc/<pid>/fdinfo/<inotify_fd>. For example, could this cause a NULL
dereference in inotify_fdinfo() via igrab()?

fs/notify/fdinfo.c:inotify_fdinfo() {
    ...
    inode = igrab(fsnotify_conn_inode(mark->connector));
    ...
}

Similar lockless access patterns appear to exist in fanotify_fdinfo() and
when fanotify_remove_mark() locklessly evaluates the connector mask. Can we
safely detach the connector this early without breaking these readers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-work-mount-fixes-3-v1-0-be34c83956ae@kernel.org?part=14