Bug 169029 - Leak under Options::setOptions
Summary: Leak under Options::setOptions
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tomas Popela
URL:
Keywords:
Depends on: 169055
Blocks: 104114
  Show dependency treegraph
 
Reported: 2017-03-01 04:06 PST by Tomas Popela
Modified: 2017-03-08 06:49 PST (History)
6 users (show)

See Also:


Attachments
Patch (1.81 KB, patch)
2017-03-01 04:09 PST, Tomas Popela
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Popela 2017-03-01 04:06:50 PST
The optionsStrCopy variable is leaked in Options::setOptions in Source/JavaScriptCore/runtime/Options.cpp
Comment 1 Tomas Popela 2017-03-01 04:09:12 PST
Created attachment 303061 [details]
Patch
Comment 2 Michael Saboff 2017-03-01 07:35:03 PST
Comment on attachment 303061 [details]
Patch

r=me
Comment 3 Tomas Popela 2017-03-01 07:40:12 PST
Comment on attachment 303061 [details]
Patch

Clearing flags on attachment: 303061

Committed r213222: <http://trac.webkit.org/changeset/213222>
Comment 4 Tomas Popela 2017-03-01 07:40:21 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Mark Lam 2017-03-01 09:59:22 PST
Comment on attachment 303061 [details]
Patch

This exposes a problem.  Previously, by leaking the optionsStrCopy, we ensure that the option string being parsed by setOption() persists.  Now that we free it properly, we can have a use after free scenario because we don't strdup string type options.  See the FOR_EACH_OPTION macro in Options::setOptionWithoutAlias().  I think we should revert this patch and take the leak (since we don't parse options like this all the time) until we can fix string type options to strdup appropriately.
Comment 6 Alexey Proskuryakov 2017-03-01 13:32:06 PST
Yes, please roll out. A use after free is a much worse symptom.
Comment 7 Michael Saboff 2017-03-01 14:01:22 PST
I talked with Mark and I have a follow patch that fixes the UAF and reduces any leaks.  I'll post in a few minutes.  Filed <https://bugs.webkit.org/show_bug.cgi?id=169055> to track that change.