[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20200629181258.GI26507@quack2.suse.cz>
Date: Mon, 29 Jun 2020 20:12:58 +0200
From: Jan Kara <jack@...e.cz>
To: Mel Gorman <mgorman@...hsingularity.net>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Jan Kara <jack@...e.cz>, Maxim Levitsky <mlevitsk@...hat.com>,
linux-kernel <linux-kernel@...r.kernel.org>,
Alexander Viro <viro@...iv.linux.org.uk>,
Amir Goldstein <amir73il@...il.com>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH] Revert "fs: Do not check if there is a fsnotify watcher
on pseudo inodes"
On Mon 29-06-20 15:41:45, Mel Gorman wrote:
> This reverts commit e9c15badbb7b ("fs: Do not check if there is a
> fsnotify watcher on pseudo inodes"). The commit intended to eliminate
> fsnotify-related overhead for pseudo inodes but it is broken in
> concept. inotify can receive events of pipe files under /proc/X/fd and
> chromium relies on close and open events for sandboxing. Maxim Levitsky
> reported the following
>
> Chromium starts as a white rectangle, shows few white rectangles that
> resemble its notifications and then crashes.
>
> The stdout output from chromium:
>
> [mlevitsk@...rship ~]$chromium-freeworld
> mesa: for the --simplifycfg-sink-common option: may only occur zero or one times!
> mesa: for the --global-isel-abort option: may only occur zero or one times!
> [3379:3379:0628/135151.440930:ERROR:browser_switcher_service.cc(238)] XXX Init()
> ../../sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc:**CRASHING**:seccomp-bpf failure in syscall 0072
> Received signal 11 SEGV_MAPERR 0000004a9048
>
> Crashes are not universal but even if chromium does not crash, it certainly
> does not work properly. While filtering just modify and access might be
> safe, the benefit is not worth the risk hence the revert.
>
> Reported-by: Maxim Levitsky <mlevitsk@...hat.com>
> Fixes: e9c15badbb7b ("fs: Do not check if there is a fsnotify watcher on pseudo inodes")
> Signed-off-by: Mel Gorman <mgorman@...hsingularity.net>
Thanks for the revert Mel. I can see Linus already picked it up so we are
done.
Honza
> ---
> fs/file_table.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/file_table.c b/fs/file_table.c
> index 65603502fed6..656647f9575a 100644
> --- a/fs/file_table.c
> +++ b/fs/file_table.c
> @@ -230,7 +230,7 @@ struct file *alloc_file_pseudo(struct inode *inode, struct vfsmount *mnt,
> d_set_d_op(path.dentry, &anon_ops);
> path.mnt = mntget(mnt);
> d_instantiate(path.dentry, inode);
> - file = alloc_file(&path, flags | FMODE_NONOTIFY, fops);
> + file = alloc_file(&path, flags, fops);
> if (IS_ERR(file)) {
> ihold(inode);
> path_put(&path);
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists