fs/smb: Fix inconsistent refcnt update

commit ab529e6ca1f67bcf31f3ea80c72bffde2e9e053e upstream.

A possible inconsistent update of refcount was identified in `smb2_compound_op`.
Such inconsistent update could lead to possible resource leaks.

Why it is a possible bug:
1. In the comment section of the function, it clearly states that the
reference to `cfile` should be dropped after calling this function.
2. Every control flow path would check and drop the reference to
`cfile`, except the patched one.
3. Existing callers would not handle refcount update of `cfile` if
-ENOMEM is returned.

To fix the bug, an extra goto label "out" is added, to make sure that the
cleanup logic would always be respected. As the problem is caused by the
allocation failure of `vars`, the cleanup logic between label "finished"
and "out" can be safely ignored. According to the definition of function
`is_replayable_error`, the error code of "-ENOMEM" is not recoverable.
Therefore, the replay logic also gets ignored.

Signed-off-by: Shuhao Fu <sfual@cse.ust.hk>
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 6048b3fed3e787065628f149b9702fef14b4db70..b51ccfb88439416f3a3ffbe8a71480ed5ef64159 100644 (file)
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -206,8 +206,10 @@ replay_again:
        server = cifs_pick_channel(ses);
 
        vars = kzalloc(sizeof(*vars), GFP_ATOMIC);
-       if (vars == NULL)
-               return -ENOMEM;
+       if (vars == NULL) {
+               rc = -ENOMEM;
+               goto out;
+       }
        rqst = &vars->rqst[0];
        rsp_iov = &vars->rsp_iov[0];
 
@@ -832,6 +834,7 @@ finished:
            smb2_should_replay(tcon, &retries, &cur_sleep))
                goto replay_again;
 
+out:
        if (cfile)
                cifsFileInfo_put(cfile);