187468 2018-07-09 07:59:44 -0700 valgrind claims memcpy overlap in CSSPropertyParser.cpp 2018-07-10 02:56:42 -0700 1 1 1 Unclassified WebKit CSS Other Unspecified Unspecified RESOLVED INVALID P2 Normal --- 1 mcrha webkit-unassigned ap bugs-noreply calvaris mcatanzaro oldest_to_newest 1440380 0 mcrha 2018-07-09 07:59:44 -0700 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. 1440384 1 mcatanzaro 2018-07-09 08:16:05 -0700 This is memmove, not memcpy, so the source and destination are allowed to overlap.... 1440386 2 mcrha 2018-07-09 08:21:02 -0700 Do you mean this had been addressed after 2.20.3 release already? That would be fine then. 1440558 3 ap 2018-07-09 14:27:40 -0700 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; } 1440793 4 mcatanzaro 2018-07-10 02:56:42 -0700 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.