422623 2020-06-08 10:48:53 +0000 epoll_ctl triggers valgrind warnings for uninitialized padding on non-x86_64 64bit arches 2020-08-18 22:05:26 +0000 1 1 6 Developer tools valgrind general unspecified Other Linux RESOLVED FIXED https://bugzilla.redhat.com/show_bug.cgi?id=1844778 NOR normal --- 1 mark jseward Simon.Richter 0 oldest_to_newest 1935201 0 mark 2020-06-08 10:48:53 +0000 We check the event param with: PRE_MEM_READ( "epoll_ctl(event)", ARG4, sizeof(struct vki_epoll_event) ); But struct vki_epoll_event is defined as: #ifdef __x86_64__ #define VKI_EPOLL_PACKED __attribute__((packed)) #else #define VKI_EPOLL_PACKED #endif struct vki_epoll_event { __vki_u32 events; __vki_u64 data; } VKI_EPOLL_PACKED; Which mimics the kernel headers and means it isn't packed on non-x86_64 arches and might contain padding on such architectures (as it does on arm64). This can be solved with something like the following: diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-l index f1ecbbf..e94c448 100644 --- a/coregrind/m_syswrap/syswrap-linux.c +++ b/coregrind/m_syswrap/syswrap-linux.c @@ -1923,8 +1923,13 @@ PRE(sys_epoll_ctl) SARG1, ( ARG2<3 ? epoll_ctl_s[ARG2] : "?" ), SARG3, ARG4); PRE_REG_READ4(long, "epoll_ctl", int, epfd, int, op, int, fd, struct vki_epoll_event *, event); - if (ARG2 != VKI_EPOLL_CTL_DEL) - PRE_MEM_READ( "epoll_ctl(event)", ARG4, sizeof(struct vki_epoll_event) ); + if (ARG2 != VKI_EPOLL_CTL_DEL) { + struct vki_epoll_event *event = (struct vki_epoll_event *) ARG4; + PRE_MEM_READ( "epoll_ctl(event.events)", (Addr) &event->events, + sizeof(__vki_u32) ); + PRE_MEM_READ( "epoll_ctl(event.data)", (Addr) &event->data, + sizeof(__vki_u64) ); + } } PRE(sys_epoll_wait) But as pointed out in the Fedora bug report (https://bugzilla.redhat.com/show_bug.cgi?id=1844778) even that might not be completely correct since epoll_data_t is a union of both 32- and 64-bit fields and so might only have seen half of the 64-bits be defined. 1941967 1 mark 2020-07-05 21:45:08 +0000 *** Bug 415621 has been marked as a duplicate of this bug. *** 1941970 2 mark 2020-07-05 22:21:15 +0000 (In reply to Mark Wielaard from comment #0) > But as pointed out in the Fedora bug report > (https://bugzilla.redhat.com/show_bug.cgi?id=1844778) even that might not be > completely correct since epoll_data_t is a union of both 32- and 64-bit > fields and so might only have seen half of the 64-bits be defined. This is indeed tricky. The epoll_event data field is user data that the kernel will provide to epoll_[p]wait. Normally one would set data.fd to the fd, otherwise you won't know which file descriptor the event belongs to. But this is not required. Setting just fd would indeed mean only 32bits are initialized (although some example code explicitly does data.u64 = fd). I don't immediately see how we can track whether or not (and how much of the union) the user sets the data field. So I propose for epoll_ctl we only check the events field is initialized and on epoll_[p]wait we assume both the events and data fields are completely initialized (note that epoll_[p]wait also assumes the padding is initialized, I propose we don't). 1946494 3 130417 mark 2020-07-26 20:47:38 +0000 Created attachment 130417 epoll_ctl warns for uninitialized padding on non-amd64 64bit arches Patch as proposed in comment #2 1947845 4 mark 2020-08-01 14:18:18 +0000 commit ecf5ba1197c3833759b40712c401f0bdc768589f Author: Mark Wielaard <mark@klomp.org> Date: Sun Jul 26 22:40:22 2020 +0200 epoll_ctl warns for uninitialized padding on non-amd64 64bit arches struct vki_epoll_event is packed on x86_64, but not on other 64bit arches. This means that on 64bit arches there can be padding in the epoll_event struct. Seperately the data field is only used by user space (which might not set the data field if it doesn't need to). Only check the events field on epoll_ctl. But assume both events and data are both written to by epoll_[p]wait (exclude padding). https://bugs.kde.org/show_bug.cgi?id=422623 1951629 5 mark 2020-08-18 22:05:26 +0000 commit b74f9f23c8758c77367f18368ea95baa858544cb Author: Mark Wielaard <mark@klomp.org> Date: Tue Aug 18 23:58:55 2020 +0200 Fix epoll_ctl setting of array event and data fields. Fix for https://bugs.kde.org/show_bug.cgi?id=422623 in commit ecf5ba119 epoll_ctl warns for uninitialized padding on non-amd64 64bit arches contained a bug. A pointer to an array is not a pointer to a pointer to an array. Found by a Fedora user: https://bugzilla.redhat.com/show_bug.cgi?id=1844778#c10 130417 2020-07-26 20:47:38 +0000 2020-07-26 20:47:38 +0000 epoll_ctl warns for uninitialized padding on non-amd64 64bit arches 0001-epoll_ctl-warns-for-uninitialized-padding-on-non-amd.patch text/plain 3980 mark RnJvbSBmMzI2ZDY4ZDc2MmVkZjRiMGU5NjA0ZGFhNDQ2YjZmOGNhMjU3MjVhIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNYXJrIFdpZWxhYXJkIDxtYXJrQGtsb21wLm9yZz4KRGF0ZTog U3VuLCAyNiBKdWwgMjAyMCAyMjo0MDoyMiArMDIwMApTdWJqZWN0OiBbUEFUQ0hdIGVwb2xsX2N0 bCB3YXJucyBmb3IgdW5pbml0aWFsaXplZCBwYWRkaW5nIG9uIG5vbi1hbWQ2NCA2NGJpdAogYXJj aGVzCgpzdHJ1Y3QgdmtpX2Vwb2xsX2V2ZW50IGlzIHBhY2tlZCBvbiB4ODZfNjQsIGJ1dCBub3Qg b24gb3RoZXIgNjRiaXQKYXJjaGVzLiBUaGlzIG1lYW5zIHRoYXQgb24gNjRiaXQgYXJjaGVzIHRo ZXJlIGNhbiBiZSBwYWRkaW5nIGluIHRoZQplcG9sbF9ldmVudCBzdHJ1Y3QuIFNlcGVyYXRlbHkg dGhlIGRhdGEgZmllbGQgaXMgb25seSB1c2VkIGJ5IHVzZXIKc3BhY2UgKHdoaWNoIG1pZ2h0IG5v dCBzZXQgdGhlIGRhdGEgZmllbGQgaWYgaXQgZG9lc24ndCBuZWVkIHRvKS4KCk9ubHkgY2hlY2sg dGhlIGV2ZW50cyBmaWVsZCBvbiBlcG9sbF9jdGwuIEJ1dCBhc3N1bWUgYm90aCBldmVudHMKYW5k IGRhdGEgYXJlIGJvdGggd3JpdHRlbiB0byBieSBlcG9sbF9bcF13YWl0IChleGNsdWRlIHBhZGRp bmcpLgoKaHR0cHM6Ly9idWdzLmtkZS5vcmcvc2hvd19idWcuY2dpP2lkPTQyMjYyMwotLS0KIE5F V1MgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHwgIDEgKwogY29yZWdyaW5kL21fc3lz d3JhcC9zeXN3cmFwLWxpbnV4LmMgfCAzNSArKysrKysrKysrKysrKysrKysrKysrLS0tLS0tLQog MiBmaWxlcyBjaGFuZ2VkLCAyOCBpbnNlcnRpb25zKCspLCA4IGRlbGV0aW9ucygtKQoKZGlmZiAt LWdpdCBhL05FV1MgYi9ORVdTCmluZGV4IDBmZmNiNWJmOS4uMDQzMTFmZjBjIDEwMDY0NAotLS0g YS9ORVdTCisrKyBiL05FV1MKQEAgLTQyLDYgKzQyLDcgQEAgd2hlcmUgWFhYWFhYIGlzIHRoZSBi dWcgbnVtYmVyIGFzIGxpc3RlZCBiZWxvdy4KIDM0NTA3NyAgbGludXggc3lzY2FsbCBleGVjdmVh dCBzdXBwb3J0IChsaW51eCAzLjE5KQogbi1pLWJ6ICBoZWxncmluZDogSWYgaGdfY2xpX19yZWFs bG9jIGZhaWxzLCByZXR1cm4gTlVMTC4KIAorNDIyNjIzICBlcG9sbF9jdGwgd2FybnMgZm9yIHVu aW5pdGlhbGl6ZWQgcGFkZGluZyBvbiBub24tYW1kNjQgNjRiaXQgYXJjaGVzCiA0MjMwMjEgIFBQ QzogIEFkZCBtaXNzaW5nIElTQSAzLjAgZG9jdW1lbnRhdGlvbiBsaW5rIGFuZCBIV0NBUFMgdGVz dC4KIAogCmRpZmYgLS1naXQgYS9jb3JlZ3JpbmQvbV9zeXN3cmFwL3N5c3dyYXAtbGludXguYyBi L2NvcmVncmluZC9tX3N5c3dyYXAvc3lzd3JhcC1saW51eC5jCmluZGV4IDViNWI3ZWVlNi4uOTI5 YTRkOWFmIDEwMDY0NAotLS0gYS9jb3JlZ3JpbmQvbV9zeXN3cmFwL3N5c3dyYXAtbGludXguYwor KysgYi9jb3JlZ3JpbmQvbV9zeXN3cmFwL3N5c3dyYXAtbGludXguYwpAQCAtMjA5OSw4ICsyMDk5 LDI5IEBAIFBSRShzeXNfZXBvbGxfY3RsKQogICAgICAgICAgU0FSRzEsICggQVJHMjwzID8gZXBv bGxfY3RsX3NbQVJHMl0gOiAiPyIgKSwgU0FSRzMsIEFSRzQpOwogICAgUFJFX1JFR19SRUFENChs b25nLCAiZXBvbGxfY3RsIiwKICAgICAgICAgICAgICAgICAgaW50LCBlcGZkLCBpbnQsIG9wLCBp bnQsIGZkLCBzdHJ1Y3QgdmtpX2Vwb2xsX2V2ZW50ICosIGV2ZW50KTsKLSAgIGlmIChBUkcyICE9 IFZLSV9FUE9MTF9DVExfREVMKQotICAgICAgUFJFX01FTV9SRUFEKCAiZXBvbGxfY3RsKGV2ZW50 KSIsIEFSRzQsIHNpemVvZihzdHJ1Y3QgdmtpX2Vwb2xsX2V2ZW50KSApOworICAgaWYgKEFSRzIg IT0gVktJX0VQT0xMX0NUTF9ERUwpIHsKKyAgICAgIC8qIEp1c3QgY2hlY2sgdGhlIGV2ZW50cyBm aWVsZCwgdGhlIGRhdGEgZmllbGQgaXMgZm9yIHVzZXIgc3BhY2UgYW5kCisgICAgICAgICB1bnVz ZWQgYnkgdGhlIGtlcm5lbC4gICovCisgICAgICBzdHJ1Y3QgdmtpX2Vwb2xsX2V2ZW50ICpldmVu dCA9IChzdHJ1Y3QgdmtpX2Vwb2xsX2V2ZW50ICopIEFSRzQ7CisgICAgICBQUkVfTUVNX1JFQUQo ICJlcG9sbF9jdGwoZXZlbnQpIiwgKEFkZHIpICZldmVudC0+ZXZlbnRzLAorICAgICAgICAgICAg ICAgICAgICBzaXplb2YoX192a2lfdTMyKSApOworICAgfQorfQorCisvKiBSRVMgZXZlbnQgcmVj b3JkcyBoYXZlIGJlZW4gd3JpdHRlbiAoZXhjbHVkZSBwYWRkaW5nKS4gICovCitzdGF0aWMgdm9p ZCBlcG9sbF9wb3N0X2hlbHBlciAoIFRocmVhZElkIHRpZCwgU3lzY2FsbEFyZ3MqIGFycmdocywK KyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgU3lzY2FsbFN0YXR1cyogc3RhdHVzICkK K3sKKyAgIHZnX2Fzc2VydChTVUNDRVNTKTsKKyAgIGlmIChSRVMgPiAwKSB7CisgICAgICBJbnQg aTsKKyAgICAgIHN0cnVjdCB2a2lfZXBvbGxfZXZlbnQgKipldmVudHMgPSAoc3RydWN0IHZraV9l cG9sbF9ldmVudCoqKShBZGRyKUFSRzI7CisgICAgICBmb3IgKGkgPSAwOyBpIDwgUkVTOyBpKysp IHsKKyAgICAgICAgIC8qIEFzc3VtZSBib3RoIGV2ZW50cyBhbmQgZGF0YSBhcmUgc2V0IChkYXRh IGlzIHVzZXIgc3BhY2Ugb25seSkuICovCisgICAgICAgICBQT1NUX0ZJRUxEX1dSSVRFKGV2ZW50 c1tpXS0+ZXZlbnRzKTsKKyAgICAgICAgIFBPU1RfRklFTERfV1JJVEUoZXZlbnRzW2ldLT5kYXRh KTsKKyAgICAgIH0KKyAgIH0KIH0KIAogUFJFKHN5c19lcG9sbF93YWl0KQpAQCAtMjExMSwxMyAr MjEzMiwxMiBAQCBQUkUoc3lzX2Vwb2xsX3dhaXQpCiAgICBQUkVfUkVHX1JFQUQ0KGxvbmcsICJl cG9sbF93YWl0IiwKICAgICAgICAgICAgICAgICAgaW50LCBlcGZkLCBzdHJ1Y3QgdmtpX2Vwb2xs X2V2ZW50ICosIGV2ZW50cywKICAgICAgICAgICAgICAgICAgaW50LCBtYXhldmVudHMsIGludCwg dGltZW91dCk7CisgICAvKiBBc3N1bWUgYWxsIChtYXhldmVudHMpIGV2ZW50cyByZWNvcmRzIHNo b3VsZCBiZSAoZnVsbHkpIHdyaXRhYmxlLiAqLwogICAgUFJFX01FTV9XUklURSggImVwb2xsX3dh aXQoZXZlbnRzKSIsIEFSRzIsIHNpemVvZihzdHJ1Y3QgdmtpX2Vwb2xsX2V2ZW50KSpBUkczKTsK IH0KIFBPU1Qoc3lzX2Vwb2xsX3dhaXQpCiB7Ci0gICB2Z19hc3NlcnQoU1VDQ0VTUyk7Ci0gICBp ZiAoUkVTID4gMCkKLSAgICAgIFBPU1RfTUVNX1dSSVRFKCBBUkcyLCBzaXplb2Yoc3RydWN0IHZr aV9lcG9sbF9ldmVudCkqUkVTICkgOworICAgZXBvbGxfcG9zdF9oZWxwZXIgKHRpZCwgYXJyZ2hz LCBzdGF0dXMpOwogfQogCiBQUkUoc3lzX2Vwb2xsX3B3YWl0KQpAQCAtMjEzMCwxNSArMjE1MCwx NCBAQCBQUkUoc3lzX2Vwb2xsX3B3YWl0KQogICAgICAgICAgICAgICAgICBpbnQsIGVwZmQsIHN0 cnVjdCB2a2lfZXBvbGxfZXZlbnQgKiwgZXZlbnRzLAogICAgICAgICAgICAgICAgICBpbnQsIG1h eGV2ZW50cywgaW50LCB0aW1lb3V0LCB2a2lfc2lnc2V0X3QgKiwgc2lnbWFzaywKICAgICAgICAg ICAgICAgICAgdmtpX3NpemVfdCwgc2lnc2V0c2l6ZSk7CisgICAvKiBBc3N1bWUgYWxsIChtYXhl dmVudHMpIGV2ZW50cyByZWNvcmRzIHNob3VsZCBiZSAoZnVsbHkpIHdyaXRhYmxlLiAqLwogICAg UFJFX01FTV9XUklURSggImVwb2xsX3B3YWl0KGV2ZW50cykiLCBBUkcyLCBzaXplb2Yoc3RydWN0 IHZraV9lcG9sbF9ldmVudCkqQVJHMyk7CiAgICBpZiAoQVJHNSkKICAgICAgIFBSRV9NRU1fUkVB RCggImVwb2xsX3B3YWl0KHNpZ21hc2spIiwgQVJHNSwgc2l6ZW9mKHZraV9zaWdzZXRfdCkgKTsK IH0KIFBPU1Qoc3lzX2Vwb2xsX3B3YWl0KQogewotICAgdmdfYXNzZXJ0KFNVQ0NFU1MpOwotICAg aWYgKFJFUyA+IDApCi0gICAgICBQT1NUX01FTV9XUklURSggQVJHMiwgc2l6ZW9mKHN0cnVjdCB2 a2lfZXBvbGxfZXZlbnQpKlJFUyApIDsKKyAgIGVwb2xsX3Bvc3RfaGVscGVyICh0aWQsIGFycmdo cywgc3RhdHVzKTsKIH0KIAogUFJFKHN5c19ldmVudGZkKQotLSAKMi4xOC40Cgo=