commit 4a81ab81e4d947a3a0c8408c743ad94e7c1b8a30
author Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> Fri Sep 30 11:39:11 2011 +0100
committer Justin M. Forbes <jforbes@redhat.com> Wed Oct 05 11:33:31 2011 -0500
tree 1ebb1bf863631bd49832d5460adc02f8aab998ab
parent 68e3508eafd111c050e0a6f4dc8588dbe75d0e1c

qed: fix use-after-free during l2 cache commit

QED's metadata caching strategy allows two parallel requests to race for
metadata lookup.  The first one to complete will populate the metadata
cache and the second one will drop the data it just read in favor of the
cached data.

There is a use-after-free in qed_read_l2_table_cb() and
qed_commit_l2_update() where l2_table->offset was used after the
l2_table may have been freed due to a metadata lookup race.  Fix this by
keeping the l2_offset in a local variable and not reaching into the
possibly freed l2_table.

Reported-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

block/qed-table.c

diff --git a/block/qed-table.c b/block/qed-table.c
index d38c673..5c5d4b4 100644
--- a/block/qed-table.c
+++ b/block/qed-table.c
@@ -230,21 +230,21 @@
     QEDRequest *request = read_l2_table_cb->request;
     BDRVQEDState *s = read_l2_table_cb->s;
     CachedL2Table *l2_table = request->l2_table;
+    uint64_t l2_offset = read_l2_table_cb->l2_offset;
 
     if (ret) {
         /* can't trust loaded L2 table anymore */
         qed_unref_l2_cache_entry(l2_table);
         request->l2_table = NULL;
     } else {
-        l2_table->offset = read_l2_table_cb->l2_offset;
+        l2_table->offset = l2_offset;
 
         qed_commit_l2_cache_entry(&s->l2_cache, l2_table);
 
         /* This is guaranteed to succeed because we just committed the entry
          * to the cache.
          */
-        request->l2_table = qed_find_l2_cache_entry(&s->l2_cache,
-                                                    l2_table->offset);
+        request->l2_table = qed_find_l2_cache_entry(&s->l2_cache, l2_offset);
         assert(request->l2_table != NULL);
     }
 

block/qed.c

diff --git a/block/qed.c b/block/qed.c
index 3970379..4dc33be 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -915,14 +915,14 @@
     QEDAIOCB *acb = opaque;
     BDRVQEDState *s = acb_to_s(acb);
     CachedL2Table *l2_table = acb->request.l2_table;
+    uint64_t l2_offset = l2_table->offset;
 
     qed_commit_l2_cache_entry(&s->l2_cache, l2_table);
 
     /* This is guaranteed to succeed because we just committed the entry to the
      * cache.
      */
-    acb->request.l2_table = qed_find_l2_cache_entry(&s->l2_cache,
-                                                    l2_table->offset);
+    acb->request.l2_table = qed_find_l2_cache_entry(&s->l2_cache, l2_offset);
     assert(acb->request.l2_table != NULL);
 
     qed_aio_next_io(opaque, ret);