cifs_atomic_open(): fix double-put on late allocation failure

BugLink: https://bugs.launchpad.net/bugs/1869732
commit d9a9f4849fe0c9d560851ab22a85a666cddfdd24 upstream.

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>

diff --git a/Documentation/filesystems/porting b/Documentation/filesystems/porting
index 17bb4dc28fae03371c328cc2135b576c4235ab03..b6335c9517994330c07e5fb10a2c17c9fa70a361 100644 (file)
--- a/Documentation/filesystems/porting
+++ b/Documentation/filesystems/porting
@@ -602,3 +602,10 @@ in your dentry operations instead.
        dentry separately, and it now has request_mask and query_flags arguments
        to specify the fields and sync type requested by statx.  Filesystems not
        supporting any statx-specific features may ignore the new arguments.
+--
+[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.

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index f00a7ce3eb6e4fa86f3c24bfeef291f7fbf17828..03293e543c075a2f41911b7f679b91178035af9f 100644 (file)
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -562,7 +562,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;
        }
 

diff --git a/fs/open.c b/fs/open.c
index d5362b96cd6be826d1f310cccbf16b03fc371802..2cde49fbdf399f4d860b7227c3527afa24da73b1 100644 (file)
--- a/fs/open.c
+++ b/fs/open.c
@@ -829,9 +829,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,