All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] qed: fix use-after-free during l2 cache commit
@ 2011-09-30 10:39 Stefan Hajnoczi
  2011-09-30 11:27 ` Amit Shah
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Stefan Hajnoczi @ 2011-09-30 10:39 UTC (permalink / raw)
  To: Amit Shah; +Cc: Anthony Liguori, qemu-devel, Stefan Hajnoczi

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>
---
Hi Amit,
Thanks for reporting the assertion failure you saw at http://fpaste.org/CDuv/.
Does this patch fix the problem?

If not, please send details on your setup and how to reproduce the issue.

Thanks,
Stefan

 block/qed-table.c |    6 +++---
 block/qed.c       |    4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/qed-table.c b/block/qed-table.c
index d96afa8..f31f9ff 100644
--- a/block/qed-table.c
+++ b/block/qed-table.c
@@ -222,21 +222,21 @@ static void qed_read_l2_table_cb(void *opaque, int ret)
     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);
     }
 
diff --git a/block/qed.c b/block/qed.c
index 624e261..e87dc4d 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -911,14 +911,14 @@ static void qed_commit_l2_update(void *opaque, int ret)
     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);
-- 
1.7.6.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-10-12  7:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-30 10:39 [Qemu-devel] [PATCH] qed: fix use-after-free during l2 cache commit Stefan Hajnoczi
2011-09-30 11:27 ` Amit Shah
2011-09-30 15:23   ` Stefan Hajnoczi
2011-09-30 15:49     ` Amit Shah
2011-10-11 14:22       ` Kevin Wolf
2011-10-12  7:53         ` Stefan Hajnoczi
2011-09-30 15:26 ` Stefan Hajnoczi
2011-10-03 19:53   ` Anthony Liguori
2011-10-05 16:17 ` Anthony Liguori

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.