From patchwork Fri Jul 7 17:35:41 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Linus Torvalds X-Patchwork-Id: 9830845 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2146F60352 for ; Fri, 7 Jul 2017 17:36:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A8CA52868D for ; Fri, 7 Jul 2017 17:35:58 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9C764286E6; Fri, 7 Jul 2017 17:35:58 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4C3842868D for ; Fri, 7 Jul 2017 17:35:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750938AbdGGRfq (ORCPT ); Fri, 7 Jul 2017 13:35:46 -0400 Received: from mail-oi0-f67.google.com ([209.85.218.67]:34948 "EHLO mail-oi0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750726AbdGGRfn (ORCPT ); Fri, 7 Jul 2017 13:35:43 -0400 Received: by mail-oi0-f67.google.com with SMTP id l130so4674580oib.2; Fri, 07 Jul 2017 10:35:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=NCgoo45QmY1Q3M0KKwDVC1GnG/w4JoYzF45AWi7Nx7k=; b=j8B/dPqi37mPTf9xJatJ53+jeMVWYq0rvV1XE7LLtFd9eEotQuyMztVjTnAyzyQDM6 G/lQcjkj6j3eXDLPTyrajPfXji6GycWN7ocOKK6I1cadGFsKB9NNh7XGsK1zh+XP30mD lqBz9DGBDoI/InbUeNuwb8vS5HrXvivC2gv0jvoR1ffKPMSt8+GeRXYqXvc63KR0lOaU tf6Pwx0x0J2fxm5tkdrpEa4llZksO/SMAKcyfme0JKB+XiDwg9XCZLaO4mIafEWaC5QV GVNdpz2fcyS9jUBAfUY4ZJLq4ABTegaahla1tTJcZhxhcpsSAKHPXycW0zYI9XnJVX5V JyPg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=NCgoo45QmY1Q3M0KKwDVC1GnG/w4JoYzF45AWi7Nx7k=; b=o1r6LkSOTG8G7Y57JxDi/yHR/rZjv4U8q0y2LXJhRlJ9p7T82olJOE1BhISnLYfUA/ l3+OyCU8bI4BJtYUMN4/HYgwRerQHc0Sk3w/Weg5oUAfpb9dmcJBMp0Nsc1phHElx9Ko er6R8cuqOC309dgcUK92xWjH0dVbtpQ2I+kyv15honvE4LH5/qz5GQogKNeGlJ4QFuRh 2Gnvr1WCF/ySRw9ULwrJyrjRn8WtuCQGOt4yvFhL0XVHt9uHaUJHdDEcecMRlvym2Ody ODGh7XzG7u8pUru+c7SpDUDHH6Aku9AG5ussgzCjL8jRV05wp5oQkh7L1VyItUSDHGBs x3yQ== X-Gm-Message-State: AIVw111VovB8vBbdCRqzMHs5FH6SzZ8/ckufqGLFCKMcmGyynzjqHoR6 B85R4T28MzA8XYQ+hSH8gyZZZNdb6ScLpfs= X-Received: by 10.202.0.65 with SMTP id 62mr1354580oia.86.1499448942592; Fri, 07 Jul 2017 10:35:42 -0700 (PDT) MIME-Version: 1.0 Received: by 10.182.162.10 with HTTP; Fri, 7 Jul 2017 10:35:41 -0700 (PDT) In-Reply-To: References: <20170705071423.GY10672@ZenIV.linux.org.uk> <87eftsv2hj.fsf@concordia.ellerman.id.au> From: Linus Torvalds Date: Fri, 7 Jul 2017 10:35:41 -0700 X-Google-Sender-Auth: eCHFHL_fZwQ2VN7CTLycLP139fA Message-ID: Subject: Re: [git pull] vfs.git part 1 To: Michael Ellerman Cc: Al Viro , Linux Kernel Mailing List , linux-fsdevel , "linuxppc-dev@lists.ozlabs.org" Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Jul 7, 2017 at 8:59 AM, Linus Torvalds wrote: > > The copy_flock_fields() macro has the arguments in order , > but all the users seem to do it the other way around. Looking more at it, I think I'd also like copy_flock_fields() to take pointer arguments, to match all the code around it (both copy_to/from_user and the memset calls. The actual order of arguments I suspect Michael's patch did better - make the copy_flock_fields() just match the order of memcpy() and copy_to/from_user(), both of which have order. So I think my preferred patch would be something like this, even if it is bigger than either. Comments? Michael, does this work for your case? Linus fs/fcntl.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/fs/fcntl.c b/fs/fcntl.c index b6bd89628025..3b01b646e528 100644 --- a/fs/fcntl.c +++ b/fs/fcntl.c @@ -520,50 +520,50 @@ SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd, #ifdef CONFIG_COMPAT /* careful - don't use anywhere else */ -#define copy_flock_fields(from, to) \ - (to).l_type = (from).l_type; \ - (to).l_whence = (from).l_whence; \ - (to).l_start = (from).l_start; \ - (to).l_len = (from).l_len; \ - (to).l_pid = (from).l_pid; - -static int get_compat_flock(struct flock *kfl, struct compat_flock __user *ufl) +#define copy_flock_fields(dst, src) \ + (dst)->l_type = (src)->l_type; \ + (dst)->l_whence = (src)->l_whence; \ + (dst)->l_start = (src)->l_start; \ + (dst)->l_len = (src)->l_len; \ + (dst)->l_pid = (src)->l_pid; + +static int get_compat_flock(struct flock *kfl, const 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); + copy_flock_fields(kfl, &fl); return 0; } -static int get_compat_flock64(struct flock *kfl, struct compat_flock64 __user *ufl) +static int get_compat_flock64(struct flock *kfl, const struct compat_flock64 __user *ufl) { struct compat_flock64 fl; if (copy_from_user(&fl, ufl, sizeof(struct compat_flock64))) return -EFAULT; - copy_flock_fields(*kfl, fl); + copy_flock_fields(kfl, &fl); return 0; } -static int put_compat_flock(struct flock *kfl, struct compat_flock __user *ufl) +static int put_compat_flock(const struct flock *kfl, struct compat_flock __user *ufl) { struct compat_flock fl; memset(&fl, 0, sizeof(struct compat_flock)); - copy_flock_fields(fl, *kfl); + copy_flock_fields(&fl, kfl); if (copy_to_user(ufl, &fl, sizeof(struct compat_flock))) return -EFAULT; return 0; } -static int put_compat_flock64(struct flock *kfl, struct compat_flock64 __user *ufl) +static int put_compat_flock64(const struct flock *kfl, struct compat_flock64 __user *ufl) { struct compat_flock64 fl; memset(&fl, 0, sizeof(struct compat_flock64)); - copy_flock_fields(fl, *kfl); + copy_flock_fields(&fl, kfl); if (copy_to_user(ufl, &fl, sizeof(struct compat_flock64))) return -EFAULT; return 0;