All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] fanotify: Disallow permission events for proc filesystem
@ 2019-05-16 11:56 Jan Kara
  2019-05-28 15:54 ` Jan Kara
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Kara @ 2019-05-16 11:56 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Amir Goldstein, Jan Kara

Proc filesystem has special locking rules for various files. Thus
fanotify which opens files on event delivery can easily deadlock
against another process that waits for fanotify permission event to be
handled. Since permission events on /proc have doubtful value anyway,
just disallow them.

Link: https://lore.kernel.org/linux-fsdevel/20190320131642.GE9485@quack2.suse.cz/
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/notify/fanotify/fanotify_user.c | 22 ++++++++++++++++++++++
 fs/proc/root.c                     |  2 +-
 include/linux/fs.h                 |  1 +
 3 files changed, 24 insertions(+), 1 deletion(-)

Changes since v1:
* use type flag to detect filesystems not supporting permission events
* return -EINVAL instead of -EOPNOTSUPP for such filesystems

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index a90bb19dcfa2..91006f47e420 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -920,6 +920,22 @@ static int fanotify_test_fid(struct path *path, __kernel_fsid_t *fsid)
 	return 0;
 }
 
+static int fanotify_events_supported(struct path *path, __u64 mask)
+{
+	/*
+	 * Some filesystems such as 'proc' acquire unusual locks when opening
+	 * files. For them fanotify permission events have high chances of
+	 * deadlocking the system - open done when reporting fanotify event
+	 * blocks on this "unusual" lock while another process holding the lock
+	 * waits for fanotify permission event to be answered. Just disallow
+	 * permission events for such filesystems.
+	 */
+	if (mask & FANOTIFY_PERM_EVENTS &&
+	    path->mnt->mnt_sb->s_type->fs_flags & FS_DISALLOW_NOTIFY_PERM)
+		return -EINVAL;
+	return 0;
+}
+
 static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 			    int dfd, const char  __user *pathname)
 {
@@ -1018,6 +1034,12 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 	if (ret)
 		goto fput_and_out;
 
+	if (flags & FAN_MARK_ADD) {
+		ret = fanotify_events_supported(&path, mask);
+		if (ret)
+			goto path_put_and_out;
+	}
+
 	if (FAN_GROUP_FLAG(group, FAN_REPORT_FID)) {
 		ret = fanotify_test_fid(&path, &__fsid);
 		if (ret)
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 8b145e7b9661..522199e9525e 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -211,7 +211,7 @@ static struct file_system_type proc_fs_type = {
 	.init_fs_context	= proc_init_fs_context,
 	.parameters		= &proc_fs_parameters,
 	.kill_sb		= proc_kill_sb,
-	.fs_flags		= FS_USERNS_MOUNT,
+	.fs_flags		= FS_USERNS_MOUNT | FS_DISALLOW_NOTIFY_PERM,
 };
 
 void __init proc_root_init(void)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f7fdfe93e25d..c7136c98b5ba 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2184,6 +2184,7 @@ struct file_system_type {
 #define FS_BINARY_MOUNTDATA	2
 #define FS_HAS_SUBTYPE		4
 #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
+#define FS_DISALLOW_NOTIFY_PERM	16	/* Disable fanotify permission events */
 #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
 	int (*init_fs_context)(struct fs_context *);
 	const struct fs_parameter_description *parameters;
-- 
2.16.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] fanotify: Disallow permission events for proc filesystem
  2019-05-16 11:56 [PATCH v2] fanotify: Disallow permission events for proc filesystem Jan Kara
@ 2019-05-28 15:54 ` Jan Kara
  2019-05-28 16:02   ` Amir Goldstein
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Kara @ 2019-05-28 15:54 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Amir Goldstein, Jan Kara

On Thu 16-05-19 13:56:19, Jan Kara wrote:
> Proc filesystem has special locking rules for various files. Thus
> fanotify which opens files on event delivery can easily deadlock
> against another process that waits for fanotify permission event to be
> handled. Since permission events on /proc have doubtful value anyway,
> just disallow them.
> 
> Link: https://lore.kernel.org/linux-fsdevel/20190320131642.GE9485@quack2.suse.cz/
> Signed-off-by: Jan Kara <jack@suse.cz>

Amir, ping? What do you think about this version of the patch?

								Honza

> ---
>  fs/notify/fanotify/fanotify_user.c | 22 ++++++++++++++++++++++
>  fs/proc/root.c                     |  2 +-
>  include/linux/fs.h                 |  1 +
>  3 files changed, 24 insertions(+), 1 deletion(-)
> 
> Changes since v1:
> * use type flag to detect filesystems not supporting permission events
> * return -EINVAL instead of -EOPNOTSUPP for such filesystems
> 
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index a90bb19dcfa2..91006f47e420 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -920,6 +920,22 @@ static int fanotify_test_fid(struct path *path, __kernel_fsid_t *fsid)
>  	return 0;
>  }
>  
> +static int fanotify_events_supported(struct path *path, __u64 mask)
> +{
> +	/*
> +	 * Some filesystems such as 'proc' acquire unusual locks when opening
> +	 * files. For them fanotify permission events have high chances of
> +	 * deadlocking the system - open done when reporting fanotify event
> +	 * blocks on this "unusual" lock while another process holding the lock
> +	 * waits for fanotify permission event to be answered. Just disallow
> +	 * permission events for such filesystems.
> +	 */
> +	if (mask & FANOTIFY_PERM_EVENTS &&
> +	    path->mnt->mnt_sb->s_type->fs_flags & FS_DISALLOW_NOTIFY_PERM)
> +		return -EINVAL;
> +	return 0;
> +}
> +
>  static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>  			    int dfd, const char  __user *pathname)
>  {
> @@ -1018,6 +1034,12 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>  	if (ret)
>  		goto fput_and_out;
>  
> +	if (flags & FAN_MARK_ADD) {
> +		ret = fanotify_events_supported(&path, mask);
> +		if (ret)
> +			goto path_put_and_out;
> +	}
> +
>  	if (FAN_GROUP_FLAG(group, FAN_REPORT_FID)) {
>  		ret = fanotify_test_fid(&path, &__fsid);
>  		if (ret)
> diff --git a/fs/proc/root.c b/fs/proc/root.c
> index 8b145e7b9661..522199e9525e 100644
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@ -211,7 +211,7 @@ static struct file_system_type proc_fs_type = {
>  	.init_fs_context	= proc_init_fs_context,
>  	.parameters		= &proc_fs_parameters,
>  	.kill_sb		= proc_kill_sb,
> -	.fs_flags		= FS_USERNS_MOUNT,
> +	.fs_flags		= FS_USERNS_MOUNT | FS_DISALLOW_NOTIFY_PERM,
>  };
>  
>  void __init proc_root_init(void)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index f7fdfe93e25d..c7136c98b5ba 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2184,6 +2184,7 @@ struct file_system_type {
>  #define FS_BINARY_MOUNTDATA	2
>  #define FS_HAS_SUBTYPE		4
>  #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
> +#define FS_DISALLOW_NOTIFY_PERM	16	/* Disable fanotify permission events */
>  #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
>  	int (*init_fs_context)(struct fs_context *);
>  	const struct fs_parameter_description *parameters;
> -- 
> 2.16.4
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] fanotify: Disallow permission events for proc filesystem
  2019-05-28 15:54 ` Jan Kara
@ 2019-05-28 16:02   ` Amir Goldstein
  2019-05-28 16:08     ` Jan Kara
  0 siblings, 1 reply; 4+ messages in thread
From: Amir Goldstein @ 2019-05-28 16:02 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-fsdevel

On Tue, May 28, 2019 at 6:54 PM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 16-05-19 13:56:19, Jan Kara wrote:
> > Proc filesystem has special locking rules for various files. Thus
> > fanotify which opens files on event delivery can easily deadlock
> > against another process that waits for fanotify permission event to be
> > handled. Since permission events on /proc have doubtful value anyway,
> > just disallow them.
> >
> > Link: https://lore.kernel.org/linux-fsdevel/20190320131642.GE9485@quack2.suse.cz/
> > Signed-off-by: Jan Kara <jack@suse.cz>
>
> Amir, ping? What do you think about this version of the patch?

Sorry, I reviewed but forgot to reply.
You may add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] fanotify: Disallow permission events for proc filesystem
  2019-05-28 16:02   ` Amir Goldstein
@ 2019-05-28 16:08     ` Jan Kara
  0 siblings, 0 replies; 4+ messages in thread
From: Jan Kara @ 2019-05-28 16:08 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Jan Kara, linux-fsdevel

On Tue 28-05-19 19:02:46, Amir Goldstein wrote:
> On Tue, May 28, 2019 at 6:54 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Thu 16-05-19 13:56:19, Jan Kara wrote:
> > > Proc filesystem has special locking rules for various files. Thus
> > > fanotify which opens files on event delivery can easily deadlock
> > > against another process that waits for fanotify permission event to be
> > > handled. Since permission events on /proc have doubtful value anyway,
> > > just disallow them.
> > >
> > > Link: https://lore.kernel.org/linux-fsdevel/20190320131642.GE9485@quack2.suse.cz/
> > > Signed-off-by: Jan Kara <jack@suse.cz>
> >
> > Amir, ping? What do you think about this version of the patch?
> 
> Sorry, I reviewed but forgot to reply.
> You may add:
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks! I'll push it to my tree for the next merge window then.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-05-28 16:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-16 11:56 [PATCH v2] fanotify: Disallow permission events for proc filesystem Jan Kara
2019-05-28 15:54 ` Jan Kara
2019-05-28 16:02   ` Amir Goldstein
2019-05-28 16:08     ` Jan Kara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.