cifs_atomic_open(): fix double-put on late allocation failure
authorAl Viro <viro@zeniv.linux.org.uk>
Thu, 12 Mar 2020 22:25:20 +0000 (18:25 -0400)
committerAl Viro <viro@zeniv.linux.org.uk>
Thu, 12 Mar 2020 22:25:20 +0000 (18:25 -0400)
several iterations of ->atomic_open() calling conventions ago, we
used to need fput() if ->atomic_open() failed at some point after
successful finish_open().  Now (since 2016) it's not needed -
struct file carries enough state to make fput() work regardless
of the point in struct file lifecycle and discarding it on
failure exits in open() got unified.  Unfortunately, I'd missed
the fact that we had an instance of ->atomic_open() (cifs one)
that used to need that fput(), as well as the stale comment in
finish_open() demanding such late failure handling.  Trivially
fixed...

Fixes: fe9ec8291fca "do_last(): take fput() on error after opening to out:"
Cc: stable@kernel.org # v4.7+
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Documentation/filesystems/porting.rst
fs/cifs/dir.c
fs/open.c

index f18506083ced831cf0ceef4870d964144152cd99..26c093969573639fc815803aa275d609057c7bfc 100644 (file)
@@ -850,3 +850,11 @@ business doing so.
 d_alloc_pseudo() is internal-only; uses outside of alloc_file_pseudo() are
 very suspect (and won't work in modules).  Such uses are very likely to
 be misspelled d_alloc_anon().
+
+---
+
+**mandatory**
+
+[should've been added in 2016] stale comment in finish_open() nonwithstanding,
+failure exits in ->atomic_open() instances should *NOT* fput() the file,
+no matter what.  Everything is handled by the caller.
index 0ef099442f209c779a1b2e3588cbaade5cf173c3..36e7b2fd2190b32462eba9c65c424408a15d0fe6 100644 (file)
@@ -555,7 +555,6 @@ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
                if (server->ops->close)
                        server->ops->close(xid, tcon, &fid);
                cifs_del_pending_open(&open);
-               fput(file);
                rc = -ENOMEM;
        }
 
index 0788b3715731186f326a24296ed3b00ad870bb46..b69d6eed67e6a93753cce13080ebb2208dee3a0b 100644 (file)
--- a/fs/open.c
+++ b/fs/open.c
@@ -860,9 +860,6 @@ cleanup_file:
  * the return value of d_splice_alias(), then the caller needs to perform dput()
  * on it after finish_open().
  *
- * On successful return @file is a fully instantiated open file.  After this, if
- * an error occurs in ->atomic_open(), it needs to clean up with fput().
- *
  * Returns zero on success or -errno if the open failed.
  */
 int finish_open(struct file *file, struct dentry *dentry,