[PATCH] fsnotify: compile out fsnotify permission hooks if !FANOTIFY_ACCESS_PERMISSIONS

[PATCH] fsnotify: compile out fsnotify permission hooks if !FANOTIFY_ACCESS_PERMISSIONS

[Amir Goldstein]

The depency of FANOTIFY_ACCESS_PERMISSIONS on SECURITY made sure that
the fsnotify permission hooks were never called when SECURITY was
disabled.

Moving the fsnotify permission hook out of the secutiy hook broke that
optimisation.

Reported-and-tested-by: Jens Axboe <axboe@kernel.dk>
Closes: https://lore.kernel.org/linux-fsdevel/53682ece-f0e7-48de-9a1c-879ee34b0449@kernel.dk/
Fixes: d9e5d31084b0 ("fsnotify: optionally pass access range in file permission hooks")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 include/linux/fsnotify.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 11e6434b8e71..8300a5286988 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -100,6 +100,7 @@ static inline int fsnotify_file(struct file *file, __u32 mask)
 	return fsnotify_parent(path->dentry, mask, path, FSNOTIFY_EVENT_PATH);
 }
 
+#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
 /*
  * fsnotify_file_area_perm - permission hook before access to file range
  */
@@ -145,6 +146,24 @@ static inline int fsnotify_open_perm(struct file *file)
 	return fsnotify_file(file, FS_OPEN_PERM);
 }
 
+#else
+static inline int fsnotify_file_area_perm(struct file *file, int perm_mask,
+					  const loff_t *ppos, size_t count)
+{
+	return 0;
+}
+
+static inline int fsnotify_file_perm(struct file *file, int perm_mask)
+{
+	return 0;
+}
+
+static inline int fsnotify_open_perm(struct file *file)
+{
+	return 0;
+}
+#endif
+
 /*
  * fsnotify_link_count - inode's link count changed
  */
-- 
2.34.1

Re: [PATCH] fsnotify: compile out fsnotify permission hooks if !FANOTIFY_ACCESS_PERMISSIONS

[Jens Axboe]

On 1/9/24 11:22 AM, Amir Goldstein wrote:
> The depency of FANOTIFY_ACCESS_PERMISSIONS on SECURITY made sure that

dependency

> the fsnotify permission hooks were never called when SECURITY was
> disabled.
> 
> Moving the fsnotify permission hook out of the secutiy hook broke that

security

> optimisation.

Patch obviously looks good to me, as I already tested it :-)

Thanks for fixing this up.

-- 
Jens Axboe

Re: [PATCH] fsnotify: compile out fsnotify permission hooks if !FANOTIFY_ACCESS_PERMISSIONS

[Christian Brauner]

On Tue, 09 Jan 2024 20:22:45 +0200, Amir Goldstein wrote:
> The depency of FANOTIFY_ACCESS_PERMISSIONS on SECURITY made sure that
> the fsnotify permission hooks were never called when SECURITY was
> disabled.
> 
> Moving the fsnotify permission hook out of the secutiy hook broke that
> optimisation.
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] fsnotify: compile out fsnotify permission hooks if !FANOTIFY_ACCESS_PERMISSIONS
      https://git.kernel.org/vfs/vfs/c/b0f2ac4fe541

Re: [PATCH] fsnotify: compile out fsnotify permission hooks if !FANOTIFY_ACCESS_PERMISSIONS

[Jan Kara]

On Tue 09-01-24 11:23:49, Jens Axboe wrote:
> On 1/9/24 11:22 AM, Amir Goldstein wrote:
> > The depency of FANOTIFY_ACCESS_PERMISSIONS on SECURITY made sure that
> 
> dependency
> 
> > the fsnotify permission hooks were never called when SECURITY was
> > disabled.
> > 
> > Moving the fsnotify permission hook out of the secutiy hook broke that
> 
> security
> 
> > optimisation.
> 
> Patch obviously looks good to me, as I already tested it :-)
> 
> Thanks for fixing this up.

Thanks Amir for the fix and Jens for the spelling fixes. I'll fix them on
commit.

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

Re: [PATCH] fsnotify: compile out fsnotify permission hooks if !FANOTIFY_ACCESS_PERMISSIONS

[Jan Kara]

On Tue 09-01-24 20:22:45, Amir Goldstein wrote:
> The depency of FANOTIFY_ACCESS_PERMISSIONS on SECURITY made sure that
> the fsnotify permission hooks were never called when SECURITY was
> disabled.
> 
> Moving the fsnotify permission hook out of the secutiy hook broke that
> optimisation.
> 
> Reported-and-tested-by: Jens Axboe <axboe@kernel.dk>
> Closes: https://lore.kernel.org/linux-fsdevel/53682ece-f0e7-48de-9a1c-879ee34b0449@kernel.dk/
> Fixes: d9e5d31084b0 ("fsnotify: optionally pass access range in file permission hooks")
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Originally I didn't notice this was directed to Christian but it makes
sense since he merged the original patches. The fix looks good (modulo the
typo fixes from Jens). Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  include/linux/fsnotify.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 11e6434b8e71..8300a5286988 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -100,6 +100,7 @@ static inline int fsnotify_file(struct file *file, __u32 mask)
>  	return fsnotify_parent(path->dentry, mask, path, FSNOTIFY_EVENT_PATH);
>  }
>  
> +#ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
>  /*
>   * fsnotify_file_area_perm - permission hook before access to file range
>   */
> @@ -145,6 +146,24 @@ static inline int fsnotify_open_perm(struct file *file)
>  	return fsnotify_file(file, FS_OPEN_PERM);
>  }
>  
> +#else
> +static inline int fsnotify_file_area_perm(struct file *file, int perm_mask,
> +					  const loff_t *ppos, size_t count)
> +{
> +	return 0;
> +}
> +
> +static inline int fsnotify_file_perm(struct file *file, int perm_mask)
> +{
> +	return 0;
> +}
> +
> +static inline int fsnotify_open_perm(struct file *file)
> +{
> +	return 0;
> +}
> +#endif
> +
>  /*
>   * fsnotify_link_count - inode's link count changed
>   */
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR