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 <v...@zeniv.linux.org.uk>
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: sta...@vger.kernel.org # v3.6+
Signed-off-by: Al Viro <v...@zeniv.linux.org.uk>
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;
