DynamoRIO / DynamoRIO/drmemory

UNADDRs beyond TOS in WebCore::RenderStyle::resetBorder*: compiler reads beyond TOS before setting up frame!

Open
#582 3 comments 0 reactions 0 assignees View on GitHub
Bug-FalsePositive Migrated OpSys-Windows Priority-Medium
Dominant language
C
Stars
2.7k
Forks
290
PR merge metrics
No merged PRs in 30d

Description

_From [bruen...@google.com](https://code.google.com/u/109494838902877177630/) on September 07, 2011 11:48:09_

on chrome Release build (with /Ob0 /Oy-):

seen in several ui_tests:
test-name=OptionsUITest.LoadOptionsByURL
test-name=AutomationProxyTest.AcceleratorExtensions
test-name=OptionsUITest.NavBarCheck

Error `#12476`: UNADDRESSABLE ACCESS: reading 0x0037e764-0x0037e768 4 byte(s)
@0:29:15.135 in thread 3124
Note: instruction: mov 0xfffffffc(%esp) -> %eax
#0 chrome.dll!WebCore::RenderStyle::resetBorderTop [e:\src\chromium\src\third_party\webkit\source\webcore\rendering\style\renderstyle.h:845]
#1 chrome.dll!WebCore::RenderTheme::adjustRadioStyle [e:\src\chromium\src\third_party\webkit\source\webcore\rendering\rendertheme.cpp:875]
#2 chrome.dll!WebCore::RenderTheme::adjustStyle [e:\src\chromium\src\third_party\webkit\source\webcore\rendering\rendertheme.cpp:238]
#3 chrome.dll!WebCore::CSSStyleSelector::adjustRenderStyle [e:\src\chromium\src\third_party\webkit\source\webcore\css\cssstyleselector.cpp:1989]
#4 chrome.dll!WebCore::CSSStyleSelector::styleForElement [e:\src\chromium\src\third_party\webkit\source\webcore\css\cssstyleselector.cpp:1553]
#5 chrome.dll!WebCore::Node::styleForRenderer [e:\src\chromium\src\third_party\webkit\source\webcore\dom\node.cpp:1476]
#6 chrome.dll!WebCore::NodeRendererFactory::createRendererAndStyle [e:\src\chromium\src\third_party\webkit\source\webcore\dom\noderenderingcontext.cpp:317]
#7 chrome.dll!WebCore::NodeRendererFactory::createRendererIfNeeded [e:\src\chromium\src\third_party\webkit\source\webcore\dom\noderenderingcontext.cpp:366]
#8 chrome.dll!WebCore::Node::createRendererIfNeeded [e:\src\chromium\src\third_party\webkit\source\webcore\dom\node.cpp:1465]
#9 chrome.dll!WebCore::Element::attach [e:\src\chromium\src\third_party\webkit\source\webcore\dom\element.cpp:1015]
#10 chrome.dll!WebCore::HTMLFormControlElement::attach [e:\src\chromium\src\third_party\webkit\source\webcore\html\htmlformcontrolelement.cpp:155]
#11 chrome.dll!WebCore::HTMLInputElement::attach [e:\src\chromium\src\third_party\webkit\source\webcore\html\htmlinputelement.cpp:872]
#12 chrome.dll!WebCore::ContainerNode::attach [e:\src\chromium\src\third_party\webkit\source\webcore\dom\containernode.cpp:767]
#13 chrome.dll!WebCore::Element::attach [e:\src\chromium\src\third_party\webkit\source\webcore\dom\element.cpp:1021]
#14 chrome.dll!WebCore::ContainerNode::attach [e:\src\chromium\src\third_party\webkit\source\webcore\dom\containernode.cpp:767]
#15 chrome.dll!WebCore::Element::attach [e:\src\chromium\src\third_party\webkit\source\webcore\dom\element.cpp:1021]
#16 chrome.dll!WebCore::ContainerNode::attach [e:\src\chromium\src\third_party\webkit\source\webcore\dom\containernode.cpp:767]
#17 chrome.dll!WebCore::Element::attach [e:\src\chromium\src\third_party\webkit\source\webcore\dom\element.cpp:1021]
#18 chrome.dll!WebCore::ContainerNode::attach [e:\src\chromium\src\third_party\webkit\source\webcore\dom\containernode.cpp:767]
#19 chrome.dll!WebCore::Element::attach [e:\src\chromium\src\third_party\webkit\source\webcore\dom\element.cpp:1021]

other near-identical ones w/ same beyond-TOS address but
resetBorderRight, resetBorderBottom, and resetBorderLeft

```
void resetBorderTop() { SET_VAR(surround, border.m_top, BorderValue()) }
void resetBorderRight() { SET_VAR(surround, border.m_right, BorderValue()) }
void resetBorderBottom() { SET_VAR(surround, border.m_bottom, BorderValue()) }
void resetBorderLeft() { SET_VAR(surround, border.m_left, BorderValue()) }
```

#define SET_VAR(group, variable, value) \
if (!compareEqual(group->variable, value)) \
group.access()->variable = value;

check out the disasm:
0:025> Uf chrome_68780000!WebCore::RenderStyle::resetBorderTop
chrome_68780000!WebCore::RenderStyle::resetBorderTop [e:\src\chromium\src\third_party\webkit\source\webcore\rendering\style\renderstyle.h @ 845]:
845 6977e6c0 8b4424fc mov eax,[esp-0x4]
845 6977e6c4 83ec0c sub esp,0xc
845 6977e6c7 56 push esi
845 6977e6c8 8bf1 mov esi,ecx
845 6977e6ca 8b4e18 mov ecx,[esi+0x18]
845 6977e6cd 8b9184000000 mov edx,[ecx+0x84]
845 6977e6d3 83c17c add ecx,0x7c
845 6977e6d6 250300ffff and eax,0xffff0003
845 6977e6db 57 push edi

(hmmm, I should run w/ sym+offs, it would have shown +0)

wow, I've never seen a compiler produce this before: access beyond TOS
(skipping retaddr) to get 1st param, before setting up new frame. on *nix
this would be unsafe (signals). on windows this still doesn't feel right,
though guard page handled in kernel w/o touching app stack I suppose, and
other fault on reading stack would result in failure to set up exception
stack frame anyway (no alt stack on windows).

the question is, should this just be suppressed for chrome, or since cl
seems to generate this, should drmem allow beyond-TOS on function entry (on
windows only)?

_Original issue: http://code.google.com/p/drmemory/issues/detail?id=582_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.