Bug 2143040

Summary: valgrind Test 3012 fails for curl because memmove is seen as memcopy
Product: [Fedora] Fedora Reporter: Siddhesh Poyarekar <sipoyare>
Component: valgrindAssignee: Mark Wielaard <mjw>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 38CC: dodji, fweimer, jakub, jchecahi, kdudka, lzaoral, mjw, mjw, msekleta, paul, svashisht
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: valgrind-3.21.0-2.fc38 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2023-05-10 01:40:49 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Siddhesh Poyarekar 2022-11-15 21:37:59 UTC
Description of problem:
This is either a valgrind bug or a debuginfo bug of some sort (see Additional info) but I'm routing it through curl since I want to be sure that I haven't missed anything.

Version-Release number of selected component (if applicable):
curl-7.86.0-2.fc38

How reproducible:
Always

Steps to Reproduce:
Build curl-7.86.0-2.fc38 in rawhide with redhat-rpm-config from this copr:

https://copr.fedorainfracloud.org/coprs/siddhesh/fortify-source-3/

Actual results:
test 3012...[--output-dir with -J]
../libtool --mode=execute /usr/bin/valgrind --tool=memcheck --quiet --leak-check=yes --suppressions=../../tests/valgrind.supp --num-callers=16 --log-file=log/valgrind3012 ../src/curl --trace-ascii log/trace3012 --trace-time http://127.0.0.1:35981/this/is/the/3012 -OJ --output-dir /root/rpmbuild/BUILD/curl-7.86.0/build-minimal/tests/log >log/stdout3012 2>log/stderr3012
CMD (0): ../libtool --mode=execute /usr/bin/valgrind --tool=memcheck --quiet --leak-check=yes --suppressions=../../tests/valgrind.supp --num-callers=16 --log-file=log/valgrind3012 ../src/curl --trace-ascii log/trace3012 --trace-time http://127.0.0.1:35981/this/is/the/3012 -OJ --output-dir /root/rpmbuild/BUILD/curl-7.86.0/build-minimal/tests/log >log/stdout3012 2>log/stderr3012
 valgrind ERROR ==496584== Source and destination overlap in memcpy_chk(0x54ad1a0, 0x54ad1a1, 11)
==496584==    at 0x484C332: __memcpy_chk (vg_replace_strmem.c:1741)
==496584==    by 0x118FDB: UnknownInlinedFun (string_fortified.h:36)
==496584==    by 0x118FDB: UnknownInlinedFun (tool_cb_hdr.c:301)
==496584==    by 0x118FDB: tool_header_cb (tool_cb_hdr.c:173)
==496584==    by 0x489907B: chop_write.lto_priv.0 (sendf.c:620)
==496584==    by 0x489CDD1: UnknownInlinedFun (http.c:4449)
==496584==    by 0x489CDD1: UnknownInlinedFun (transfer.c:633)
==496584==    by 0x489CDD1: Curl_readwrite (transfer.c:1219)
==496584==    by 0x488C116: multi_runsingle (multi.c:2404)
==496584==    by 0x488F491: curl_multi_perform (multi.c:2682)
==496584==    by 0x486A9DA: UnknownInlinedFun (easy.c:663)
==496584==    by 0x486A9DA: UnknownInlinedFun (easy.c:753)
==496584==    by 0x486A9DA: curl_easy_perform (easy.c:772)
==496584==    by 0x114B28: UnknownInlinedFun (tool_operate.c:2406)
==496584==    by 0x114B28: UnknownInlinedFun (tool_operate.c:2594)
==496584==    by 0x114B28: UnknownInlinedFun (tool_operate.c:2706)
==496584==    by 0x114B28: main (tool_main.c:284)
==496584== 


Expected results:
No failure.

Additional info:

The function in tool_cb_hdr.c line 303 is memmove and looking at the generated code, it does correctly get generated as __memmove_chk.  There is a memcpy earlier in the parse_filename function, but it operates on non-aliased pointers (input ptr and a malloc'd object) so there's no question of overlap there.

I tinkered with the sources a bit and found that even though valgrind warns about line 303, replacing the malloc+memcpy with strndup is what silences the valgrind error.  This seems to indicate valgrind doing something funny.

EDIT: Updated valgrind error since the earlier one was from one of my debugging hacks.

Comment 1 Siddhesh Poyarekar 2022-11-15 21:38:59 UTC
Mark, this might interest you since it's likely that curl is all good and valgrind is acting funny.

Comment 2 Mark Wielaard 2022-11-15 23:59:28 UTC
This most likely is upstream bug https://bugzilla.redhat.com/show_bug.cgi?id=2143040 "memcheck/tests/overlap testcase fails, memcpy seen as memmove" where because of how valgrind handles ifuncs it looks like memmove is handled as if it is memcpy. I assume the _chk variants are (badly) intercepted the same way.

Comment 3 Kamil Dudka 2022-11-16 07:53:31 UTC
I checked the source code of curl-7.86.0-2.fc38 and there is indeed `memmove(copy, p, strlen(p) + 1)` on line 301 in curl-7.86.0/src/tool_cb_hdr.c which should be safe when source and destination overlap.  I am switching the component to vaglrind.

Mark, there seems to be a copy/paste mistake in comment #2 - the URL points to this bug rather than an upstream bug.

Comment 4 Mark Wielaard 2022-11-16 09:07:29 UTC
(In reply to Kamil Dudka from comment #3)
> Mark, there seems to be a copy/paste mistake in comment #2 - the URL points
> to this bug rather than an upstream bug.

Doh. Sorry. Should be:
https://bugs.kde.org/show_bug.cgi?id=402833
"memcheck/tests/overlap testcase fails, memcpy seen as memmove"

Comment 5 Siddhesh Poyarekar 2022-11-16 16:18:38 UTC
(In reply to Kamil Dudka from comment #3)
> I checked the source code of curl-7.86.0-2.fc38 and there is indeed
> `memmove(copy, p, strlen(p) + 1)` on line 301 in
> curl-7.86.0/src/tool_cb_hdr.c which should be safe when source and
> destination overlap.  I am switching the component to vaglrind.

On the curl front, we probably need to figure out what to do until the valgrind bug gets fixed, since that valgrind bug looks pretty old. I'm hoping to propose building with _FORTIFY_SOURCE=3 systemwide in Fedora and this bug may prevent curl from being built out of the box.

Comment 6 Kamil Dudka 2022-11-17 20:19:11 UTC
> On the curl front, we probably need to figure out what to do until the valgrind bug gets fixed

We can easily disable valgrind for a specific test case like this:

    https://src.fedoraproject.org/rpms/curl/c/768ce396

Comment 7 Siddhesh Poyarekar 2022-11-17 21:19:04 UTC
FWIW, replacing the malloc+memcpy sequence with strndup gets rid of the problem.  I wonder why curl avoids strndup though, since the omission seems deliberate.

Comment 8 Kamil Dudka 2022-11-18 08:34:26 UTC
I think they avoid strndup() because of portability.  Upstream supports curl on weird platforms.  They have even their own implementation of strdup():

    https://github.com/curl/curl/blob/master/src/tool_strdup.c

Comment 9 Kamil Dudka 2023-01-11 08:04:29 UTC
ELN builds of curl started to fail because of this bug:

    https://koji.fedoraproject.org/koji/taskinfo?taskID=95974757

I have temporarily disabled valgrind for test3012 of curl:

    https://src.fedoraproject.org/rpms/curl/c/04ebed546a2129a215ef7f8db67b906b7bcb6f12?branch=rawhide

Comment 10 Mark Wielaard 2023-01-11 14:09:52 UTC
I renamed this bug to make it a bit more clear what is going on.
Unfortunately there has been no progress getting this bug fixed upstream.
It is a little tricky when the ifunc for memcpy and memmove point to the same address.
I'll see if I can disable the overlap checking at least when this situation occurs.

Comment 11 Ben Cotton 2023-02-07 14:59:11 UTC
This bug appears to have been reported against 'rawhide' during the Fedora Linux 38 development cycle.
Changing version to 38.

Comment 12 Fedora Update System 2023-05-05 17:06:00 UTC
FEDORA-2023-db81140761 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-db81140761

Comment 13 Fedora Update System 2023-05-06 02:00:28 UTC
FEDORA-2023-db81140761 has been pushed to the Fedora 38 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-db81140761`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-db81140761

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 14 Fedora Update System 2023-05-10 01:40:49 UTC
FEDORA-2023-db81140761 has been pushed to the Fedora 38 stable repository.
If problem still persists, please make note of it in this bug report.