[webkit-reviews] review granted: [Bug 201570] Harden protection of the Gigacage Config parameters. : [Attachment 378259] proposed patch.

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 201570: Harden protection of the Gigacage Config parameters.
https://bugs.webkit.org/show_bug.cgi?id=201570

Attachment 378259: proposed patch.

https://bugs.webkit.org/attachment.cgi?id=378259&action=review




--- Comment #6 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 378259
  --> https://bugs.webkit.org/attachment.cgi?id=378259
proposed patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=378259&action=review

> Source/bmalloc/ChangeLog:20
> +	      On OS(DARWIN), this is made possible by using vm_protect with a
true
> +	      set_maximum argument.  We also add a
g_gigacageConfig.isPermanentlyFrozen flag
> +	      that we assert.

you should explain what this is for DARWIN

> Source/bmalloc/bmalloc/Gigacage.cpp:90
> +    result = vm_protect(mach_task_self(),
reinterpret_cast<vm_address_t>(&g_gigacageConfig), configSizeToProtect, false,
VM_PROT_READ);

let's name "false" here

> Source/bmalloc/bmalloc/Gigacage.cpp:116
> +    std::atomic_thread_fence(std::memory_order_seq_cst);

also is unneeded. Is is not expected that multiple threads can call this at the
same time, right?