diff mbox series

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

Message ID 20200629144145.GA3183@techsingularity.net
State New
Headers show
Series Revert "fs: Do not check if there is a fsnotify watcher on pseudo inodes" | expand

Commit Message

Mel Gorman June 29, 2020, 2:41 p.m. UTC
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@redhat.com>
Fixes: e9c15badbb7b ("fs: Do not check if there is a fsnotify watcher on pseudo inodes")
Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
 fs/file_table.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jan Kara June 29, 2020, 6:12 p.m. UTC | #1
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@redhat.com>
> Fixes: e9c15badbb7b ("fs: Do not check if there is a fsnotify watcher on pseudo inodes")
> Signed-off-by: Mel Gorman <mgorman@techsingularity.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);
diff mbox series

Patch

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);