WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED INVALID
187468
valgrind claims memcpy overlap in CSSPropertyParser.cpp
https://bugs.webkit.org/show_bug.cgi?id=187468
Summary
valgrind claims memcpy overlap in CSSPropertyParser.cpp
Milan Crha
Reported
2018-07-09 07:59:44 PDT
I noticed this claim (among of several other) with webkit2gtk3-2.20.3-1.fc28.x86_64: ==2082== Source and destination overlap in memcpy_chk(0x1ffeffbbd7, 0x1ffeffbbd6, 8) ==2082== at 0x4C36490: __memcpy_chk (vg_replace_strmem.c:1581) ==2082== by 0x602E331: UnknownInlinedFun (string_fortified.h:40) ==2082== by 0x602E331: cssValueKeywordID<char16_t> (CSSPropertyParser.cpp:184) ==2082== by 0x602E331: WebCore::cssValueKeywordID(WTF::StringView) (CSSPropertyParser.cpp:202) ==2082== by 0x602E495: WebCore::CSSParserToken::id() const [clone .part.152] (CSSParserToken.cpp:309) ==2082== by 0x60501DB: WebCore::CSSPropertyParser::consumeCSSWideKeyword(WebCore::CSSPropertyID, bool) (CSSPropertyParser.cpp:342) ==2082== by 0x6058B08: WebCore::CSSPropertyParser::parseValueStart(WebCore::CSSPropertyID, bool) (CSSPropertyParser.cpp:302) ==2082== by 0x6058DD9: WebCore::CSSPropertyParser::parseValue(WebCore::CSSPropertyID, bool, WebCore::CSSParserTokenRange const&, WebCore::CSSParserContext const&, WTF::Vector<WebCore::CSSProperty, 256ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&, WebCore::StyleRuleBase::Type) (CSSPropertyParser.cpp:267) ==2082== by 0x60220FD: WebCore::CSSParserImpl::consumeDeclarationValue(WebCore::CSSParserTokenRange, WebCore::CSSPropertyID, bool, WebCore::StyleRuleBase::Type) (CSSParserImpl.cpp:857) ==2082== by 0x602677E: WebCore::CSSParserImpl::consumeDeclaration(WebCore::CSSParserTokenRange, WebCore::StyleRuleBase::Type) (CSSParserImpl.cpp:840) ==2082== by 0x6027DA9: WebCore::CSSParserImpl::consumeDeclarationList(WebCore::CSSParserTokenRange, WebCore::StyleRuleBase::Type) (CSSParserImpl.cpp:778) ==2082== by 0x6029094: WebCore::CSSParserImpl::consumeStyleRule(WebCore::CSSParserTokenRange, WebCore::CSSParserTokenRange) (CSSParserImpl.cpp:747) ==2082== by 0x602956E: WebCore::CSSParserImpl::consumeQualifiedRule(WebCore::CSSParserTokenRange&, WebCore::CSSParserImpl::AllowedRulesType) (CSSParserImpl.cpp:473) ==2082== by 0x602A331: consumeRuleList<WebCore::CSSParserImpl::parseStyleSheet(const WTF::String&, const WebCore::CSSParserContext&, WebCore::StyleSheetContents*, WebCore::CSSParser::RuleParsing)::<lambda(WTF::RefPtr<WebCore::StyleRuleBase>)> > (CSSParserImpl.cpp:387) ==2082== by 0x602A331: WebCore::CSSParserImpl::parseStyleSheet(WTF::String const&, WebCore::CSSParserContext const&, WebCore::StyleSheetContents*, WebCore::CSSParser::RuleParsing) (CSSParserImpl.cpp:245) The other claims are from /usr/lib64/libjavascriptcoregtk-4.0.so.18.7.11 (I do have webkit2gtk3-jsc-debuginfo-2.20.3-1.fc28.x86_64 installed, but it still shows only the library file for some reason) about invalid writes and use of uninitialized memory, but I do not want to mix it here. If you'd like a new bug for those, even without line numbers, then I can file it, but I'm afraid missing line numbers is a pita, thus I didn't do it.
Attachments
Add attachment
proposed patch, testcase, etc.
Michael Catanzaro
Comment 1
2018-07-09 08:16:05 PDT
This is memmove, not memcpy, so the source and destination are allowed to overlap....
Milan Crha
Comment 2
2018-07-09 08:21:02 PDT
Do you mean this had been addressed after 2.20.3 release already? That would be fine then.
Alexey Proskuryakov
Comment 3
2018-07-09 14:27:40 PDT
No, the code in question didn't change for the last couple years. if (isAppleLegacyCssValueKeyword(buffer, length) || hasPrefix(buffer, length, "-khtml-")) { memmove(buffer + 7, buffer + 6, length + 1 - 6); // This is line 184 memcpy(buffer, "-webkit", 7); ++length; }
Michael Catanzaro
Comment 4
2018-07-10 02:56:42 PDT
This seems to be a valgrind bug. I see the memmove() definition in glibc calls GCC's __builtin___memmove_chk(). valgrind seems to be intercepting that with __memcpy_chk rather than __memmove_chk like it should be... dunno how that's going wrong. I would report this to the valgrind developers and allow them to figure it out.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug