Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Tue, 7 Jun 2022 05:31:07 +0000
From:   Al Viro <viro@zeniv.linux.org.uk>
To:     Oliver Ford <ojford@gmail.com>
Cc:     linux-fsdevel@vger.kernel.org, jack@suse.cz, amir73il@gmail.com,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/1] fs: inotify: Add full paths option to inotify
Message-ID: <Yp7im6e4gugY2pSA@zeniv-ca.linux.org.uk>
References: <20220606224241.25254-1-ojford@gmail.com>
 <20220606224241.25254-2-ojford@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220606224241.25254-2-ojford@gmail.com>
Sender: Al Viro <viro@ftp.linux.org.uk>
Precedence: bulk

On Mon, Jun 06, 2022 at 11:42:41PM +0100, Oliver Ford wrote:

> @@ -203,6 +204,8 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>  {
>  	struct inotify_event inotify_event;
>  	struct inotify_event_info *event;
> +	struct path event_path;
> +	struct inotify_inode_mark *i_mark;
>  	size_t event_size = sizeof(struct inotify_event);
>  	size_t name_len;
>  	size_t pad_name_len;
> @@ -210,6 +213,18 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
>  	pr_debug("%s: group=%p event=%p\n", __func__, group, fsn_event);
>  
>  	event = INOTIFY_E(fsn_event);
> +	/* ensure caller has access to view the full path */
> +	if (event->mask & IN_FULL_PATHS && event->mask & IN_MOVE_SELF &&
> +	    kern_path(event->name, 0, &event_path)) {
> +		i_mark = inotify_idr_find(group, event->wd);
> +		if (likely(i_mark)) {
> +			fsnotify_destroy_mark(&i_mark->fsn_mark, group);
> +			/* match ref taken by inotify_idr_find */
> +			fsnotify_put_mark(&i_mark->fsn_mark);
> +		}
> +		return -EACCES;
> +	}
> +

What.  The.  Hell?

Could you please explain how is it not a massive dentry and mount leak and
just what is being attempted here, anyway?

Incidentally, who said that pathname will be still resolving to whatever
it used to resolve to back when the operation had happened?  Or that
it would make any sense for the read(2) caller, while we are at it...

NAKed-by: Al Viro <viro@zeniv.linux.org.uk>