From patchwork Wed Apr 27 05:34:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Al Viro X-Patchwork-Id: 8952711 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 040EBBF29F for ; Wed, 27 Apr 2016 05:35:42 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 19F7C20225 for ; Wed, 27 Apr 2016 05:35:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id F28B620219 for ; Wed, 27 Apr 2016 05:35:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752352AbcD0FfE (ORCPT ); Wed, 27 Apr 2016 01:35:04 -0400 Received: from zeniv.linux.org.uk ([195.92.253.2]:36082 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751257AbcD0FfD (ORCPT ); Wed, 27 Apr 2016 01:35:03 -0400 Received: from viro by ZenIV.linux.org.uk with local (Exim 4.76 #1 (Red Hat Linux)) id 1avI7m-0007wK-5q; Wed, 27 Apr 2016 05:34:58 +0000 Date: Wed, 27 Apr 2016 06:34:58 +0100 From: Al Viro To: Linus Torvalds Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, Miklos Szeredi Subject: Re: [RFC] a corner case of open(2) Message-ID: <20160427053458.GA30149@ZenIV.linux.org.uk> References: <20160426175538.GO25498@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160426175538.GO25498@ZenIV.linux.org.uk> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Fun bugs caught while trying to massage atomic_open()... Patch below is in vfs.git#for-linus (along with two more fixes); I would like to get an ACK from Miklos on that one - it's his code and this thing had been present in there since the original merge. I might be misreading what it tries to do, but open("/mnt/no-such-file", O_CREAT | O_TRUNC); perror("open"); errno = 0; stat("/mnt/no-such-file", &st); perror("stat"); errno = 0; open("/mnt/no-such-file", O_CREAT | O_TRUNC); perror("open"); should *not* end up with open: Read-only file system stat: No such file or directory open: No such file or directory no matter what. And it's very easy to arrange just that - mount nfs4 read-only on /mnt and run the snippet above. First open() will fail with EROFS (as it should), but as soon as the thing is in dcache we start getting ENOENT. Obviously bogus. commit 1aa57f2aaa108ead7d81481af68085b0a77708f1 Author: Al Viro Date: Wed Apr 27 01:11:55 2016 -0400 atomic_open(): fix the handling of create_error * if we have a hashed negative dentry and either CREAT|EXCL on r/o filesystem, or CREAT|TRUNC on r/o filesystem, or CREAT|EXCL with failing may_o_create(), we should fail with EROFS or the error may_o_create() has returned, but not ENOENT. Which is what the current code ends up returning. * if we have CREAT|TRUNC hitting a regular file on a read-only filesystem, we can't fail with EROFS here. At the very least, not until we'd done follow_managed() - we might have a writable file (or a device, for that matter) bound on top of that one. Moreover, the code downstream will see that O_TRUNC and attempt to grab the write access (*after* following possible mount), so if we really should fail with EROFS, it will happen. No need to do that inside atomic_open(). The real logics is much simpler than what the current code is trying to do - if we decided to go for simple lookup, ended up with a negative dentry *and* had create_error set, fail with create_error. No matter whether we'd got that negative dentry from lookup_real() or had found it in dcache. Cc: stable@vger.kernel.org # v3.6+ Signed-off-by: Al Viro Acked-by: Miklos Szeredi --- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/namei.c b/fs/namei.c index 1d9ca2d..b458992 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2942,22 +2942,10 @@ no_open: dentry = lookup_real(dir, dentry, nd->flags); if (IS_ERR(dentry)) return PTR_ERR(dentry); - - if (create_error) { - int open_flag = op->open_flag; - - error = create_error; - if ((open_flag & O_EXCL)) { - if (!dentry->d_inode) - goto out; - } else if (!dentry->d_inode) { - goto out; - } else if ((open_flag & O_TRUNC) && - d_is_reg(dentry)) { - goto out; - } - /* will fail later, go on to get the right error */ - } + } + if (create_error && !dentry->d_inode) { + error = create_error; + goto out; } looked_up: path->dentry = dentry;