Skip to content

Commit af04fad

Browse files
Al Virotorvalds
authored andcommitted
Revert "fs: fold open_check_o_direct into do_dentry_open"
This reverts commit cab64df. Having vfs_open() in some cases drop the reference to struct file combined with error = vfs_open(path, f, cred); if (error) { put_filp(f); return ERR_PTR(error); } return f; is flat-out wrong. It used to be error = vfs_open(path, f, cred); if (!error) { /* from now on we need fput() to dispose of f */ error = open_check_o_direct(f); if (error) { fput(f); f = ERR_PTR(error); } } else { put_filp(f); f = ERR_PTR(error); } and sure, having that open_check_o_direct() boilerplate gotten rid of is nice, but not that way... Worse, another call chain (via finish_open()) is FUBAR now wrt FILE_OPENED handling - in that case we get error returned, with file already hit by fput() *AND* FILE_OPENED not set. Guess what happens in path_openat(), when it hits if (!(opened & FILE_OPENED)) { BUG_ON(!error); put_filp(file); } The root cause of all that crap is that the callers of do_dentry_open() have no way to tell which way did it fail; while that could be fixed up (by passing something like int *opened to do_dentry_open() and have it marked if we'd called ->open()), it's probably much too late in the cycle to do so right now. Signed-off-by: Al Viro <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 4faa999 commit af04fad

File tree

3 files changed

+33
-19
lines changed

3 files changed

+33
-19
lines changed

fs/internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ int do_fchmodat(int dfd, const char __user *filename, umode_t mode);
125125
int do_fchownat(int dfd, const char __user *filename, uid_t user, gid_t group,
126126
int flag);
127127

128+
extern int open_check_o_direct(struct file *f);
128129
extern int vfs_open(const struct path *, struct file *, const struct cred *);
129130
extern struct file *filp_clone_open(struct file *);
130131

fs/namei.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3367,7 +3367,9 @@ static int do_last(struct nameidata *nd,
33673367
goto out;
33683368
*opened |= FILE_OPENED;
33693369
opened:
3370-
error = ima_file_check(file, op->acc_mode, *opened);
3370+
error = open_check_o_direct(file);
3371+
if (!error)
3372+
error = ima_file_check(file, op->acc_mode, *opened);
33713373
if (!error && will_truncate)
33723374
error = handle_truncate(file);
33733375
out:
@@ -3447,6 +3449,9 @@ static int do_tmpfile(struct nameidata *nd, unsigned flags,
34473449
error = finish_open(file, child, NULL, opened);
34483450
if (error)
34493451
goto out2;
3452+
error = open_check_o_direct(file);
3453+
if (error)
3454+
fput(file);
34503455
out2:
34513456
mnt_drop_write(path.mnt);
34523457
out:

fs/open.c

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,16 @@ SYSCALL_DEFINE3(fchown, unsigned int, fd, uid_t, user, gid_t, group)
724724
return ksys_fchown(fd, user, group);
725725
}
726726

727+
int open_check_o_direct(struct file *f)
728+
{
729+
/* NB: we're sure to have correct a_ops only after f_op->open */
730+
if (f->f_flags & O_DIRECT) {
731+
if (!f->f_mapping->a_ops || !f->f_mapping->a_ops->direct_IO)
732+
return -EINVAL;
733+
}
734+
return 0;
735+
}
736+
727737
static int do_dentry_open(struct file *f,
728738
struct inode *inode,
729739
int (*open)(struct inode *, struct file *),
@@ -745,7 +755,7 @@ static int do_dentry_open(struct file *f,
745755
if (unlikely(f->f_flags & O_PATH)) {
746756
f->f_mode = FMODE_PATH;
747757
f->f_op = &empty_fops;
748-
goto done;
758+
return 0;
749759
}
750760

751761
if (f->f_mode & FMODE_WRITE && !special_file(inode->i_mode)) {
@@ -798,12 +808,7 @@ static int do_dentry_open(struct file *f,
798808
f->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
799809

800810
file_ra_state_init(&f->f_ra, f->f_mapping->host->i_mapping);
801-
done:
802-
/* NB: we're sure to have correct a_ops only after f_op->open */
803-
error = -EINVAL;
804-
if ((f->f_flags & O_DIRECT) &&
805-
(!f->f_mapping->a_ops || !f->f_mapping->a_ops->direct_IO))
806-
goto out_fput;
811+
807812
return 0;
808813

809814
cleanup_all:
@@ -818,9 +823,6 @@ static int do_dentry_open(struct file *f,
818823
f->f_path.dentry = NULL;
819824
f->f_inode = NULL;
820825
return error;
821-
out_fput:
822-
fput(f);
823-
return error;
824826
}
825827

826828
/**
@@ -918,14 +920,20 @@ struct file *dentry_open(const struct path *path, int flags,
918920
BUG_ON(!path->mnt);
919921

920922
f = get_empty_filp();
921-
if (IS_ERR(f))
922-
return f;
923-
924-
f->f_flags = flags;
925-
error = vfs_open(path, f, cred);
926-
if (error) {
927-
put_filp(f);
928-
return ERR_PTR(error);
923+
if (!IS_ERR(f)) {
924+
f->f_flags = flags;
925+
error = vfs_open(path, f, cred);
926+
if (!error) {
927+
/* from now on we need fput() to dispose of f */
928+
error = open_check_o_direct(f);
929+
if (error) {
930+
fput(f);
931+
f = ERR_PTR(error);
932+
}
933+
} else {
934+
put_filp(f);
935+
f = ERR_PTR(error);
936+
}
929937
}
930938
return f;
931939
}

0 commit comments

Comments
 (0)