cifs: Fix use after free of a mid_q_entry

commit 696e420bb2a6624478105651d5368d45b502b324 upstream.

With protocol version 2.0 mounts we have seen crashes with corrupt mid
entries. Either the server->pending_mid_q list becomes corrupt with a
cyclic reference in one element or a mid object fetched by the
demultiplexer thread becomes overwritten during use.

Code review identified a race between the demultiplexer thread and the
request issuing thread. The demultiplexer thread seems to be written
with the assumption that it is the sole user of the mid object until
it calls the mid callback which either wakes the issuer task or
deletes the mid.

This assumption is not true because the issuer task can be woken up
earlier by a signal. If the demultiplexer thread has proceeded as far
as setting the mid_state to MID_RESPONSE_RECEIVED then the issuer
thread will happily end up calling cifs_delete_mid while the
demultiplexer thread still is using the mid object.

Inserting a delay in the cifs demultiplexer thread widens the race
window and makes reproduction of the race very easy:

		if (server->large_buf)
			buf = server->bigbuf;

+		usleep_range(500, 4000);

		server->lstrp = jiffies;

To resolve this I think the proper solution involves putting a
reference count on the mid object. This patch makes sure that the
demultiplexer thread holds a reference until it has finished
processing the transaction.

Cc: stable@vger.kernel.org
Signed-off-by: Lars Persson <larper@axis.com>
Acked-by: Paulo Alcantara <palcantara@suse.de>
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index cb950a5fa0782e9d3446a8a0bc3a8fdd642bc57e..c7ee09d9a236225a66afe6ed4b130b0ee8905af2 100644 (file)
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1362,6 +1362,7 @@ typedef int (mid_handle_t)(struct TCP_Server_Info *server,
 /* one of these for every pending CIFS request to the server */
 struct mid_q_entry {
        struct list_head qhead; /* mids waiting on reply from this server */
+       struct kref refcount;
        struct TCP_Server_Info *server; /* server corresponding to this mid */
        __u64 mid;              /* multiplex id */
        __u32 pid;              /* process id */

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 365a414a75e9d145da7786fee5f501b229501c31..c4e5c69810f98a1e891a49c2d83f8201b19fba14 100644 (file)
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -76,6 +76,7 @@ extern struct mid_q_entry *AllocMidQEntry(const struct smb_hdr *smb_buffer,
                                        struct TCP_Server_Info *server);
 extern void DeleteMidQEntry(struct mid_q_entry *midEntry);
 extern void cifs_delete_mid(struct mid_q_entry *mid);
+extern void cifs_mid_q_entry_release(struct mid_q_entry *midEntry);
 extern void cifs_wake_up_task(struct mid_q_entry *mid);
 extern int cifs_handle_standard(struct TCP_Server_Info *server,
                                struct mid_q_entry *mid);

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 7a10a5d0731f078247d5636d3e00aae169b48345..5e1c09a3e0eaf0d266ca7f748e5df586d509e9e3 100644 (file)
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -906,6 +906,7 @@ cifs_demultiplex_thread(void *p)
                        continue;
                server->total_read += length;
 
+               mid_entry = NULL;
                if (server->ops->is_transform_hdr &&
                    server->ops->receive_transform &&
                    server->ops->is_transform_hdr(buf)) {
@@ -920,8 +921,11 @@ cifs_demultiplex_thread(void *p)
                                length = mid_entry->receive(server, mid_entry);
                }
 
-               if (length < 0)
+               if (length < 0) {
+                       if (mid_entry)
+                               cifs_mid_q_entry_release(mid_entry);
                        continue;
+               }
 
                if (server->large_buf)
                        buf = server->bigbuf;
@@ -938,6 +942,8 @@ cifs_demultiplex_thread(void *p)
 
                        if (!mid_entry->multiRsp || mid_entry->multiEnd)
                                mid_entry->callback(mid_entry);
+
+                       cifs_mid_q_entry_release(mid_entry);
                } else if (server->ops->is_oplock_break &&
                           server->ops->is_oplock_break(buf, server)) {
                        cifs_dbg(FYI, "Received oplock break\n");

diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c
index aff8ce8ba34d55485d1d15aa8b7ea498cf6726f3..646dcd149de1e368baebac10a940a70a095ef479 100644 (file)
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -107,6 +107,7 @@ cifs_find_mid(struct TCP_Server_Info *server, char *buffer)
                if (compare_mid(mid->mid, buf) &&
                    mid->mid_state == MID_REQUEST_SUBMITTED &&
                    le16_to_cpu(mid->command) == buf->Command) {
+                       kref_get(&mid->refcount);
                        spin_unlock(&GlobalMid_Lock);
                        return mid;
                }

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 4ee32488ff74a857fca08909561fa20fb234e95d..80ca393ae51155ff6217fae7403e31a05be4f944 100644 (file)
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -203,6 +203,7 @@ smb2_find_mid(struct TCP_Server_Info *server, char *buf)
                if ((mid->mid == wire_mid) &&
                    (mid->mid_state == MID_REQUEST_SUBMITTED) &&
                    (mid->command == shdr->Command)) {
+                       kref_get(&mid->refcount);
                        spin_unlock(&GlobalMid_Lock);
                        return mid;
                }

diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
index 8806f3f76c1d6c25c88bf98ba66aadd332d73b09..97f24d82ae6b6613c63242c9d606d0ef4d3a6626 100644 (file)
--- a/fs/cifs/smb2transport.c
+++ b/fs/cifs/smb2transport.c
@@ -548,6 +548,7 @@ smb2_mid_entry_alloc(const struct smb2_sync_hdr *shdr,
 
        temp = mempool_alloc(cifs_mid_poolp, GFP_NOFS);
        memset(temp, 0, sizeof(struct mid_q_entry));
+       kref_init(&temp->refcount);
        temp->mid = le64_to_cpu(shdr->MessageId);
        temp->pid = current->pid;
        temp->command = shdr->Command; /* Always LE */

diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index 927226a2122f4dde57955fe805013d14fd6237df..60faf2fcec7f5b9d80355190484348fb693c4d5d 100644 (file)
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -61,6 +61,7 @@ AllocMidQEntry(const struct smb_hdr *smb_buffer, struct TCP_Server_Info *server)
 
        temp = mempool_alloc(cifs_mid_poolp, GFP_NOFS);
        memset(temp, 0, sizeof(struct mid_q_entry));
+       kref_init(&temp->refcount);
        temp->mid = get_mid(smb_buffer);
        temp->pid = current->pid;
        temp->command = cpu_to_le16(smb_buffer->Command);
@@ -82,6 +83,21 @@ AllocMidQEntry(const struct smb_hdr *smb_buffer, struct TCP_Server_Info *server)
        return temp;
 }
 
+static void _cifs_mid_q_entry_release(struct kref *refcount)
+{
+       struct mid_q_entry *mid = container_of(refcount, struct mid_q_entry,
+                                              refcount);
+
+       mempool_free(mid, cifs_mid_poolp);
+}
+
+void cifs_mid_q_entry_release(struct mid_q_entry *midEntry)
+{
+       spin_lock(&GlobalMid_Lock);
+       kref_put(&midEntry->refcount, _cifs_mid_q_entry_release);
+       spin_unlock(&GlobalMid_Lock);
+}
+
 void
 DeleteMidQEntry(struct mid_q_entry *midEntry)
 {
@@ -110,7 +126,7 @@ DeleteMidQEntry(struct mid_q_entry *midEntry)
                }
        }
 #endif
-       mempool_free(midEntry, cifs_mid_poolp);
+       cifs_mid_q_entry_release(midEntry);
 }
 
 void