On Wed, Nov 7, 2018 at 1:18 PM Matthew Bobrowski
<mbobrowski@xxxxxxxxxxxxxx> wrote:
>
> Permission events are not to be consolidated into a single event mask.
> In order for this to not happen, we require additional calls to
> fsnotify_parent() and fsnotify() within the fsnotify_perm() when the
> conditon to set FS_OPEN_EXEC_PERM is evaluated to true.
>
That shouldn't be a separate patch. it should be squashed into the patch
introducing FS_OPEN_EXEC_PERM there is no reason to have an
interim commit where events are merged.
> To simplify the code that provides this functionality a simple wrapper
> fsnotify_path() has been defined to keep things nice and clean. Other
> functions that used the same fsnotify_parent()/fsnotify() call
> combination have been updated to use the simplified fsnotify_path()
> wrapper.
>
And this should be a separate re-factoring patch.
> Signed-off-by: Matthew Bobrowski <mbobrowski@xxxxxxxxxxxxxx>
> ---
> include/linux/fsnotify.h | 48 ++++++++++++++++++++++------------------
> 1 file changed, 27 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> index 9c7b594bf540..660ffc751352 100644
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -26,6 +26,21 @@ static inline int fsnotify_parent(const struct path *path, struct dentry *dentry
> return __fsnotify_parent(path, dentry, mask);
> }
>
> +/*
> + * Simple wrapper to consolidate calls fsnotify_parent()/fsnotify() when
> + * an event is on a path.
> + */
> +static inline int fsnotify_path(struct inode *inode, const struct path *path,
> + __u32 mask)
> +{
> + int ret;
> +
> + ret = fsnotify_parent(path, NULL, mask);
> + if (ret) return ret;
> +
> + return fsnotify(inode, mask, path, FSNOTIFY_EVENT_PATH, NULL, 0);
> +}
> +
> /* Simple call site for access decisions */
> static inline int fsnotify_perm(struct file *file, int mask)
> {
> @@ -41,17 +56,15 @@ static inline int fsnotify_perm(struct file *file, int mask)
> if (mask & MAY_OPEN) {
> fsnotify_mask = FS_OPEN_PERM;
>
> - if (file->f_flags & __FMODE_EXEC)
> - fsnotify_mask |= FS_OPEN_EXEC_PERM;
> + if (file->f_flags & __FMODE_EXEC) {
> + ret = fsnotify_path(inode, path, FS_OPEN_EXEC_PERM);
> + if (ret) return ret;
return should be in newline - just was just me hand writing a patch in email...
After making these small fixes, you may add to patches:
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
Thanks,
Amir.
