lkml.org 
[lkml]   [2020]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.5 096/151] cifs_atomic_open(): fix double-put on late allocation failure
    Date
    From: Al Viro <viro@zeniv.linux.org.uk>

    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>

    ---
    Documentation/filesystems/porting.rst | 8 ++++++++
    fs/cifs/dir.c | 1 -
    fs/open.c | 3 ---
    3 files changed, 8 insertions(+), 4 deletions(-)

    --- a/Documentation/filesystems/porting.rst
    +++ b/Documentation/filesystems/porting.rst
    @@ -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.
    --- a/fs/cifs/dir.c
    +++ b/fs/cifs/dir.c
    @@ -558,7 +558,6 @@ cifs_atomic_open(struct inode *inode, st
    if (server->ops->close)
    server->ops->close(xid, tcon, &fid);
    cifs_del_pending_open(&open);
    - fput(file);
    rc = -ENOMEM;
    }

    --- 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,

    \
     
     \ /
      Last update: 2020-03-17 12:16    [W:4.199 / U:0.300 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site