diff mbox series

[1/5] fanotify: limit reporting of event with non-decodeable file handles

Message ID 20231018100000.2453965-2-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show
Series Support more filesystems with FAN_REPORT_FID | expand

Commit Message

Amir Goldstein Oct. 18, 2023, 9:59 a.m. UTC
Commit a95aef69a740 ("fanotify: support reporting non-decodeable file
handles") merged in v6.5-rc1, added the ability to use an fanotify group
with FAN_REPORT_FID mode to watch filesystems that do not support nfs
export, but do know how to encode non-decodeable file handles, with the
newly introduced AT_HANDLE_FID flag.

At the time that this commit was merged, there were no filesystems
in-tree with those traits.

Commit 16aac5ad1fa9 ("ovl: support encoding non-decodable file handles"),
merged in v6.6-rc1, added this trait to overlayfs, thus allowing fanotify
watching of overlayfs with FAN_REPORT_FID mode.

In retrospect, allowing an fanotify filesystem/mount mark on such
filesystem in FAN_REPORT_FID mode will result in getting events with
file handles, without the ability to resolve the filesystem objects from
those file handles (i.e. no open_by_handle_at() support).

For v6.6, the safer option would be to allow this mode for inode marks
only, where the caller has the opportunity to use name_to_handle_at() at
the time of setting the mark. In the future we can revise this decision.

Fixes: a95aef69a740 ("fanotify: support reporting non-decodeable file handles")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/notify/fanotify/fanotify_user.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

Comments

Jan Kara Oct. 19, 2023, 2:22 p.m. UTC | #1
On Wed 18-10-23 12:59:56, Amir Goldstein wrote:
> Commit a95aef69a740 ("fanotify: support reporting non-decodeable file
> handles") merged in v6.5-rc1, added the ability to use an fanotify group
> with FAN_REPORT_FID mode to watch filesystems that do not support nfs
> export, but do know how to encode non-decodeable file handles, with the
> newly introduced AT_HANDLE_FID flag.
> 
> At the time that this commit was merged, there were no filesystems
> in-tree with those traits.
> 
> Commit 16aac5ad1fa9 ("ovl: support encoding non-decodable file handles"),
> merged in v6.6-rc1, added this trait to overlayfs, thus allowing fanotify
> watching of overlayfs with FAN_REPORT_FID mode.
> 
> In retrospect, allowing an fanotify filesystem/mount mark on such
> filesystem in FAN_REPORT_FID mode will result in getting events with
> file handles, without the ability to resolve the filesystem objects from
> those file handles (i.e. no open_by_handle_at() support).
> 
> For v6.6, the safer option would be to allow this mode for inode marks
> only, where the caller has the opportunity to use name_to_handle_at() at
> the time of setting the mark. In the future we can revise this decision.
> 
> Fixes: a95aef69a740 ("fanotify: support reporting non-decodeable file handles")
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

OK, I agree sb/mount marks reporting FIDs are hardly usable without
name_to_handle_at() so better forbid them before someone comes up with some
creative abuse. I've queued the patch into my tree.

								Honza

> ---
>  fs/notify/fanotify/fanotify_user.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index f69c451018e3..537c70beaad0 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1585,16 +1585,25 @@ static int fanotify_test_fsid(struct dentry *dentry, __kernel_fsid_t *fsid)
>  }
>  
>  /* Check if filesystem can encode a unique fid */
> -static int fanotify_test_fid(struct dentry *dentry)
> +static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
>  {
> +	unsigned int mark_type = flags & FANOTIFY_MARK_TYPE_BITS;
> +	const struct export_operations *nop = dentry->d_sb->s_export_op;
> +
> +	/*
> +	 * We need to make sure that the filesystem supports encoding of
> +	 * file handles so user can use name_to_handle_at() to compare fids
> +	 * reported with events to the file handle of watched objects.
> +	 */
> +	if (!nop)
> +		return -EOPNOTSUPP;
> +
>  	/*
> -	 * We need to make sure that the file system supports at least
> -	 * encoding a file handle so user can use name_to_handle_at() to
> -	 * compare fid returned with event to the file handle of watched
> -	 * objects. However, even the relaxed AT_HANDLE_FID flag requires
> -	 * at least empty export_operations for ecoding unique file ids.
> +	 * For sb/mount mark, we also need to make sure that the filesystem
> +	 * supports decoding file handles, so user has a way to map back the
> +	 * reported fids to filesystem objects.
>  	 */
> -	if (!dentry->d_sb->s_export_op)
> +	if (mark_type != FAN_MARK_INODE && !nop->fh_to_dentry)
>  		return -EOPNOTSUPP;
>  
>  	return 0;
> @@ -1812,7 +1821,7 @@ static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
>  		if (ret)
>  			goto path_put_and_out;
>  
> -		ret = fanotify_test_fid(path.dentry);
> +		ret = fanotify_test_fid(path.dentry, flags);
>  		if (ret)
>  			goto path_put_and_out;
>  
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
index f69c451018e3..537c70beaad0 100644
--- a/fs/notify/fanotify/fanotify_user.c
+++ b/fs/notify/fanotify/fanotify_user.c
@@ -1585,16 +1585,25 @@  static int fanotify_test_fsid(struct dentry *dentry, __kernel_fsid_t *fsid)
 }
 
 /* Check if filesystem can encode a unique fid */
-static int fanotify_test_fid(struct dentry *dentry)
+static int fanotify_test_fid(struct dentry *dentry, unsigned int flags)
 {
+	unsigned int mark_type = flags & FANOTIFY_MARK_TYPE_BITS;
+	const struct export_operations *nop = dentry->d_sb->s_export_op;
+
+	/*
+	 * We need to make sure that the filesystem supports encoding of
+	 * file handles so user can use name_to_handle_at() to compare fids
+	 * reported with events to the file handle of watched objects.
+	 */
+	if (!nop)
+		return -EOPNOTSUPP;
+
 	/*
-	 * We need to make sure that the file system supports at least
-	 * encoding a file handle so user can use name_to_handle_at() to
-	 * compare fid returned with event to the file handle of watched
-	 * objects. However, even the relaxed AT_HANDLE_FID flag requires
-	 * at least empty export_operations for ecoding unique file ids.
+	 * For sb/mount mark, we also need to make sure that the filesystem
+	 * supports decoding file handles, so user has a way to map back the
+	 * reported fids to filesystem objects.
 	 */
-	if (!dentry->d_sb->s_export_op)
+	if (mark_type != FAN_MARK_INODE && !nop->fh_to_dentry)
 		return -EOPNOTSUPP;
 
 	return 0;
@@ -1812,7 +1821,7 @@  static int do_fanotify_mark(int fanotify_fd, unsigned int flags, __u64 mask,
 		if (ret)
 			goto path_put_and_out;
 
-		ret = fanotify_test_fid(path.dentry);
+		ret = fanotify_test_fid(path.dentry, flags);
 		if (ret)
 			goto path_put_and_out;