[PATCH] fs/fcntl: Fix F_GET/SETLK etc. for compat processes

[PATCH] fs/fcntl: Fix F_GET/SETLK etc. for compat processes

[Michael Ellerman]

Commit 8c6657cb50cb ("Switch flock copyin/copyout primitives to
copy_{from,to}_user()") added copy_flock_fields(from, to), but then in all cases
called it with arguments of (to, from). eg:

  static int get_compat_flock(struct flock *kfl, struct compat_flock __user *ufl)
  {
  	struct compat_flock fl;

  	if (copy_from_user(&fl, ufl, sizeof(struct compat_flock)))
  		return -EFAULT;
  	copy_flock_fields(*kfl, fl);
  	return 0;
  }

We are reading the compat_flock ufl from userspace, into flock kfl. First we
copy all of ufl into fl on the stack, and then we want to assign each field of
fl to kfl. So we are copying from fl and to kfl. But as written the
copy_flock_fields() macro takes the arguments in the other order.

copy_to/from_user() take "to" as the first argument, so change the order of
arguments in the copy_flock_fields() macro, rather than changing the callers.

Fixes: 8c6657cb50cb ("Switch flock copyin/copyout primitives to copy_{from,to}_user()")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 fs/fcntl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fcntl.c b/fs/fcntl.c
index b6bd89628025..f40e3a9c10a5 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -520,7 +520,7 @@ SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
 
 #ifdef CONFIG_COMPAT
 /* careful - don't use anywhere else */
-#define copy_flock_fields(from, to)		\
+#define copy_flock_fields(to, from)		\
 	(to).l_type = (from).l_type;		\
 	(to).l_whence = (from).l_whence;	\
 	(to).l_start = (from).l_start;		\
-- 
2.7.4

Re: [PATCH] fs/fcntl: Fix F_GET/SETLK etc. for compat processes

[Al Viro]

On Fri, Jul 07, 2017 at 10:48:51PM +1000, Michael Ellerman wrote:
> Commit 8c6657cb50cb ("Switch flock copyin/copyout primitives to
> copy_{from,to}_user()") added copy_flock_fields(from, to), but then in all cases
> called it with arguments of (to, from). eg:
> 
>   static int get_compat_flock(struct flock *kfl, struct compat_flock __user *ufl)
>   {
>   	struct compat_flock fl;
> 
>   	if (copy_from_user(&fl, ufl, sizeof(struct compat_flock)))
>   		return -EFAULT;
>   	copy_flock_fields(*kfl, fl);
>   	return 0;
>   }
> 
> We are reading the compat_flock ufl from userspace, into flock kfl. First we
> copy all of ufl into fl on the stack, and then we want to assign each field of
> fl to kfl. So we are copying from fl and to kfl. But as written the
> copy_flock_fields() macro takes the arguments in the other order.
> 
> copy_to/from_user() take "to" as the first argument, so change the order of
> arguments in the copy_flock_fields() macro, rather than changing the callers.

D'oh...

Acked-by: Al Viro <viro@zeniv.linux.org.uk>