[PATCH] fanotify: store fanotify_init() flags in group's fanotify_data

[PATCH] fanotify: store fanotify_init() flags in group's fanotify_data

[Amir Goldstein]

This averts the need to re-generate flags in fanotify_show_fdinfo()
and sets the scene for addition of more upcoming flags without growing
new members to the fanotify_data struct.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Jan,

Please consider this small "cleanup" patch for next.
It's part of my fanotify_dentry series, so wasn't going to send it out
yet, but seems like other developers can find it useful [1] in the mean
while.

Thanks,
Amir.

[1] https://lkml.org/lkml/2018/9/18/942

 fs/notify/fanotify/fanotify_user.c |  9 +++++----
 fs/notify/fdinfo.c                 | 24 +-----------------------
 include/linux/fsnotify_backend.h   |  4 ++--
 3 files changed, 8 insertions(+), 29 deletions(-)

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index 1347c588f778..0562738a4c51 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -191,7 +191,8 @@ static int process_access_response(struct fsnotify_group *group,
 	if (fd < 0)
 		return -EINVAL;
 
-	if ((response & FAN_AUDIT) && !group->fanotify_data.audit)
+	if ((response & FAN_AUDIT) &&
+	    !(group->fanotify_data.flags & FAN_ENABLE_AUDIT))
 		return -EINVAL;
 
 	event = dequeue_event(group, fd);
@@ -701,8 +702,8 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 	struct user_struct *user;
 	struct fanotify_event_info *oevent;
 
-	pr_debug("%s: flags=%d event_f_flags=%d\n",
-		__func__, flags, event_f_flags);
+	pr_debug("%s: flags=%x event_f_flags=%x\n",
+		 __func__, flags, event_f_flags);
 
 	if (!capable(CAP_SYS_ADMIN))
 		return -EPERM;
@@ -798,13 +799,13 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
 		fd = -EPERM;
 		if (!capable(CAP_AUDIT_WRITE))
 			goto out_destroy_group;
-		group->fanotify_data.audit = true;
 	}
 
 	fd = anon_inode_getfd("[fanotify]", &fanotify_fops, group, f_flags);
 	if (fd < 0)
 		goto out_destroy_group;
 
+	group->fanotify_data.flags = flags;
 	return fd;
 
 out_destroy_group:
diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
index 25385e336ac7..348a184bcdda 100644
--- a/fs/notify/fdinfo.c
+++ b/fs/notify/fdinfo.c
@@ -142,31 +142,9 @@ static void fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
 void fanotify_show_fdinfo(struct seq_file *m, struct file *f)
 {
 	struct fsnotify_group *group = f->private_data;
-	unsigned int flags = 0;
-
-	switch (group->priority) {
-	case FS_PRIO_0:
-		flags |= FAN_CLASS_NOTIF;
-		break;
-	case FS_PRIO_1:
-		flags |= FAN_CLASS_CONTENT;
-		break;
-	case FS_PRIO_2:
-		flags |= FAN_CLASS_PRE_CONTENT;
-		break;
-	}
-
-	if (group->max_events == UINT_MAX)
-		flags |= FAN_UNLIMITED_QUEUE;
-
-	if (group->fanotify_data.max_marks == UINT_MAX)
-		flags |= FAN_UNLIMITED_MARKS;
-
-	if (group->fanotify_data.audit)
-		flags |= FAN_ENABLE_AUDIT;
 
 	seq_printf(m, "fanotify flags:%x event-flags:%x\n",
-		   flags, group->fanotify_data.f_flags);
+		   group->fanotify_data.flags, group->fanotify_data.f_flags);
 
 	show_fdinfo(m, f, fanotify_fdinfo);
 }
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 81b88fc9df31..8e91341cbd8a 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -189,10 +189,10 @@ struct fsnotify_group {
 			/* allows a group to block waiting for a userspace response */
 			struct list_head access_list;
 			wait_queue_head_t access_waitq;
-			int f_flags;
+			int flags;           /* flags from fanotify_init() */
+			int f_flags; /* event_f_flags from fanotify_init() */
 			unsigned int max_marks;
 			struct user_struct *user;
-			bool audit;
 		} fanotify_data;
 #endif /* CONFIG_FANOTIFY */
 	};
-- 
2.17.1

Re: [PATCH] fanotify: store fanotify_init() flags in group's fanotify_data

[Jan Kara]

On Tue 18-09-18 18:44:34, Amir Goldstein wrote:
> This averts the need to re-generate flags in fanotify_show_fdinfo()
> and sets the scene for addition of more upcoming flags without growing
> new members to the fanotify_data struct.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Thanks. I've taken this patch to my tree for the next merge window.

								Honza

> ---
> 
> Jan,
> 
> Please consider this small "cleanup" patch for next.
> It's part of my fanotify_dentry series, so wasn't going to send it out
> yet, but seems like other developers can find it useful [1] in the mean
> while.
> 
> Thanks,
> Amir.
> 
> [1] https://lkml.org/lkml/2018/9/18/942
> 
>  fs/notify/fanotify/fanotify_user.c |  9 +++++----
>  fs/notify/fdinfo.c                 | 24 +-----------------------
>  include/linux/fsnotify_backend.h   |  4 ++--
>  3 files changed, 8 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index 1347c588f778..0562738a4c51 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -191,7 +191,8 @@ static int process_access_response(struct fsnotify_group *group,
>  	if (fd < 0)
>  		return -EINVAL;
>  
> -	if ((response & FAN_AUDIT) && !group->fanotify_data.audit)
> +	if ((response & FAN_AUDIT) &&
> +	    !(group->fanotify_data.flags & FAN_ENABLE_AUDIT))
>  		return -EINVAL;
>  
>  	event = dequeue_event(group, fd);
> @@ -701,8 +702,8 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>  	struct user_struct *user;
>  	struct fanotify_event_info *oevent;
>  
> -	pr_debug("%s: flags=%d event_f_flags=%d\n",
> -		__func__, flags, event_f_flags);
> +	pr_debug("%s: flags=%x event_f_flags=%x\n",
> +		 __func__, flags, event_f_flags);
>  
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EPERM;
> @@ -798,13 +799,13 @@ SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
>  		fd = -EPERM;
>  		if (!capable(CAP_AUDIT_WRITE))
>  			goto out_destroy_group;
> -		group->fanotify_data.audit = true;
>  	}
>  
>  	fd = anon_inode_getfd("[fanotify]", &fanotify_fops, group, f_flags);
>  	if (fd < 0)
>  		goto out_destroy_group;
>  
> +	group->fanotify_data.flags = flags;
>  	return fd;
>  
>  out_destroy_group:
> diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c
> index 25385e336ac7..348a184bcdda 100644
> --- a/fs/notify/fdinfo.c
> +++ b/fs/notify/fdinfo.c
> @@ -142,31 +142,9 @@ static void fanotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark)
>  void fanotify_show_fdinfo(struct seq_file *m, struct file *f)
>  {
>  	struct fsnotify_group *group = f->private_data;
> -	unsigned int flags = 0;
> -
> -	switch (group->priority) {
> -	case FS_PRIO_0:
> -		flags |= FAN_CLASS_NOTIF;
> -		break;
> -	case FS_PRIO_1:
> -		flags |= FAN_CLASS_CONTENT;
> -		break;
> -	case FS_PRIO_2:
> -		flags |= FAN_CLASS_PRE_CONTENT;
> -		break;
> -	}
> -
> -	if (group->max_events == UINT_MAX)
> -		flags |= FAN_UNLIMITED_QUEUE;
> -
> -	if (group->fanotify_data.max_marks == UINT_MAX)
> -		flags |= FAN_UNLIMITED_MARKS;
> -
> -	if (group->fanotify_data.audit)
> -		flags |= FAN_ENABLE_AUDIT;
>  
>  	seq_printf(m, "fanotify flags:%x event-flags:%x\n",
> -		   flags, group->fanotify_data.f_flags);
> +		   group->fanotify_data.flags, group->fanotify_data.f_flags);
>  
>  	show_fdinfo(m, f, fanotify_fdinfo);
>  }
> diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
> index 81b88fc9df31..8e91341cbd8a 100644
> --- a/include/linux/fsnotify_backend.h
> +++ b/include/linux/fsnotify_backend.h
> @@ -189,10 +189,10 @@ struct fsnotify_group {
>  			/* allows a group to block waiting for a userspace response */
>  			struct list_head access_list;
>  			wait_queue_head_t access_waitq;
> -			int f_flags;
> +			int flags;           /* flags from fanotify_init() */
> +			int f_flags; /* event_f_flags from fanotify_init() */
>  			unsigned int max_marks;
>  			struct user_struct *user;
> -			bool audit;
>  		} fanotify_data;
>  #endif /* CONFIG_FANOTIFY */
>  	};
> -- 
> 2.17.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] fanotify: store fanotify_init() flags in group's fanotify_data

[Amir Goldstein]

On Wed, Sep 19, 2018 at 4:47 PM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 18-09-18 18:44:34, Amir Goldstein wrote:
> > This averts the need to re-generate flags in fanotify_show_fdinfo()
> > and sets the scene for addition of more upcoming flags without growing
> > new members to the fanotify_data struct.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> Thanks. I've taken this patch to my tree for the next merge window.
>

Actually, could you please grab the later version of this patch, which
is a bit nicer, from:
https://github.com/amir73il/linux/commits/fanotify_unpriv

I was going to post v2, but you beat me to it.
Below is v1..v2 diff.

Thanks!
Amir.

----
diff --git a/fs/notify/fanotify/fanotify_user.c
b/fs/notify/fanotify/fanotify_user.c
index 0562738a4c51..afa89eafe79f 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -191,8 +191,7 @@ static int process_access_response(struct
fsnotify_group *group,
        if (fd < 0)
                return -EINVAL;

-       if ((response & FAN_AUDIT) &&
-           !(group->fanotify_data.flags & FAN_ENABLE_AUDIT))
+       if ((response & FAN_AUDIT) && !FAN_GROUP_FLAG(group, FAN_ENABLE_AUDIT))
                return -EINVAL;

        event = dequeue_event(group, fd);
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index 096c96f4f16a..9c5ea3bdfaa0 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -6,4 +6,8 @@

 /* not valid from userspace, only kernel internal */
 #define FAN_MARK_ONDIR         0x00000100
+
+#define FAN_GROUP_FLAG(group, flag) \
+       ((group)->fanotify_data.flags & (flag))
+
 #endif /* _LINUX_FANOTIFY_H */