* [PATCH] fanotify: refine the validation checks on non-dir inode mask
@ 2022-06-27 17:47 Amir Goldstein
2022-06-28 9:27 ` Jan Kara
0 siblings, 1 reply; 5+ messages in thread
From: Amir Goldstein @ 2022-06-27 17:47 UTC (permalink / raw)
To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, linux-api
Commit ceaf69f8eadc ("fanotify: do not allow setting dirent events in
mask of non-dir") added restrictions about setting dirent events in the
mask of a non-dir inode mark, which does not make any sense.
For backward compatibility, these restictions were added only to new
(v5.17+) APIs.
It also does not make any sense to set the flags FAN_EVENT_ON_CHILD or
FAN_ONDIR in the mask of a non-dir inode. Add these flags to the
dir-only restriction of the new APIs as well.
Move the check of the dir-only flags for new APIs into the helper
fanotify_events_supported(), which is only called for FAN_MARK_ADD,
because there is no need to error on an attempt to remove the dir-only
flags from non-dir inode.
Fixes: ceaf69f8eadc ("fanotify: do not allow setting dirent events in mask of non-dir")
Link: https://lore.kernel.org/linux-fsdevel/20220627113224.kr2725conevh53u4@quack3.lan/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
Hi Jan,
Here is the retroactive API fix that we dicussed merging to v5.19.
There is an LTP test [1] and an update for the man page patch of
FAN_REPORT_TARGET_FID [2], which is still in review.
Thanks,
Amir.
[1] https://github.com/amir73il/ltp/commits/fan_enotdir
[2] https://github.com/amir73il/man-pages/commits/fanotify_target_fid
fs/notify/fanotify/fanotify_user.c | 34 +++++++++++++++++-------------
include/linux/fanotify.h | 4 ++++
2 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index c2255b440df9..b08ce0d821a7 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1513,8 +1513,15 @@ static int fanotify_test_fid(struct dentry *dentry)
return 0;
}
-static int fanotify_events_supported(struct path *path, __u64 mask)
+static int fanotify_events_supported(struct fsnotify_group *group,
+ struct path *path, __u64 mask,
+ unsigned int flags)
{
+ unsigned int mark_type = flags & FANOTIFY_MARK_TYPE_BITS;
+ /* Strict validation of events in non-dir inode mask with v5.17+ APIs */
+ bool strict_dir_events = FAN_GROUP_FLAG(group, FAN_REPORT_TARGET_FID) ||
+ (mask & FAN_RENAME);
+
/*
* Some filesystems such as 'proc' acquire unusual locks when opening
* files. For them fanotify permission events have high chances of
@@ -1526,6 +1533,16 @@ static int fanotify_events_supported(struct path *path, __u64 mask)
if (mask & FANOTIFY_PERM_EVENTS &&
path->mnt->mnt_sb->s_type->fs_flags & FS_DISALLOW_NOTIFY_PERM)
return -EINVAL;
+
+ /*
+ * We shouldn't have allowed setting dirent events and the directory
+ * flags FAN_ONDIR and FAN_EVENT_ON_CHILD in mask of non-dir inode,
+ * but because we always allowed it, error only when using new APIs.
+ */
+ if (strict_dir_events && mark_type == FAN_MARK_INODE &&
+ !d_is_dir(path->dentry) && (mask & FANOTIFY_DIRONLY_EVENT_BITS))
+ return -ENOTDIR;
+
return 0;
}
@@ -1672,7 +1689,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
goto fput_and_out;
if (flags & FAN_MARK_ADD) {
- ret = fanotify_events_supported(&path, mask);
+ ret = fanotify_events_supported(group, &path, mask, flags);
if (ret)
goto path_put_and_out;
}
@@ -1695,19 +1712,6 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
else
mnt = path.mnt;
- /*
- * FAN_RENAME is not allowed on non-dir (for now).
- * We shouldn't have allowed setting any dirent events in mask of
- * non-dir, but because we always allowed it, error only if group
- * was initialized with the new flag FAN_REPORT_TARGET_FID.
- */
- ret = -ENOTDIR;
- if (inode && !S_ISDIR(inode->i_mode) &&
- ((mask & FAN_RENAME) ||
- ((mask & FANOTIFY_DIRENT_EVENTS) &&
- FAN_GROUP_FLAG(group, FAN_REPORT_TARGET_FID))))
- goto path_put_and_out;
-
/* Mask out FAN_EVENT_ON_CHILD flag for sb/mount/non-dir marks */
if (mnt || !S_ISDIR(inode->i_mode)) {
mask &= ~FAN_EVENT_ON_CHILD;
diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
index edc28555814c..e517dbcf74ed 100644
--- a/include/linux/fanotify.h
+++ b/include/linux/fanotify.h
@@ -111,6 +111,10 @@
FANOTIFY_PERM_EVENTS | \
FAN_Q_OVERFLOW | FAN_ONDIR)
+/* Events and flags relevant only for directories */
+#define FANOTIFY_DIRONLY_EVENT_BITS (FANOTIFY_DIRENT_EVENTS | \
+ FAN_EVENT_ON_CHILD | FAN_ONDIR)
+
#define ALL_FANOTIFY_EVENT_BITS (FANOTIFY_OUTGOING_EVENTS | \
FANOTIFY_EVENT_FLAGS)
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] fanotify: refine the validation checks on non-dir inode mask
2022-06-27 17:47 [PATCH] fanotify: refine the validation checks on non-dir inode mask Amir Goldstein
@ 2022-06-28 9:27 ` Jan Kara
2022-06-28 17:22 ` Amir Goldstein
0 siblings, 1 reply; 5+ messages in thread
From: Jan Kara @ 2022-06-28 9:27 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Jan Kara, Matthew Bobrowski, linux-fsdevel, linux-api
On Mon 27-06-22 20:47:19, Amir Goldstein wrote:
> Commit ceaf69f8eadc ("fanotify: do not allow setting dirent events in
> mask of non-dir") added restrictions about setting dirent events in the
> mask of a non-dir inode mark, which does not make any sense.
>
> For backward compatibility, these restictions were added only to new
> (v5.17+) APIs.
>
> It also does not make any sense to set the flags FAN_EVENT_ON_CHILD or
> FAN_ONDIR in the mask of a non-dir inode. Add these flags to the
> dir-only restriction of the new APIs as well.
>
> Move the check of the dir-only flags for new APIs into the helper
> fanotify_events_supported(), which is only called for FAN_MARK_ADD,
> because there is no need to error on an attempt to remove the dir-only
> flags from non-dir inode.
>
> Fixes: ceaf69f8eadc ("fanotify: do not allow setting dirent events in mask of non-dir")
> Link: https://lore.kernel.org/linux-fsdevel/20220627113224.kr2725conevh53u4@quack3.lan/
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Thanks! I've taken the patch to my tree.
Honza
> [1] https://github.com/amir73il/ltp/commits/fan_enotdir
> [2] https://github.com/amir73il/man-pages/commits/fanotify_target_fid
>
>
> fs/notify/fanotify/fanotify_user.c | 34 +++++++++++++++++-------------
> include/linux/fanotify.h | 4 ++++
> 2 files changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index c2255b440df9..b08ce0d821a7 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1513,8 +1513,15 @@ static int fanotify_test_fid(struct dentry *dentry)
> return 0;
> }
>
> -static int fanotify_events_supported(struct path *path, __u64 mask)
> +static int fanotify_events_supported(struct fsnotify_group *group,
> + struct path *path, __u64 mask,
> + unsigned int flags)
> {
> + unsigned int mark_type = flags & FANOTIFY_MARK_TYPE_BITS;
> + /* Strict validation of events in non-dir inode mask with v5.17+ APIs */
> + bool strict_dir_events = FAN_GROUP_FLAG(group, FAN_REPORT_TARGET_FID) ||
> + (mask & FAN_RENAME);
> +
> /*
> * Some filesystems such as 'proc' acquire unusual locks when opening
> * files. For them fanotify permission events have high chances of
> @@ -1526,6 +1533,16 @@ static int fanotify_events_supported(struct path *path, __u64 mask)
> if (mask & FANOTIFY_PERM_EVENTS &&
> path->mnt->mnt_sb->s_type->fs_flags & FS_DISALLOW_NOTIFY_PERM)
> return -EINVAL;
> +
> + /*
> + * We shouldn't have allowed setting dirent events and the directory
> + * flags FAN_ONDIR and FAN_EVENT_ON_CHILD in mask of non-dir inode,
> + * but because we always allowed it, error only when using new APIs.
> + */
> + if (strict_dir_events && mark_type == FAN_MARK_INODE &&
> + !d_is_dir(path->dentry) && (mask & FANOTIFY_DIRONLY_EVENT_BITS))
> + return -ENOTDIR;
> +
> return 0;
> }
>
> @@ -1672,7 +1689,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
> goto fput_and_out;
>
> if (flags & FAN_MARK_ADD) {
> - ret = fanotify_events_supported(&path, mask);
> + ret = fanotify_events_supported(group, &path, mask, flags);
> if (ret)
> goto path_put_and_out;
> }
> @@ -1695,19 +1712,6 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
> else
> mnt = path.mnt;
>
> - /*
> - * FAN_RENAME is not allowed on non-dir (for now).
> - * We shouldn't have allowed setting any dirent events in mask of
> - * non-dir, but because we always allowed it, error only if group
> - * was initialized with the new flag FAN_REPORT_TARGET_FID.
> - */
> - ret = -ENOTDIR;
> - if (inode && !S_ISDIR(inode->i_mode) &&
> - ((mask & FAN_RENAME) ||
> - ((mask & FANOTIFY_DIRENT_EVENTS) &&
> - FAN_GROUP_FLAG(group, FAN_REPORT_TARGET_FID))))
> - goto path_put_and_out;
> -
> /* Mask out FAN_EVENT_ON_CHILD flag for sb/mount/non-dir marks */
> if (mnt || !S_ISDIR(inode->i_mode)) {
> mask &= ~FAN_EVENT_ON_CHILD;
> diff --git a/include/linux/fanotify.h b/include/linux/fanotify.h
> index edc28555814c..e517dbcf74ed 100644
> --- a/include/linux/fanotify.h
> +++ b/include/linux/fanotify.h
> @@ -111,6 +111,10 @@
> FANOTIFY_PERM_EVENTS | \
> FAN_Q_OVERFLOW | FAN_ONDIR)
>
> +/* Events and flags relevant only for directories */
> +#define FANOTIFY_DIRONLY_EVENT_BITS (FANOTIFY_DIRENT_EVENTS | \
> + FAN_EVENT_ON_CHILD | FAN_ONDIR)
> +
> #define ALL_FANOTIFY_EVENT_BITS (FANOTIFY_OUTGOING_EVENTS | \
> FANOTIFY_EVENT_FLAGS)
>
> --
> 2.25.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fanotify: refine the validation checks on non-dir inode mask
2022-06-28 9:27 ` Jan Kara
@ 2022-06-28 17:22 ` Amir Goldstein
2022-06-29 10:09 ` Jan Kara
2022-07-02 5:59 ` Matthew Bobrowski
0 siblings, 2 replies; 5+ messages in thread
From: Amir Goldstein @ 2022-06-28 17:22 UTC (permalink / raw)
To: Jan Kara; +Cc: Matthew Bobrowski, linux-fsdevel, Linux API
On Tue, Jun 28, 2022 at 12:27 PM Jan Kara <jack@suse.cz> wrote:
>
> On Mon 27-06-22 20:47:19, Amir Goldstein wrote:
> > Commit ceaf69f8eadc ("fanotify: do not allow setting dirent events in
> > mask of non-dir") added restrictions about setting dirent events in the
> > mask of a non-dir inode mark, which does not make any sense.
> >
> > For backward compatibility, these restictions were added only to new
> > (v5.17+) APIs.
> >
> > It also does not make any sense to set the flags FAN_EVENT_ON_CHILD or
> > FAN_ONDIR in the mask of a non-dir inode. Add these flags to the
> > dir-only restriction of the new APIs as well.
> >
> > Move the check of the dir-only flags for new APIs into the helper
> > fanotify_events_supported(), which is only called for FAN_MARK_ADD,
> > because there is no need to error on an attempt to remove the dir-only
> > flags from non-dir inode.
> >
> > Fixes: ceaf69f8eadc ("fanotify: do not allow setting dirent events in mask of non-dir")
> > Link: https://lore.kernel.org/linux-fsdevel/20220627113224.kr2725conevh53u4@quack3.lan/
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>
> Thanks! I've taken the patch to my tree.
>
> Honza
>
> > [1] https://github.com/amir73il/ltp/commits/fan_enotdir
> > [2] https://github.com/amir73il/man-pages/commits/fanotify_target_fid
Mathew and Jan,
Please let me know if I can keep your RVB on the man page patch for
FAN_REPORT_TARGET_FID linked above.
The only change is an update to the ENOTDIR section which ends up like this:
ENOTDIR
flags contains FAN_MARK_ONLYDIR, and dirfd and pathname
do not specify a directory.
ENOTDIR
mask contains FAN_RENAME, and dirfd and pathname do not
specify a directory.
ENOTDIR
flags contains FAN_MARK_IGNORE, or the fanotify group
was initialized with
flag FAN_REPORT_TARGET_FID, and mask contains directory
entry modification
events (e.g., FAN_CREATE, FAN_DELETE), or directory event flags
(e.g., FAN_ONDIR, FAN_EVENT_ON_CHILD),
and dirfd and pathname do not specify a directory.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fanotify: refine the validation checks on non-dir inode mask
2022-06-28 17:22 ` Amir Goldstein
@ 2022-06-29 10:09 ` Jan Kara
2022-07-02 5:59 ` Matthew Bobrowski
1 sibling, 0 replies; 5+ messages in thread
From: Jan Kara @ 2022-06-29 10:09 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Jan Kara, Matthew Bobrowski, linux-fsdevel, Linux API
On Tue 28-06-22 20:22:28, Amir Goldstein wrote:
> On Tue, Jun 28, 2022 at 12:27 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Mon 27-06-22 20:47:19, Amir Goldstein wrote:
> > > Commit ceaf69f8eadc ("fanotify: do not allow setting dirent events in
> > > mask of non-dir") added restrictions about setting dirent events in the
> > > mask of a non-dir inode mark, which does not make any sense.
> > >
> > > For backward compatibility, these restictions were added only to new
> > > (v5.17+) APIs.
> > >
> > > It also does not make any sense to set the flags FAN_EVENT_ON_CHILD or
> > > FAN_ONDIR in the mask of a non-dir inode. Add these flags to the
> > > dir-only restriction of the new APIs as well.
> > >
> > > Move the check of the dir-only flags for new APIs into the helper
> > > fanotify_events_supported(), which is only called for FAN_MARK_ADD,
> > > because there is no need to error on an attempt to remove the dir-only
> > > flags from non-dir inode.
> > >
> > > Fixes: ceaf69f8eadc ("fanotify: do not allow setting dirent events in mask of non-dir")
> > > Link: https://lore.kernel.org/linux-fsdevel/20220627113224.kr2725conevh53u4@quack3.lan/
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >
> > Thanks! I've taken the patch to my tree.
> >
> > Honza
> >
> > > [1] https://github.com/amir73il/ltp/commits/fan_enotdir
> > > [2] https://github.com/amir73il/man-pages/commits/fanotify_target_fid
>
> Mathew and Jan,
>
> Please let me know if I can keep your RVB on the man page patch for
> FAN_REPORT_TARGET_FID linked above.
>
> The only change is an update to the ENOTDIR section which ends up like this:
>
> ENOTDIR
> flags contains FAN_MARK_ONLYDIR, and dirfd and pathname
> do not specify a directory.
>
> ENOTDIR
> mask contains FAN_RENAME, and dirfd and pathname do not
> specify a directory.
>
> ENOTDIR
> flags contains FAN_MARK_IGNORE, or the fanotify group
> was initialized with
> flag FAN_REPORT_TARGET_FID, and mask contains directory
> entry modification
> events (e.g., FAN_CREATE, FAN_DELETE), or directory event flags
> (e.g., FAN_ONDIR, FAN_EVENT_ON_CHILD),
> and dirfd and pathname do not specify a directory.
Looks good to me. Thanks!
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fanotify: refine the validation checks on non-dir inode mask
2022-06-28 17:22 ` Amir Goldstein
2022-06-29 10:09 ` Jan Kara
@ 2022-07-02 5:59 ` Matthew Bobrowski
1 sibling, 0 replies; 5+ messages in thread
From: Matthew Bobrowski @ 2022-07-02 5:59 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Jan Kara, linux-fsdevel, Linux API
On Tue, Jun 28, 2022 at 08:22:28PM +0300, Amir Goldstein wrote:
> On Tue, Jun 28, 2022 at 12:27 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Mon 27-06-22 20:47:19, Amir Goldstein wrote:
> > > Commit ceaf69f8eadc ("fanotify: do not allow setting dirent events in
> > > mask of non-dir") added restrictions about setting dirent events in the
> > > mask of a non-dir inode mark, which does not make any sense.
> > >
> > > For backward compatibility, these restictions were added only to new
> > > (v5.17+) APIs.
> > >
> > > It also does not make any sense to set the flags FAN_EVENT_ON_CHILD or
> > > FAN_ONDIR in the mask of a non-dir inode. Add these flags to the
> > > dir-only restriction of the new APIs as well.
> > >
> > > Move the check of the dir-only flags for new APIs into the helper
> > > fanotify_events_supported(), which is only called for FAN_MARK_ADD,
> > > because there is no need to error on an attempt to remove the dir-only
> > > flags from non-dir inode.
> > >
> > > Fixes: ceaf69f8eadc ("fanotify: do not allow setting dirent events in mask of non-dir")
> > > Link: https://lore.kernel.org/linux-fsdevel/20220627113224.kr2725conevh53u4@quack3.lan/
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >
> > Thanks! I've taken the patch to my tree.
> >
> > Honza
> >
> > > [1] https://github.com/amir73il/ltp/commits/fan_enotdir
> > > [2] https://github.com/amir73il/man-pages/commits/fanotify_target_fid
>
> Mathew and Jan,
>
> Please let me know if I can keep your RVB on the man page patch for
> FAN_REPORT_TARGET_FID linked above.
>
> The only change is an update to the ENOTDIR section which ends up like this:
>
> ENOTDIR
> flags contains FAN_MARK_ONLYDIR, and dirfd and pathname
> do not specify a directory.
>
> ENOTDIR
> mask contains FAN_RENAME, and dirfd and pathname do not
> specify a directory.
>
> ENOTDIR
> flags contains FAN_MARK_IGNORE, or the fanotify group
> was initialized with
> flag FAN_REPORT_TARGET_FID, and mask contains directory
> entry modification
> events (e.g., FAN_CREATE, FAN_DELETE), or directory event flags
> (e.g., FAN_ONDIR, FAN_EVENT_ON_CHILD),
> and dirfd and pathname do not specify a directory.
Sorry for the delay. This looks fine to me.
/M
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-02 5:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-27 17:47 [PATCH] fanotify: refine the validation checks on non-dir inode mask Amir Goldstein
2022-06-28 9:27 ` Jan Kara
2022-06-28 17:22 ` Amir Goldstein
2022-06-29 10:09 ` Jan Kara
2022-07-02 5:59 ` Matthew Bobrowski
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.