From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============1088036526705681648==" MIME-Version: 1.0 From: Mark Wielaard To: elfutils-devel@lists.fedorahosted.org Subject: Re: [PATCH] Fix section corruption bug Date: Thu, 12 Jun 2014 14:30:52 +0200 Message-ID: <1402576252.3940.95.camel@bordewijk.wildebeest.org> In-Reply-To: 201406101531.09654.thilo@tjps.eu --===============1088036526705681648== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On Tue, 2014-06-10 at 15:31 +0200, Thilo Schulz wrote: > On Tuesday 10 June 2014 11:48:15 Mark Wielaard wrote: > > On Mon, 2014-06-09 at 21:05 +0200, Thilo Schulz wrote: > > > When adding data to existing sections in ELF files, libelf may corrupt > > > those sections, i.e. overwrite the existing data if certain conditions > > > are met. > > > = > > > If an Elf_Scn structure has seen a call to elf_rawdata(scn) before bu= t no > > > call to elf_getdata(scn), scn->read_data flag is set, but not > > > scn->data_list_rear. > > = > > Do you happen to have a small testcase that shows the buggy behavior? > = > Sure. This is an excerpt from the final program. Thanks. It think that shows the second bug you describe. It helped me write a specific test case for both issues (attached). > > I was wondering whether we want to check scn->rawdata.s directly, or if > > we could rely on ELF_F_FILEDATA being set for scn->flags? > = > Seems reasonable though I don't know the code as well as you do I guess. I wish I understood the code very well :) But now that I wrote the testcase and you pointed out the second bug, I am not sure of the fix anymore. It does seem to fix the first issue, but then you immediately hit the second. > As a further note: A similar bug, albeit for slightly different reasons, = occurs = > when adding relocations. Adding a relocation with elf_newdata() then = > elf_update() = > results in the old data being "forgotten" if there was no elf_getdata() c= all = > before to load that data into memory. The cause is a bit different becaus= e in = > this case, there was not a call to elf_rawdata() before and this still = > happened. I imagine, this might also be a problem for string tables. Indeed. The attached testcase shows both issues. Calling elf_getdata() and then elf_newdata() works as expected. But elf_newdata drops all existing data when elf_rawdata is called before and elf_newdata keeps the size, but not the actual content bytes of existing data of a section if elf_getdata isn't called before. Still scratching my head a little how to resolve both issues properly. Thanks, Mark --===============1088036526705681648== Content-Type: text/x-csrc MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="newdata.c" LyogVGVzdCBwcm9ncmFtIGZvciBlbGZfbmV3ZGF0YS4KICAgQ29weXJpZ2h0IChDKSAyMDE0IFJl ZCBIYXQsIEluYy4KICAgVGhpcyBmaWxlIGlzIHBhcnQgb2YgZWxmdXRpbHMuCgogICBUaGlzIGZp bGUgaXMgZnJlZSBzb2Z0d2FyZTsgeW91IGNhbiByZWRpc3RyaWJ1dGUgaXQgYW5kL29yIG1vZGlm eQogICBpdCB1bmRlciB0aGUgdGVybXMgb2YgdGhlIEdOVSBHZW5lcmFsIFB1YmxpYyBMaWNlbnNl IGFzIHB1Ymxpc2hlZCBieQogICB0aGUgRnJlZSBTb2Z0d2FyZSBGb3VuZGF0aW9uOyBlaXRoZXIg dmVyc2lvbiAzIG9mIHRoZSBMaWNlbnNlLCBvcgogICAoYXQgeW91ciBvcHRpb24pIGFueSBsYXRl ciB2ZXJzaW9uLgoKICAgZWxmdXRpbHMgaXMgZGlzdHJpYnV0ZWQgaW4gdGhlIGhvcGUgdGhhdCBp dCB3aWxsIGJlIHVzZWZ1bCwgYnV0CiAgIFdJVEhPVVQgQU5ZIFdBUlJBTlRZOyB3aXRob3V0IGV2 ZW4gdGhlIGltcGxpZWQgd2FycmFudHkgb2YKICAgTUVSQ0hBTlRBQklMSVRZIG9yIEZJVE5FU1Mg Rk9SIEEgUEFSVElDVUxBUiBQVVJQT1NFLiAgU2VlIHRoZQogICBHTlUgR2VuZXJhbCBQdWJsaWMg TGljZW5zZSBmb3IgbW9yZSBkZXRhaWxzLgoKICAgWW91IHNob3VsZCBoYXZlIHJlY2VpdmVkIGEg Y29weSBvZiB0aGUgR05VIEdlbmVyYWwgUHVibGljIExpY2Vuc2UKICAgYWxvbmcgd2l0aCB0aGlz IHByb2dyYW0uICBJZiBub3QsIHNlZSA8aHR0cDovL3d3dy5nbnUub3JnL2xpY2Vuc2VzLz4uICAq LwoKI2luY2x1ZGUgPGVycm5vLmg+CiNpbmNsdWRlIDxlcnJvci5oPgojaW5jbHVkZSA8c3RkaW8u aD4KI2luY2x1ZGUgPHN0ZGJvb2wuaD4KI2luY2x1ZGUgPHN0ZGxpYi5oPgojaW5jbHVkZSA8c3Ry aW5nLmg+CiNpbmNsdWRlIDxzeXMvdHlwZXMuaD4KI2luY2x1ZGUgPHN5cy9zdGF0Lmg+CiNpbmNs dWRlIDxmY250bC5oPgojaW5jbHVkZSA8dW5pc3RkLmg+CiNpbmNsdWRlIDxsaWJlbGYuaD4KCi8q IEFkZHMgYSBuZXcgIkhlbGxvIFdvcmxkIiBzdHJpbmcgdG8gdGhlIHNlY3Rpb24uICAqLwpzdGF0 aWMgdm9pZAphZGRfZGF0YSAoRWxmX1NjbiAqc2NuKQp7CiAgcHJpbnRmICgiQWRkIEhlbGxvIFdv cmxkXG4iKTsKCiAgRWxmX0RhdGEgKmRhdGEgPSBlbGZfbmV3ZGF0YSAoc2NuKTsKICBpZiAoZGF0 YSA9PSBOVUxMKQogICAgZXJyb3IgKDEsIDAsICJlbGZfbmV3ZGF0YTogJXMiLCBlbGZfZXJybXNn ICgtMSkpOwoKICBkYXRhLT5kX2FsaWduID0gMTsKICBkYXRhLT5kX2J1ZiA9ICJIZWxsbyBXb3Js ZCI7CiAgZGF0YS0+ZF90eXBlID0gRUxGX1RfQllURTsKICBkYXRhLT5kX3NpemUgPSBzaXplb2Yg KCJIZWxsbyBXb3JsZCIpOwogIGRhdGEtPmRfdmVyc2lvbiA9IEVWX0NVUlJFTlQ7Cn0KCi8qIENy ZWF0ZXMgYSBuZXcgbWluaW1hbCBFTEYgZmlsZSB3aXRoIGEgLmRhdGEgc2VjdGlvbiBjb250YWlu aW5nCiAgICJIZWxsbyBXb3JsZCIuICBSZXR1cm5zIHRoZSBuYW1lIG9mIHRoZSBuZXcgZmlsZS4g ICovCnN0YXRpYyBjb25zdCBjaGFyICoKY3JlYXRlX2VsZiAoKQp7CiAgRWxmICplbGY7CiAgaW50 IGZkOwoKICBzdGF0aWMgY2hhciBuYW1lW10gPSAidGVzdC5YWFhYWFgiOwogIGZkID0gbWtzdGVt cCAobmFtZSk7CiAgaWYgKGZkIDwgMCkKICAgIGVycm9yICgxLCBlcnJubywgIm1rc3RlbXAiKTsK CiAgZWxmID0gZWxmX2JlZ2luIChmZCwgRUxGX0NfV1JJVEUsIE5VTEwpOwogIGlmIChlbGYgPT0g TlVMTCkKICAgIGVycm9yICgxLCAwLCAiZWxmYmVnaW46ICVzIiwgZWxmX2Vycm1zZyAoLTEpKTsK CiAgRWxmMzJfRWhkciAqZWhkciA9IGVsZjMyX25ld2VoZHIgKGVsZik7CiAgaWYgKGVoZHIgPT0g TlVMTCkKICAgIGVycm9yICgxLCAwLCAiZWxmMzJfbmV3ZWhkcjogJXMiLCBlbGZfZXJybXNnICgt MSkpOwoKICAvKiBPdXIgZGF0YS4gICovCiAgRWxmX1NjbiAqc2NuID0gZWxmX25ld3NjbiAoZWxm KTsKICBpZiAoc2NuID09IE5VTEwpCiAgICBlcnJvciAoMSwgMCwgImVsZl9uZXdzY246ICVzIiwg ZWxmX2Vycm1zZyAoLTEpKTsKCiAgYWRkX2RhdGEgKHNjbik7CgogIEVsZjMyX1NoZHIgKnNoZHIg PSBlbGYzMl9nZXRzaGRyKHNjbik7CiAgaWYgKHNoZHIgPT0gTlVMTCkKICAgIGVycm9yICgxLCAw LCAiZWxmX2dldHNoZHI6ICVzIiwgZWxmX2Vycm1zZyAoLTEpKTsKCiAgc2hkci0+c2hfbmFtZSA9 IDE7IC8qIC5kYXRhICovCiAgc2hkci0+c2hfdHlwZSA9IFNIVF9QUk9HQklUUzsKICBzaGRyLT5z aF9mbGFncyA9IFNIRl9TVFJJTkdTOwogIHNoZHItPnNoX2VudHNpemUgPSAwOwoKICAvKiBUaGUg c2VjdGlvbiBzdHJpbmcgdGFibGUuICAqLwogIHNjbiA9IGVsZl9uZXdzY24gKGVsZik7CiAgaWYg KHNjbiA9PSBOVUxMKQogICAgZXJyb3IgKDEsIDAsICJlbGZfbmV3c2NuOiAlcyIsIGVsZl9lcnJt c2cgKC0xKSk7CgogIEVsZl9EYXRhICpkYXRhID0gZWxmX25ld2RhdGEgKHNjbik7CiAgaWYgKGRh dGEgPT0gTlVMTCkKICAgIGVycm9yICgxLCAwLCAiZWxmX25ld2RhdGE6ICVzIiwgZWxmX2Vycm1z ZyAoLTEpKTsKCiAgLy8gUXVpY2sgYW5kIGRpcnR5IHN0cmluZyB0YWJsZSBmb3Igc2VjdGlvbiB6 ZXJvLCBvdXIgbmV3IGRhdGEgc2VjdGlvbiwKICAvLyBhbmQgdGhpcyBzaHN0cnRhYiBzZWN0aW9u IGl0c2VsZi4KICBjaGFyIHN0cmluZ190YWJsZVtdID0geyAnXDAnLAoJCQkgICcuJywgJ2QnLCAn YScsICd0JywgJ2EnLCAnXDAnLAoJCQkgICcuJywgJ3MnLCAnaCcsICdzJywgJ3QnLCAncicsICd0 JywgJ2EnLCAnYicsICdcMCcgfTsKCiAgZGF0YS0+ZF9hbGlnbiA9IDE7CiAgZGF0YS0+ZF9vZmYg PSAwOwogIGRhdGEtPmRfYnVmID0gc3RyaW5nX3RhYmxlOwogIGRhdGEtPmRfdHlwZSA9IEVMRl9U X0JZVEU7CiAgZGF0YS0+ZF9zaXplID0gc2l6ZW9mIChzdHJpbmdfdGFibGUpOwogIGRhdGEtPmRf dmVyc2lvbiA9IEVWX0NVUlJFTlQ7CgogIHNoZHIgPSBlbGYzMl9nZXRzaGRyKHNjbik7CiAgaWYg KHNoZHIgPT0gTlVMTCkKICAgIGVycm9yICgxLCAwLCAiZWxmX2dldHNoZHI6ICVzIiwgZWxmX2Vy cm1zZyAoLTEpKTsKCiAgc2hkci0+c2hfbmFtZSA9IDc7IC8qIC5zaHN0cnRhYiAqLwogIHNoZHIt PnNoX3R5cGUgPSBTSFRfU1RSVEFCOwogIHNoZHItPnNoX2ZsYWdzID0gU0hGX1NUUklOR1M7CiAg c2hkci0+c2hfZW50c2l6ZSA9IDA7CgogIHNpemVfdCBuZHggPSBlbGZfbmR4c2NuIChzY24pOwog IGVoZHItPmVfc2hzdHJuZHggPSBuZHg7CiAgZWhkci0+ZV92ZXJzaW9uID0gRVZfQ1VSUkVOVDsK CiAgLyogV3JpdGUgaXQgYWxsIG91dC4gKi8KICBpZiAoZWxmX3VwZGF0ZSAoZWxmLCBFTEZfQ19X UklURSkgPCAwKQogICAgZXJyb3IgKDEsIDAsICJlbGZfdXBkYXRlOiAlcyIsIGVsZl9lcnJtc2cg KC0xKSk7CgogIGlmIChlbGZfZW5kIChlbGYpIDwgMCkKICAgIGVycm9yICgxLCAwLCAiZWxmX2Vu ZDogJXMiLCBlbGZfZXJybXNnICgtMSkpOwoKICBpZiAoY2xvc2UgKGZkKSA8IDApCiAgICBlcnJv ciAoMSwgZXJybm8sICJjbG9zZSIpOwoKICByZXR1cm4gbmFtZTsKfQoKLyogT3BlbnMgYW4gZXhp c3RpbmcgRUxGIGZpbGUsIGNoZWNrcyB0aGF0IGl0IGhhcyBhIC5kYXRhIHNlY3Rpb24sIGlmCiAg IHJlYWQgaXMgdHJ1ZSBjaGVja3MgdGhhdCBpdCBjb250YWlucyBvbmUgb3IgbW9yZSAiSGVsbG8g V29ybGQiCiAgIHN0cmluZ3MsIGFuZCBhZGRzIGFuIGV4dHJhICJIZWxsbyBXb3JsZCIgdG8gdGhh dCBzZWN0aW9uLiAgR2V0cyB0aGUKICAgc2VjdGlvbiBkYXRhIGVpdGhlciB0aHJvdWdoIGVsZl9n ZXRkYXRhICgpIG9yIGVsZl9yYXdkYXRhICgpIGJhc2VkCiAgIG9uIHRoZSBsYXN0IGFyZ3VtZW50 LiAgKi8Kc3RhdGljIHZvaWQKbW9kaWZ5X2VsZiAoY29uc3QgY2hhciAqbmFtZSwgYm9vbCByZWFk LCBib29sIHJhdykKewogIGludCBmZCA9IG9wZW4gKG5hbWUsIE9fUkRXUik7CiAgaWYgKGZkIDwg MCkKICAgIGVycm9yICgxLCBlcnJubywgIm9wZW4gJyVzJyIsIG5hbWUpOwoKICBFbGYgKmVsZiA9 IGVsZl9iZWdpbiAoZmQsIEVMRl9DX1JEV1IsIE5VTEwpOwogIGlmIChlbGYgPT0gTlVMTCkKICAg IGVycm9yICgxLCAwLCAiZWxmX2JlZ2luOiAlcyIsIGVsZl9lcnJtc2cgKC0xKSk7CgogIC8qIEdl dCB0aGUgc2VjdGlvbiBoZWFkZXIgc3RyaW5nIHRhYmxlIGluZGV4LiAgKi8KICBzaXplX3QgbmR4 OwogIGlmIChlbGZfZ2V0c2hkcnN0cm5keCAoZWxmLCAmbmR4KSA8IDApCiAgICBlcnJvciAoMSwg MCwgImVsZl9nZXRzaGRyc3RybmR4OiAlcyIsIGVsZl9lcnJtc2cgKC0xKSk7CgogIC8qIEdldCB0 aGUgLmRhdGEgc2VjdGlvbi4gKi8KICBFbGZfU2NuICpzY24gPSBOVUxMOwogIEVsZjMyX1NoZHIg KnNoZHIgPSBOVUxMOwogIHdoaWxlICgoc2NuID0gZWxmX25leHRzY24gKGVsZiwgc2NuKSkgIT0g TlVMTCkKICAgIHsKICAgICAgc2hkciA9IGVsZjMyX2dldHNoZHIgKHNjbik7CiAgICAgIGlmIChz aGRyICE9IE5VTEwgJiYgc2hkci0+c2hfdHlwZSA9PSBTSFRfUFJPR0JJVFMpCgl7CgkgIGNvbnN0 IGNoYXIgKnNuYW1lID0gZWxmX3N0cnB0ciAoZWxmLCBuZHgsIHNoZHItPnNoX25hbWUpOwoJICBp ZiAoc25hbWUgIT0gTlVMTCAmJiBzdHJjbXAgKHNuYW1lLCAiLmRhdGEiKSA9PSAwKQoJICAgIGJy ZWFrOwoJfQogICAgfQoKICBpZiAoc2NuID09IE5VTEwpCiAgICBlcnJvciAoMSwgMCwgIk5vIFBS T0dCSVRTIC5kYXRhIHNlY3Rpb24uIik7CgogIGlmIChyZWFkKQogICAgewogICAgICBFbGZfRGF0 YSAqZGF0YTsKICAgICAgaWYgKHJhdykKCXsKCSAgZGF0YSAgPSBlbGZfcmF3ZGF0YSAoc2NuLCBO VUxMKTsKCSAgaWYgKGRhdGEgPT0gTlVMTCkKCSAgICBlcnJvciAoMSwgMCwgImVsZl9yYXdkYXRh OiAlcyIsIGVsZl9lcnJtc2cgKC0xKSk7Cgl9CiAgICAgIGVsc2UKCXsKCSAgZGF0YSAgPSBlbGZf Z2V0ZGF0YSAoc2NuLCBOVUxMKTsKCSAgaWYgKGRhdGEgPT0gTlVMTCkKCSAgICBlcnJvciAoMSwg MCwgImVsZl9nZXRkYXRhOiAlcyIsIGVsZl9lcnJtc2cgKC0xKSk7Cgl9CgogICAgICAvKiBJcyBp dCBhbGwgSGVsbG8gV29ybGQ/ICAqLwogICAgICBwcmludGYgKCJkYXRhIHNpemU6ICV6ZFxuIiwg ZGF0YS0+ZF9zaXplKTsKICAgICAgaW50IG5yID0gZGF0YS0+ZF9zaXplIC8gc2l6ZW9mICgiSGVs bG8gV29ybGQiKTsKICAgICAgcHJpbnRmICgic3RyaW5nczogJWRcbiIsIG5yKTsKICAgICAgY2hh ciAqc3RyID0gZGF0YS0+ZF9idWY7CiAgICAgIGZvciAoaW50IGkgPSAwOyBpIDwgbnI7IGkrKykK CXsKCSAgcHJpbnRmICgiJXNcbiIsIChjaGFyICopIHN0cik7CgkgIHN0ciArPSBzaXplb2YgKCJI ZWxsbyBXb3JsZCIpOwoJfQogICAgfQoKICAvKiBBbmQgYWRkIG9uZSBtb3JlIHN0cmluZy4uLiAq LwogIGFkZF9kYXRhIChzY24pOwoKICAvKiBXcml0ZSBpdCBhbGwgb3V0LiAqLwogIGlmIChlbGZf dXBkYXRlIChlbGYsIEVMRl9DX1dSSVRFKSA8IDApCiAgICBlcnJvciAoMSwgMCwgImVsZl91cGRh dGU6ICVzIiwgZWxmX2Vycm1zZyAoLTEpKTsKCiAgaWYgKGVsZl9lbmQgKGVsZikgPCAwKQogICAg ZXJyb3IgKDEsIDAsICJlbGZfZW5kOiAlcyIsIGVsZl9lcnJtc2cgKC0xKSk7CgogIGlmIChjbG9z ZSAoZmQpIDwgMCkKICAgIGVycm9yICgxLCBlcnJubywgImNsb3NlIik7Cn0KCmludAptYWluIChp bnQgYXJnYywgY2hhciAqKmFyZ3YpCnsKICAvLyBJbml0aWFsaXplIGxpYmVsZi4KICBpZiAoZWxm X3ZlcnNpb24gKEVWX0NVUlJFTlQpID09IEVWX05PTkUpCiAgICBlcnJvciAoMSwgMCwgImVsZl92 ZXJzaW9uOiAlcyIsIGVsZl9lcnJtc2cgKC0xKSk7CgogIGNvbnN0IGNoYXIgKm5hbWUgPSBjcmVh dGVfZWxmICgpOwogIHByaW50ZiAoIm5hbWU6ICVzXG4iLCBuYW1lKTsKICBtb2RpZnlfZWxmIChu YW1lLCBmYWxzZSwgZmFsc2UpOyAvKiBEb24ndCByZWFkLCBqdXN0IGFkZC4gICAqLwogIG1vZGlm eV9lbGYgKG5hbWUsIHRydWUsIGZhbHNlKTsgIC8qIFJlYWQgd2l0aCBlbGZfZ2V0ZGF0YS4gICov CiAgbW9kaWZ5X2VsZiAobmFtZSwgdHJ1ZSwgdHJ1ZSk7ICAgLyogUmVhZCB3aXRoIGVsZl9yYXdk YXRhLiAgKi8KICBtb2RpZnlfZWxmIChuYW1lLCB0cnVlLCBmYWxzZSk7ICAvKiBSZWFkIHdpdGgg ZWxmX2dldGRhdGEgYWdhaW4uICAqLwoKICByZXR1cm4gMDsKfQo= --===============1088036526705681648==--