From patchwork Tue Jan 17 00:04:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Michael Kerrisk (man-pages)" X-Patchwork-Id: 9519681 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 C8FEB6020B for ; Tue, 17 Jan 2017 00:05:05 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AEF3826E55 for ; Tue, 17 Jan 2017 00:05:05 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9FC3B28494; Tue, 17 Jan 2017 00:05:05 +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_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham 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 1B9AC26E55 for ; Tue, 17 Jan 2017 00:05:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751567AbdAQAFC (ORCPT ); Mon, 16 Jan 2017 19:05:02 -0500 Received: from mail-wm0-f68.google.com ([74.125.82.68]:36449 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751012AbdAQAFA (ORCPT ); Mon, 16 Jan 2017 19:05:00 -0500 Received: by mail-wm0-f68.google.com with SMTP id r126so34509681wmr.3; Mon, 16 Jan 2017 16:04:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:reply-to:in-reply-to:references:from:date:message-id :subject:to:cc; bh=HVi+AmMWg1GcowvlnK7pkwu8fkxbJwwCivfIn4Jkx3c=; b=TOr7WWiwFWEsDlZuvRCyAnR0R7grDBpgi2zGnUVyin2bfU1Oxq0tsC+0d90dyq0ub8 airiaI8yKmzQ7QftBHLwCe5p9Olyv+EGMqUemP5SFddb2PUA6fQFCRIunb2yY5QnoTU/ sf6gU0MazB98fpkfw3ekz2m3D27zKgVSMlUusr/4uf8oc/E5mwfey1ssScOl0LZqWQOf /nR0bu+sMzWzaMkKlnCnamQ/JPP0+fWoVB6FZ+ISQhAEDk2zR+9PA9Ug1KbHqBgU2yDR yivgXINt+rT9KtlZr19d3mIjt8f4+vdN5/OMPGwjDxz3Bylhbja2LdXsIYOP3dEXcfI6 lR2Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :from:date:message-id:subject:to:cc; bh=HVi+AmMWg1GcowvlnK7pkwu8fkxbJwwCivfIn4Jkx3c=; b=aeiYjtMqGqPS+pNnk9hWhgWYy0uhAwUofVisqqh/JzgPmk35uJNspB8yJB2botM0Se HFIOpPC53PtnuaYbvhPKnvt344g31yTnfFYi7gghnRtxlGDe9crvNwZgWpBGJcVVGbwt xDV8+RolSeRkEaCqitkpZMrI/jcT/Iem1HX5Ztoo3H6DIyTLl+cZTfBWxOHzX3HFwX15 c3mxxFqfwYxqCEPrGf8skRQHc/4MgjfBFlUMCm9mP3YfAHm12n+56YT/BAdXyGE0JdqR WVPBWuPvA6Ek3PGXmU8bM2jE3/7tgXyf3/N+JdCiQ1oCfhfbcF23lhea8Z8Rd5xOKM0b QJ8w== X-Gm-Message-State: AIkVDXImjKy3bwhbhCjrBWUIrs+Cwp/2tVWRGc7I8s4Kv8nBptTE3Lbk75c96CJKF1g/j3BTZlx4aZ3xgLZHYw== X-Received: by 10.28.191.79 with SMTP id p76mr13494747wmf.21.1484611498603; Mon, 16 Jan 2017 16:04:58 -0800 (PST) MIME-Version: 1.0 Received: by 10.28.216.138 with HTTP; Mon, 16 Jan 2017 16:04:38 -0800 (PST) Reply-To: mtk.manpages@gmail.com In-Reply-To: References: <18a5b416-ad6a-e679-d993-af7ffa0dcc10@redhat.com> From: "Michael Kerrisk (man-pages)" Date: Tue, 17 Jan 2017 13:04:38 +1300 Message-ID: Subject: Re: utimensat EACCES vs. EPERM in 4.8+ To: Miklos Szeredi Cc: Jan Stancek , linux-fsdevel , viro , guaneryu@gmail.com, Cyril Hrubis , ltp@lists.linux.it, Linux API , Dave Chinner 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 [CC += linux-api + Dave Chinner] On 17 January 2017 at 04:53, Miklos Szeredi wrote: > On Mon, Jan 16, 2017 at 4:46 PM, Jan Stancek wrote: >> Hi, >> >> we seem to have a conflict between kernel and man pages. Jan, thanks for spotting this. >> From utimensat man page: >> >> EACCES times is NULL, or both tv_nsec values are UTIME_NOW, and either: >> * the effective user ID of the caller does not match the owner of the >> file, the caller does not have write access to the file, and the >> caller is not privileged (Linux: does not have either the CAP_FOWNER >> or the CAP_DAC_OVERRIDE capability); or, >> * the file is marked immutable (see chattr(1)). >> >> But following 2 commits gradually replaced EACCES with EPERM. >> >> commit 337684a1746f93ae107e05d90977b070bb7e39d8 >> Author: Eryu Guan >> Date: Tue Aug 2 19:58:28 2016 +0800 >> fs: return EPERM on immutable inode > > I agree with Eryu that consistently returning EPERM for immutable is > better than sometimes returning EACCESS and sometimes EPERM. I'm not so sure about that. In Eryu's patch (which *really, really* should have CCed linux-api@, and it would be kind if subsystem maintainers reminded patch submitters about that), there was this change: [[ retval = do_inode_permission(inode, mask); ]] [1] The effects of that change are pretty wide ranging, affecting open(2)/openat(2) (of an existing file for writing), access(2)/faccessat(2) (W_OK), and [f]truncate(2). In addition, there is the observed change (from another part of the patch) in utimensat(2) (and friends). Those cases formerly gave EACCES for immutable files, now they give EPERM. [2] By contrast, the following always gave EPERM: fallocate(2), setxattr(2), unlink(2), link(2) [in certain cases], chown(2), chmod(2), and some per-filesystem cases of operations such as truncate. > So I think the man page should be fixed. I agree that the inconsistency in the error return for immutable files is unfortunate. But, consider the following: * Although the set of calls in [1] is shorter, they (in particular, open(2)) are probably much more commonly used than the system calls in [2]. (That is, Eryu's statement "In most cases, EPERM is returned on immutable inode" that accompanied the kernel patch isn't correct.) * For access(W_OK), we introduced a new error (EPERM) that previously never previously occurred. If there are applications that use access() and check specific error returns, they'll be confused. (I acknowledge there may be few such applications.) * We changed the carefully documented behavior of utimensat(2) (and friends). [Read the man page!] * EACCES is the typical error for "file not writable" (because of file permissions or other reasons such as immutability). It's long been the behavior for open(O_WRONLY/O_RDWR) on immutable files; now that has changed. * Now various man pages need to document two different (kernel version dependent) errors for immutable files (for the syscalls in [1], above), and applications may need to deal with those two errors. Summary of the above list: there's a nontrivial risk that something in userspace got broken. (And just because we didn't hear about it yet doesn't mean it didn't happen; sometimes these reports only arrive many months or even years later.) So, (1) I'm struggling to see the rationale for this change (I don't think "consistency" is enough) and (2) if "consistency" is the argument then (because the set of system calls in [1] are more frequently used than those in [2]), there's a reasonable argument that the change should have gone the other way: changing all IS_IMMUTABLE cases to fail with EACCES. Summary: I think there's an argument for reverting the kernel patch. Cheers, Michael --- a/fs/namei.c +++ b/fs/namei.c @@ -402,23 +402,23 @@ static inline int do_inode_permission(struct inode *inode, int mask) * inode_permission(). */ int __inode_permission(struct inode *inode, int mask) { int retval; if (unlikely(mask & MAY_WRITE)) { /* * Nobody gets write access to an immutable file. */ if (IS_IMMUTABLE(inode)) - return -EACCES; + return -EPERM; /* * Updating mtime will likely cause i_uid and i_gid to be * written back improperly if their true value is unknown * to the vfs. */ if (HAS_UNMAPPED_ID(inode)) return -EACCES; }