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
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.