Re: [PATCH 1/1] fs: inotify: Add full paths option to inotify

[Al Viro <viro@zeniv.linux.org.uk>]

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>