WebKit Bugzilla
Attachment 372639 Details for
Bug 199077
: [bmalloc] IsoTLS Layout extension initializes one IsoTLSEntry twice
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-199077-20190621123657.patch (text/plain), 3.88 KB, created by
Yusuke Suzuki
on 2019-06-21 12:36:57 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Yusuke Suzuki
Created:
2019-06-21 12:36:57 PDT
Size:
3.88 KB
patch
obsolete
>Subversion Revision: 246687 >diff --git a/Source/bmalloc/ChangeLog b/Source/bmalloc/ChangeLog >index ec4db9070e69305067db9d9e52548ab59a43d074..ee236d441a25f07d178a54d11c2e51e6aab99a69 100644 >--- a/Source/bmalloc/ChangeLog >+++ b/Source/bmalloc/ChangeLog >@@ -1,3 +1,26 @@ >+2019-06-21 Yusuke Suzuki <ysuzuki@apple.com> >+ >+ [bmalloc] IsoTLS Layout extension initializes one IsoTLSEntry twice >+ https://bugs.webkit.org/show_bug.cgi?id=199077 >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Found that IsoTLS::ensureEntries can construct the same IsoTLSEntry many times, it can leak memory because the construction clears previous fields including freelist. >+ >+ 1. We have oldLastEntry. >+ 2. In that case, startEntry is oldLastEntry. >+ 3. We find some targetEntry. >+ 4. Finally, if startEntry exists, we newly construct [startEntry, targetEntry] >+ 5. In the above sequence, oldLastEntry (== startEntry) is constructed again, while oldLastEntry is already constructed previously. >+ >+ We fix this issue by changing the startEntry. We already have `RELEASE_BASSERT(!oldLastEntry || oldLastEntry->offset() < offset);` >+ assertion. This means that `oldLastEntry->m_next` must exist, otherwise the following loop would not find a `targetEntry`. And `layout.head()` >+ must return non nullptr at `IsoTLS::ensureEntries` because `IsoTLS::ensureEntries` requires that `IsoHeap<>` is initialized, and `IsoHeap<>` >+ must add at least one TLS entry to the IsoTLSLayout. >+ >+ * bmalloc/IsoTLS.cpp: >+ (bmalloc::IsoTLS::ensureEntries): >+ > 2019-06-19 Yusuke Suzuki <ysuzuki@apple.com> > > [bmalloc] IsoHeap's initialization is racy with IsoHeap::isInitialized >diff --git a/Source/bmalloc/bmalloc/IsoTLS.cpp b/Source/bmalloc/bmalloc/IsoTLS.cpp >index c203213b855bdcdc9ed0796fd204ab5d948d8006..b5d322194e3a051eac09a41029ca241f5d47890d 100644 >--- a/Source/bmalloc/bmalloc/IsoTLS.cpp >+++ b/Source/bmalloc/bmalloc/IsoTLS.cpp >@@ -82,21 +82,19 @@ IsoTLS* IsoTLS::ensureEntries(unsigned offset) > IsoTLSEntry* oldLastEntry = tls ? tls->m_lastEntry : nullptr; > RELEASE_BASSERT(!oldLastEntry || oldLastEntry->offset() < offset); > >- IsoTLSEntry* startEntry = oldLastEntry ? oldLastEntry : layout.head(); >+ IsoTLSEntry* startEntry = oldLastEntry ? oldLastEntry->m_next : layout.head(); >+ RELEASE_BASSERT(startEntry); > > IsoTLSEntry* targetEntry = startEntry; >- size_t requiredCapacity = 0; >- if (startEntry) { >- for (;;) { >- RELEASE_BASSERT(targetEntry); >- RELEASE_BASSERT(targetEntry->offset() <= offset); >- if (targetEntry->offset() == offset) >- break; >- targetEntry = targetEntry->m_next; >- } >+ for (;;) { > RELEASE_BASSERT(targetEntry); >- requiredCapacity = targetEntry->extent(); >+ RELEASE_BASSERT(targetEntry->offset() <= offset); >+ if (targetEntry->offset() == offset) >+ break; >+ targetEntry = targetEntry->m_next; > } >+ RELEASE_BASSERT(targetEntry); >+ size_t requiredCapacity = targetEntry->extent(); > > if (!tls || requiredCapacity > tls->m_capacity) { > size_t requiredSize = sizeForCapacity(requiredCapacity); >@@ -122,16 +120,14 @@ IsoTLS* IsoTLS::ensureEntries(unsigned offset) > set(tls); > } > >- if (startEntry) { >- startEntry->walkUpToInclusive( >- targetEntry, >- [&] (IsoTLSEntry* entry) { >- entry->construct(tls->m_data + entry->offset()); >- }); >- >- tls->m_lastEntry = targetEntry; >- tls->m_extent = targetEntry->extent(); >- } >+ startEntry->walkUpToInclusive( >+ targetEntry, >+ [&] (IsoTLSEntry* entry) { >+ entry->construct(tls->m_data + entry->offset()); >+ }); >+ >+ tls->m_lastEntry = targetEntry; >+ tls->m_extent = targetEntry->extent(); > > return tls; > }
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
saam
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 199077
:
372622
|
372623
| 372639