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 33d6eb58ce34124174650d471ff705d6b56ae359..f29cdb1cdeb773670ce88c4074ba1e688b03df32 100644 (file)
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1340,6 +1340,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 762d513a5087507ce4f0b362dec3840c32027d98..ccdb42f71b2e8715aed67b557b0062a53d9a4696 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 f7db2fedfa8c63a75331bacf7226c072723fe2e9..fd24c72bd2cd6783bd38f831d9f21a576356b3c6 100644 (file)
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -889,6 +889,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)) {
@@ -903,8 +904,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;
@@ -920,6 +924,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 a723df3e01978cdca30afbf107772898ce66f33a..d8cd82001c1cb56a071226405f9aca381fb1ae69 100644 (file)
--- a/fs/cifs/smb1ops.c
+++ b/fs/cifs/smb1ops.c
@@ -105,6 +105,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 36bc9a7eb8ea5fbb93d43512e5e54bc836c209ab..51d69473dbd9ac458078f18cd3c51dc387167f41 100644 (file)
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -202,6 +202,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 bf49cb73b9e6ad80f4e67b961e22640fda6df3db..a41fc4a63a59d962999169c2ef4cd0c3214c0294 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 7efbab013957551c1812f73d833426a537bd99c7..a10f51dfa7f51543b7bb5aa7ee9cf6568eebed67 100644 (file)
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -56,6 +56,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);
@@ -77,6 +78,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)
 {
@@ -105,7 +121,7 @@ DeleteMidQEntry(struct mid_q_entry *midEntry)
                }
        }
 #endif
-       mempool_free(midEntry, cifs_mid_poolp);
+       cifs_mid_q_entry_release(midEntry);
 }
 
 void