Re: [PATCH v4 17/17] vfs: expose delegation support to userland

On Mon, 2025-11-03 at 07:52 -0500, Jeff Layton wrote:
> Now that support for recallable directory delegations is available,
> expose this functionality to userland with new F_SETDELEG and F_GETDELEG
> commands for fcntl().
> 
> Note that this also allows userland to request a FL_DELEG type lease on
> files too. Userland applications that do will get signalled when there
> are metadata changes in addition to just data changes (which is a
> limitation of FL_LEASE leases).
> 
> These commands accept a new "struct delegation" argument that contains a
> flags field for future expansion.
> 
> Signed-off-by: Jeff Layton <[email protected]>
> ---
>  fs/fcntl.c                 | 14 ++++++++++++++
>  fs/locks.c                 | 42 +++++++++++++++++++++++++++++++++++++-----
>  include/linux/filelock.h   | 12 ++++++++++++
>  include/uapi/linux/fcntl.h | 10 ++++++++++
>  4 files changed, 73 insertions(+), 5 deletions(-)
> 

[...]

> +
> +void fcntl_getdeleg(struct file *filp, struct delegation *deleg)
> +{
> +	deleg->d_type = __fcntl_getlease(filp, FL_DELEG);
> +}
> +


It occurs to me that this function should probably check d_flags and
return an error like fcntl_setlease() does. I'm testing this now in my
own tree. Returning an error on flags that the kernel doesn't
understand seems like the right thing to do. Thoughts?

--------------8<---------------

commit 92dcb7653003dc74edf29f07347d19271d629818
Author: Jeff Layton <[email protected]>
Date:   Mon Nov 3 07:59:19 2025 -0500

    SQUASH: make fcntl_getdeleg check d_flags too
    
    Signed-off-by: Jeff Layton <[email protected]>

diff --git a/fs/fcntl.c b/fs/fcntl.c
index 8d57a6e34076..f93dbca08435 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -554,10 +554,9 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
 	case F_GETDELEG:
 		if (copy_from_user(&deleg, argp, sizeof(deleg)))
 			return -EFAULT;
-		fcntl_getdeleg(filp, &deleg);
-		if (copy_to_user(argp, &deleg, sizeof(deleg)))
+		err = fcntl_getdeleg(filp, &deleg);
+		if (!err && copy_to_user(argp, &deleg, sizeof(deleg)))
 			return -EFAULT;
-		err = 0;
 		break;
 	case F_SETDELEG:
 		if (copy_from_user(&deleg, argp, sizeof(deleg)))
diff --git a/fs/locks.c b/fs/locks.c
index 1e29aecf79b8..c52f6a7b6a5c 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1736,9 +1736,12 @@ int fcntl_getlease(struct file *filp)
 	return __fcntl_getlease(filp, FL_LEASE);
 }
 
-void fcntl_getdeleg(struct file *filp, struct delegation *deleg)
+int fcntl_getdeleg(struct file *filp, struct delegation *deleg)
 {
+	if (deleg->d_flags != 0)
+		return -EINVAL;
 	deleg->d_type = __fcntl_getlease(filp, FL_DELEG);
+	return 0;
 }
 
 /**
diff --git a/include/linux/filelock.h b/include/linux/filelock.h
index 4384c6f61fad..54b824c05299 100644
--- a/include/linux/filelock.h
+++ b/include/linux/filelock.h
@@ -160,7 +160,7 @@ int fcntl_setlk64(unsigned int, struct file *, unsigned int,
 int fcntl_setlease(unsigned int fd, struct file *filp, int arg);
 int fcntl_getlease(struct file *filp);
 int fcntl_setdeleg(unsigned int fd, struct file *filp, struct delegation *deleg);
-void fcntl_getdeleg(struct file *filp, struct delegation *deleg);
+int fcntl_getdeleg(struct file *filp, struct delegation *deleg);
 
 static inline bool lock_is_unlock(struct file_lock *fl)
 {
@@ -287,7 +287,7 @@ static inline int fcntl_setdeleg(unsigned int fd, struct file *filp, struct dele
 
 static inline int fcntl_getdeleg(struct file *filp, struct delegation *deleg)
 {
-	return F_UNLCK;
+	return -EINVAL;
 }
 
 static inline bool lock_is_unlock(struct file_lock *fl)