All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Aurich <paul@darkrain42.org>
To: linux-cifs@vger.kernel.org, sfrench@samba.org
Cc: paul@darkrain42.org, "Ronnie Sahlberg" <lsahlber@redhat.com>,
	"Aurélien Aptel" <aaptel@suse.com>
Subject: [PATCH v2] cifs: Fix leak when handling lease break for cached root fid
Date: Mon, 29 Jun 2020 19:30:03 -0700	[thread overview]
Message-ID: <20200630023003.1858066-1-paul@darkrain42.org> (raw)

Handling a lease break for the cached root didn't free the
smb2_lease_break_work allocation, resulting in a leak:

    unreferenced object 0xffff98383a5af480 (size 128):
      comm "cifsd", pid 684, jiffies 4294936606 (age 534.868s)
      hex dump (first 32 bytes):
        c0 ff ff ff 1f 00 00 00 88 f4 5a 3a 38 98 ff ff  ..........Z:8...
        88 f4 5a 3a 38 98 ff ff 80 88 d6 8a ff ff ff ff  ..Z:8...........
      backtrace:
        [<0000000068957336>] smb2_is_valid_oplock_break+0x1fa/0x8c0
        [<0000000073b70b9e>] cifs_demultiplex_thread+0x73d/0xcc0
        [<00000000905fa372>] kthread+0x11c/0x150
        [<0000000079378e4e>] ret_from_fork+0x22/0x30

Avoid this leak by only allocating when necessary.

Fixes: a93864d93977 ("cifs: add lease tracking to the cached root fid")
Signed-off-by: Paul Aurich <paul@darkrain42.org>
CC: Stable <stable@vger.kernel.org> # v4.18+
---
 fs/cifs/smb2misc.c | 47 ++++++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
index 6a39451973f8..570c0521fc3c 100644
--- a/fs/cifs/smb2misc.c
+++ b/fs/cifs/smb2misc.c
@@ -505,8 +505,7 @@ cifs_ses_oplock_break(struct work_struct *work)
 }
 
 static bool
-smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp,
-		    struct smb2_lease_break_work *lw)
+smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp)
 {
 	bool found;
 	__u8 lease_state;
@@ -516,9 +515,13 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp,
 	struct cifsInodeInfo *cinode;
 	int ack_req = le32_to_cpu(rsp->Flags &
 				  SMB2_NOTIFY_BREAK_LEASE_FLAG_ACK_REQUIRED);
+	struct smb2_lease_break_work *lw;
+	struct tcon_link *tlink;
+	__u8 lease_key[SMB2_LEASE_KEY_SIZE];
 
 	lease_state = le32_to_cpu(rsp->NewLeaseState);
 
+	spin_lock(&tcon->open_file_lock);
 	list_for_each(tmp, &tcon->openFileList) {
 		cfile = list_entry(tmp, struct cifsFileInfo, tlist);
 		cinode = CIFS_I(d_inode(cfile->dentry));
@@ -542,7 +545,8 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp,
 		cfile->oplock_level = lease_state;
 
 		cifs_queue_oplock_break(cfile);
-		kfree(lw);
+		spin_unlock(&tcon->open_file_lock);
+		spin_unlock(&cifs_tcp_ses_lock);
 		return true;
 	}
 
@@ -554,10 +558,9 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp,
 
 		if (!found && ack_req) {
 			found = true;
-			memcpy(lw->lease_key, open->lease_key,
+			memcpy(lease_key, open->lease_key,
 			       SMB2_LEASE_KEY_SIZE);
-			lw->tlink = cifs_get_tlink(open->tlink);
-			queue_work(cifsiod_wq, &lw->lease_break);
+			tlink = cifs_get_tlink(open->tlink);
 		}
 
 		cifs_dbg(FYI, "found in the pending open list\n");
@@ -567,6 +570,23 @@ smb2_tcon_has_lease(struct cifs_tcon *tcon, struct smb2_lease_break *rsp,
 		open->oplock = lease_state;
 	}
 
+	spin_unlock(&tcon->open_file_lock);
+	if (found) {
+		spin_unlock(&cifs_tcp_ses_lock);
+
+		lw = kmalloc(sizeof(struct smb2_lease_break_work), GFP_KERNEL);
+		if (!lw) {
+			cifs_put_tlink(tlink);
+			return true;
+		}
+
+		INIT_WORK(&lw->lease_break, cifs_ses_oplock_break);
+		lw->tlink = tlink;
+		lw->lease_state = rsp->NewLeaseState;
+		memcpy(lw->lease_key, lease_key, SMB2_LEASE_KEY_SIZE);
+		queue_work(cifsiod_wq, &lw->lease_break);
+	}
+
 	return found;
 }
 
@@ -578,14 +598,6 @@ smb2_is_valid_lease_break(char *buffer)
 	struct TCP_Server_Info *server;
 	struct cifs_ses *ses;
 	struct cifs_tcon *tcon;
-	struct smb2_lease_break_work *lw;
-
-	lw = kmalloc(sizeof(struct smb2_lease_break_work), GFP_KERNEL);
-	if (!lw)
-		return false;
-
-	INIT_WORK(&lw->lease_break, cifs_ses_oplock_break);
-	lw->lease_state = rsp->NewLeaseState;
 
 	cifs_dbg(FYI, "Checking for lease break\n");
 
@@ -600,15 +612,11 @@ smb2_is_valid_lease_break(char *buffer)
 			list_for_each(tmp2, &ses->tcon_list) {
 				tcon = list_entry(tmp2, struct cifs_tcon,
 						  tcon_list);
-				spin_lock(&tcon->open_file_lock);
 				cifs_stats_inc(
 				    &tcon->stats.cifs_stats.num_oplock_brks);
-				if (smb2_tcon_has_lease(tcon, rsp, lw)) {
-					spin_unlock(&tcon->open_file_lock);
-					spin_unlock(&cifs_tcp_ses_lock);
+				if (smb2_tcon_has_lease(tcon, rsp)) {
 					return true;
 				}
-				spin_unlock(&tcon->open_file_lock);
 
 				if (tcon->crfid.is_valid &&
 				    !memcmp(rsp->LeaseKey,
@@ -625,7 +633,6 @@ smb2_is_valid_lease_break(char *buffer)
 		}
 	}
 	spin_unlock(&cifs_tcp_ses_lock);
-	kfree(lw);
 	cifs_dbg(FYI, "Can not process lease break - no lease matched\n");
 	return false;
 }
-- 
2.27.0


             reply	other threads:[~2020-06-30  2:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-30  2:30 Paul Aurich [this message]
2020-07-01 13:13 ` [PATCH v2] cifs: Fix leak when handling lease break for cached root fid Aurélien Aptel
2020-07-02  0:53 ` Steve French
2020-06-30  7:34 kernel test robot
2020-06-30 10:04 kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200630023003.1858066-1-paul@darkrain42.org \
    --to=paul@darkrain42.org \
    --cc=aaptel@suse.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=lsahlber@redhat.com \
    --cc=sfrench@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.