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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Yp7im6e4gugY2pSA@zeniv-ca.linux.org.uk>
Date:   Tue, 7 Jun 2022 05:31:07 +0000
From:   Al Viro <viro@...iv.linux.org.uk>
To:     Oliver Ford <ojford@...il.com>
Cc:     linux-fsdevel@...r.kernel.org, jack@...e.cz, amir73il@...il.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] fs: inotify: Add full paths option to inotify

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@...iv.linux.org.uk>