Re: [PATCH] Revert "fs: Do not check if there is a fsnotify watcher on pseudo inodes"

From: Jan Kara
Date: Mon Jun 29 2020 - 17:23:12 EST


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@starship ~]$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@xxxxxxxxxx>
> Fixes: e9c15badbb7b ("fs: Do not check if there is a fsnotify watcher on pseudo inodes")
> Signed-off-by: Mel Gorman <mgorman@xxxxxxxxxxxxxxxxxxx>

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@xxxxxxxx>
SUSE Labs, CR