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

[Jan Kara]

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