| Message ID | 1499431731-27058-1-git-send-email-mpe@ellerman.id.au (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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>
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; \
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(-)