Skip to content

Commit b94e0b3

Browse files
author
Al Viro
committed
open_last_lookups(): lift O_EXCL|O_CREAT handling into do_open()
Currently path_openat() has "EEXIST on O_EXCL|O_CREAT" checks done on one of the ways out of open_last_lookups(). There are 4 cases: 1) the last component is . or ..; check is not done. 2) we had FMODE_OPENED or FMODE_CREATED set while in lookup_open(); check is not done. 3) symlink to be traversed is found; check is not done (nor should it be) 4) everything else: check done (before complete_walk(), even). In case (1) O_EXCL|O_CREAT ends up failing with -EISDIR - that's open("/tmp/.", O_CREAT|O_EXCL, 0600) Note that in the same conditions open("/tmp", O_CREAT|O_EXCL, 0600) would have yielded EEXIST. Either error is allowed, switching to -EEXIST in these cases would've been more consistent. Case (2) is more subtle; first of all, if we have FMODE_CREATED set, the object hadn't existed prior to the call. The check should not be done in such a case. The rest is problematic, though - we have FMODE_OPENED set (i.e. it went through ->atomic_open() and got successfully opened there) FMODE_CREATED is *NOT* set O_CREAT and O_EXCL are both set. Any such case is a bug - either we failed to set FMODE_CREATED when we had, in fact, created an object (no such instances in the tree) or we have opened a pre-existing file despite having had both O_CREAT and O_EXCL passed. One of those was, in fact caught (and fixed) while sorting out this mess (gfs2 on cold dcache). And in such situations we should fail with EEXIST. Note that for (1) and (4) FMODE_CREATED is not set - for (1) there's nothing in handle_dots() to set it, for (4) we'd explicitly checked that. And (1), (2) and (4) are exactly the cases when we leave the loop in the caller, with do_open() called immediately after that loop. IOW, we can move the check over there, and make it If we have O_CREAT|O_EXCL and after successful pathname resolution FMODE_CREATED is *not* set, we must have run into a preexisting file and should fail with EEXIST. Signed-off-by: Al Viro <[email protected]>
1 parent 7228741 commit b94e0b3

File tree

1 file changed

+2
-5
lines changed

1 file changed

+2
-5
lines changed

fs/namei.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3201,11 +3201,6 @@ static const char *open_last_lookups(struct nameidata *nd,
32013201
return res;
32023202
}
32033203

3204-
if (unlikely((open_flag & (O_EXCL | O_CREAT)) == (O_EXCL | O_CREAT))) {
3205-
audit_inode(nd->name, nd->path.dentry, 0);
3206-
return ERR_PTR(-EEXIST);
3207-
}
3208-
32093204
/* Why this, you ask? _Now_ we might have grown LOOKUP_JUMPED... */
32103205
return ERR_PTR(complete_walk(nd));
32113206
}
@@ -3224,6 +3219,8 @@ static int do_open(struct nameidata *nd,
32243219
if (!(file->f_mode & FMODE_CREATED))
32253220
audit_inode(nd->name, nd->path.dentry, 0);
32263221
if (open_flag & O_CREAT) {
3222+
if ((open_flag & O_EXCL) && !(file->f_mode & FMODE_CREATED))
3223+
return -EEXIST;
32273224
if (d_is_dir(nd->path.dentry))
32283225
return -EISDIR;
32293226
error = may_create_in_sticky(nd->dir_mode, nd->dir_uid,

0 commit comments

Comments
 (0)