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.
This commit is contained in:
Behdad Esfahbod 2013-02-25 17:06:35 -05:00
parent 57542d7f41
commit c3ba49b6fa
2 changed files with 27 additions and 1 deletions

View File

@ -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);

View File

@ -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,