Received: from mail.wilcox-tech.com (mail.wilcox-tech.com [45.32.83.9]) by nld3-dev1.alpinelinux.org (Postfix) with ESMTPS id 8B1D5781955 for <~alpine/apk-tools@lists.alpinelinux.org>; Thu, 11 Jul 2019 01:26:05 +0000 (UTC) Received: (qmail 8808 invoked from network); 11 Jul 2019 01:26:01 -0000 Received: from localhost (HELO ?IPv6:2600:1702:2a80:1b9f:5bbc:af4c:5dd1:a183?) (awilcox@wilcox-tech.com@127.0.0.1) by localhost with ESMTPA; 11 Jul 2019 01:26:01 -0000 Subject: Re: apk_blob_pull_dep corrupts virtpkg->version->ptr under clang To: ~alpine/apk-tools@lists.alpinelinux.org References: <5bbbc186-d3c5-beca-bfdb-f530a6c307e3@toastin.space> From: "A. Wilcox" Openpgp: preference=signencrypt Organization: =?UTF-8?Q?Ad=c3=a9lie_Linux?= Message-ID: Date: Wed, 10 Jul 2019 20:26:00 -0500 User-Agent: Mozilla/5.0 (X11; Linux ppc64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <5bbbc186-d3c5-beca-bfdb-f530a6c307e3@toastin.space> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Ry3FNGV7w9KwHH8P8ouLIW8RkElUqeunR" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Ry3FNGV7w9KwHH8P8ouLIW8RkElUqeunR Content-Type: multipart/mixed; boundary="LSWXjUQatMY2DCWZvAMNFcTaxNQFt4Ips"; protected-headers="v1" From: "A. Wilcox" To: ~alpine/apk-tools@lists.alpinelinux.org Message-ID: Subject: Re: apk_blob_pull_dep corrupts virtpkg->version->ptr under clang References: <5bbbc186-d3c5-beca-bfdb-f530a6c307e3@toastin.space> In-Reply-To: <5bbbc186-d3c5-beca-bfdb-f530a6c307e3@toastin.space> --LSWXjUQatMY2DCWZvAMNFcTaxNQFt4Ips Content-Type: multipart/mixed; boundary="------------55E8806B56D33F3B38F3BCE4" Content-Language: en-US This is a multi-part message in MIME format. --------------55E8806B56D33F3B38F3BCE4 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/10/19 18:55, Chloe Kudryavtsev wrote: > Hi, virtual packages get their version corrupted under clang. > It specifically happens in add.c::171 (well, the invocation, digging > deeper turned out to be difficult). >=20 > Steps to reproduce: > 1. Launch a docker image - I used alpine:edge > 2. Install the packages we'll need for the demonstration, full list at = [1]. > 3. git clone https://git.alpinelinux.org/apk-tools > 4. cd apk-tools > 5. Apply the following patch: https://brpaste.xyz/raw/2B_m0A. Reasoning= > in [2]. (curl https://brpaste.xyz/raw/2B_m0A | patch -p1) > 6. make CC=3Dclang CFLAGS=3D-O2 > 7. ./src/apk -s add -t foo foo > 8. Observe the discrepancies >=20 > NOTE: CFLAGS are -O2 because that makes the error easier to observe. > However it shows up even under -O0. >=20 > I have no familiarity with apk internals, but clang compiles many other= > things just fine without corrupting pointers at random. > Could someone look into this? >=20 > [1]: musl-dev linux-headers lua5.2-dev openssl-dev zlib-dev clang gcc > make git curl > [2]: This adds two printf statements for easy viewing and removes -Wall= > (clang has stricter warnings, should be tackled, but maybe later). My previous fix for this was incomplete. I ran APK through Valgrind afterwards and found that it was insufficient; it was duplicating the atom but not the blob string itself, so there was still an invalid read. v2 of this patch handles that, strdup()ing the version string the same as the description string. Additionally, Valgrind leads me to believe that the checksum data may also being allocated on the stack: =3D=3D56753=3D=3D Use of uninitialised value of size 8 =3D=3D56753=3D=3D at 0x12F204: apk_hash_get_hashed (hash.c:0) =3D=3D56753=3D=3D by 0x11B1DF: apk_hash_get (apk_hash.h:70) =3D=3D56753=3D=3D by 0x11BAE7: apk_db_pkg_add (database.c:528) =3D=3D56753=3D=3D by 0x112803: add_main (add.c:193) =3D=3D56753=3D=3D by 0x1115EB: main (apk.c:681) =3D=3D56753=3D=3D Uninitialised value was created by a stack allocation =3D=3D56753=3D=3D at 0x112B24: create_virtual_package (add.c:85) =3D=3D56753=3D=3D =3D=3D56753=3D=3D Conditional jump or move depends on uninitialised value= (s) =3D=3D56753=3D=3D at 0x4913260: memcmp (in /usr/lib/valgrind/vgpreload_memcheck-ppc64be-linux.so) =3D=3D56753=3D=3D by 0x12DFEB: apk_blob_compare (blob.c:262) =3D=3D56753=3D=3D by 0x12F2C7: apk_hash_get_hashed (hash.c:66) =3D=3D56753=3D=3D by 0x11B1DF: apk_hash_get (apk_hash.h:70) =3D=3D56753=3D=3D by 0x11BAE7: apk_db_pkg_add (database.c:528) =3D=3D56753=3D=3D by 0x112803: add_main (add.c:193) =3D=3D56753=3D=3D by 0x1115EB: main (apk.c:681) =3D=3D56753=3D=3D Uninitialised value was created by a stack allocation =3D=3D56753=3D=3D at 0x112B24: create_virtual_package (add.c:85) Patch v2 does not rectify this. I am still investigating. Best, --arw --=20 A. Wilcox (awilfox) Project Lead, Ad=C3=A9lie Linux https://www.adelielinux.org --------------55E8806B56D33F3B38F3BCE4 Content-Type: text/plain; charset=UTF-8; name="0001-add-create_virtual_package-dup-ver-string.patch" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="0001-add-create_virtual_package-dup-ver-string.patch" RnJvbSBmODY1ZWQ1YWIwNjY3YmJmNWZkZjRlMGRhM2IwOWI0ZWJhMTg5NTY3IE1vbiBTZXAg MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiAiQS4gV2lsY294IiA8QVdpbGNveEBXaWxjb3gtVGVj aC5jb20+CkRhdGU6IFdlZCwgMTAgSnVsIDIwMTkgMTk6NDQ6NDggLTA1MDAKU3ViamVjdDog W1BBVENIXSBhZGQ6IGNyZWF0ZV92aXJ0dWFsX3BhY2thZ2U6IGR1cCB2ZXIgc3RyaW5nCgp2 ZXIgaXMgYSBzdGFjay1hbGxvY2F0ZWQgdmFyaWFibGUuICBJdHMgc2NvcGUgZW5kcyB3aGVu IHRoZSBmdW5jdGlvbiBkb2VzLgpUaGlzIG1lYW5zIHRoYXQgdGhlIHZlcnNpb24gYXRvbSBp cyBubyBsb25nZXIgdmFsaWQgYWZ0ZXIgdGhlIHJldHVybiBvZgpjcmVhdGVfdmlydHVhbF9w YWNrYWdlLgotLS0KIHNyYy9hZGQuYyB8IDIgKy0KIDEgZmlsZSBjaGFuZ2VkLCAxIGluc2Vy dGlvbigrKSwgMSBkZWxldGlvbigtKQoKZGlmZiAtLWdpdCBhL3NyYy9hZGQuYyBiL3NyYy9h ZGQuYwppbmRleCA0YzI4NWY3Li42MmZkZTRhIDEwMDY0NAotLS0gYS9zcmMvYWRkLmMKKysr IGIvc3JjL2FkZC5jCkBAIC05Nyw3ICs5Nyw3IEBAIHN0YXRpYyBzdHJ1Y3QgYXBrX3BhY2th Z2UgKmNyZWF0ZV92aXJ0dWFsX3BhY2thZ2Uoc3RydWN0IGFwa19kYXRhYmFzZSAqZGIsIHN0 cnVjCiAJaWYgKHZpcnRwa2cgPT0gTlVMTCkgcmV0dXJuIDA7CiAKIAl2aXJ0cGtnLT5uYW1l ID0gbmFtZTsKLQl2aXJ0cGtnLT52ZXJzaW9uID0gYXBrX2Jsb2JfYXRvbWl6ZShBUEtfQkxP Ql9TVFIodmVyKSk7CisJdmlydHBrZy0+dmVyc2lvbiA9IGFwa19ibG9iX2F0b21pemUoQVBL X0JMT0JfU1RSKHN0cmR1cCh2ZXIpKSk7CiAJdmlydHBrZy0+ZGVzY3JpcHRpb24gPSBzdHJk dXAoInZpcnR1YWwgbWV0YSBwYWNrYWdlIik7CiAJdmlydHBrZy0+YXJjaCA9IGFwa19ibG9i X2F0b21pemUoQVBLX0JMT0JfU1RSKCJub2FyY2giKSk7CiAKLS0gCjIuMjIuMAoK --------------55E8806B56D33F3B38F3BCE4-- --LSWXjUQatMY2DCWZvAMNFcTaxNQFt4Ips-- --Ry3FNGV7w9KwHH8P8ouLIW8RkElUqeunR Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEjNyWOYPU1SaTSMHHyynLUZIrnRQFAl0mkCgACgkQyynLUZIr nRTifg//VuGKgXfl8qeRFi4ohV16ohmGMUePrngGaeRzlpt3cbQkj7rKAWITZgBo 2yYZGoTIi64WREQCs+i6bDXjys/nusJ/eo28MSz8PddaceKYDVVCtLkPFldQ9taM o44/HxdmZfx32zt6YMmuNxIJKYXicpemviJNxSTeu1rk0fLRyPH+bsbVYo88y5Pv 81DmmK3Yt1eDuClHpb0rF4kXMnmlnfDAgWGxWWDyRXJg3abvqRLPze/6bL8rklnq cOGEo7povfukvAeD4kV/qlgJN4xlazuOAjcwmk4YOKUJj/V/wLA+VAyGCLv2KX6x tmfWNXYMMN/0PqE01qrUyHbtRZLiqs/7SNCxncWVS2nDRrUCJCnueLRwOzWXwKQs PMx8oQPJrQFho8KoktqLNYNOCWzHxepyJx/IFVVPnwzIhdNI0v2d6NPf69Vt1q0+ fz8GJZSzzKTC0jO6+i3Y/r0iE9scu8gUtDQWPg8OwVyIcn+NdmoiFxyh/knItIax 0cwWFmZYWzElkPAuCgGTOOU/8KweIbyGOT3NG1GyATs6EJGOebXtoP9RbcUn/LQX EKDK39zKtmitwTQy3KXRJAvUm6TBdF4PaLa3dJ+dURqlXd/o61Tu5wtob8FZhm8E eIuW6D+x+2VRBKXvCf8GP9lFv5jLLiaJ0UNsIF1js9p4e87VvQU= =lz7X -----END PGP SIGNATURE----- --Ry3FNGV7w9KwHH8P8ouLIW8RkElUqeunR--