From c3ba49b6fa1865e8318926eaa6c0f2063d1053bb Mon Sep 17 00:00:00 2001
From: Behdad Esfahbod <behdad@behdad.org>
Date: Mon, 25 Feb 2013 17:06:35 -0500
Subject: [PATCH] Always create sub-blobs in MEMORY_MODE_READONLY

This fixes a design bug with sanitize and sub-blobs that can
cause crashes.  Jonathan and I found and debugged this issue
when we tested a corrupt font with the md5sum / filename:
ea395483d37af0cb933f40689ff7b60a.  Two hours of intense
debugging we found out that the font has overlapping GSUB/GPOS
tables, and as such, sanitizing the second table can modify
the first one, which can cause all kinds of undefined behavior.

The correct way to fix this is to make sure sub-blobs are
always created readonly, since we consider the parent blob
to be a shared resource and can't modify it, even if it *is*
writable.

This essentially makes the READONLY_MAY_MAKE_WRITABLE mode
unused...  Maybe we should simply remove / deprecate it.
---
 src/hb-blob.cc |  2 +-
 src/hb-blob.h  | 26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/hb-blob.cc b/src/hb-blob.cc
index 3ca50fbb1..dfd134b77 100644
--- a/src/hb-blob.cc
+++ b/src/hb-blob.cc
@@ -123,7 +123,7 @@ hb_blob_create_sub_blob (hb_blob_t    *parent,
 
   blob = hb_blob_create (parent->data + offset,
 			 MIN (length, parent->length - offset),
-			 parent->mode,
+			 HB_MEMORY_MODE_READONLY,
 			 hb_blob_reference (parent),
 			 (hb_destroy_func_t) hb_blob_destroy);
 
diff --git a/src/hb-blob.h b/src/hb-blob.h
index 1a93baa46..d3d0f41b1 100644
--- a/src/hb-blob.h
+++ b/src/hb-blob.h
@@ -36,6 +36,26 @@
 HB_BEGIN_DECLS
 
 
+/*
+ * Note re various memory-modes:
+ *
+ * - In no case shall the HarfBuzz client modify memory
+ *   that is passed to HarfBuzz in a blob.  If there is
+ *   any such possibility, MODE_DUPLICATE should be used
+ *   such that HarfBuzz makes a copy immediately,
+ *
+ * - Use MODE_READONLY otherse, unless you really really
+ *   really know what you are doing,
+ *
+ * - MODE_WRITABLE is appropriate if you relaly made a
+ *   copy of data solely for the purpose of passing to
+ *   HarfBuzz and doing that just once (no reuse!),
+ *
+ * - If the font is mmap()ed, it's ok to use
+ *   READONLY_MAY_MAKE_WRITABLE, however, there were
+ *   design problems with that mode, so HarfBuzz do not
+ *   really use it anymore.  If not sure, use MODE_READONLY.
+ */
 typedef enum {
   HB_MEMORY_MODE_DUPLICATE,
   HB_MEMORY_MODE_READONLY,
@@ -52,6 +72,12 @@ hb_blob_create (const char        *data,
 		void              *user_data,
 		hb_destroy_func_t  destroy);
 
+/* Always creates with MEMORY_MODE_READONLY.
+ * Even if the parent blob is writable, we don't
+ * want the user of the sub-blob to be able to
+ * modify the parent data as that data may be
+ * shared among multiple sub-blobs.
+ */
 hb_blob_t *
 hb_blob_create_sub_blob (hb_blob_t    *parent,
 			 unsigned int  offset,