189741 2018-09-19 03:04:55 -0700 [Linux] Use memfd_create when available in SharedMemory implementation 2018-11-07 06:25:49 -0800 1 1 1 Unclassified WebKit WebKit2 WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=188568 Gtk, InRadar P2 Normal --- 1 cgarcia webkit-unassigned mcatanzaro pgriffis webkit-bug-importer oldest_to_newest 1460899 0 cgarcia 2018-09-19 03:04:55 -0700 Instead of shm_open. 1460901 1 350101 cgarcia 2018-09-19 03:08:22 -0700 Created attachment 350101 Patch 1460942 2 350101 mcatanzaro 2018-09-19 06:59:25 -0700 Comment on attachment 350101 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350101&action=review In this case, I would check for sys/memfd.h instead of linux/memfd.h and use memfd_create() directly if it's available (glibc 2.27 or newer) rather than using syscall(). Then you don't have to check for EINTR or ENOSYS. This way works too, of course. > Source/WebKit/Platform/unix/SharedMemoryUnix.cpp:128 > + if (errno == ENOSYS) > + return fileDescriptor; Surely this isn't right? You should just remove this condition and fall through to isMemFdAvailable = false, right? > Source/WebKit/Platform/unix/SharedMemoryUnix.cpp:154 > - WTFLogAlways("Failed to create shared memory file %s: %s", tempName.data(), strerror(errno)); > + WTFLogAlways("Failed to create shared memory file: %s", strerror(errno)); Drop the word "file" 1461258 3 cgarcia 2018-09-20 00:04:23 -0700 (In reply to Michael Catanzaro from comment #2) > Comment on attachment 350101 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=350101&action=review > > In this case, I would check for sys/memfd.h instead of linux/memfd.h and use > memfd_create() directly if it's available (glibc 2.27 or newer) rather than > using syscall(). Then you don't have to check for EINTR or ENOSYS. > > This way works too, of course. I'm assuming this is available in more systems, no? > > Source/WebKit/Platform/unix/SharedMemoryUnix.cpp:128 > > + if (errno == ENOSYS) > > + return fileDescriptor; > > Surely this isn't right? You should just remove this condition and fall > through to isMemFdAvailable = false, right? Yes, I changed this several times and ended up with the wrong check. Good catch! > > Source/WebKit/Platform/unix/SharedMemoryUnix.cpp:154 > > - WTFLogAlways("Failed to create shared memory file %s: %s", tempName.data(), strerror(errno)); > > + WTFLogAlways("Failed to create shared memory file: %s", strerror(errno)); > > Drop the word "file" Sure. 1461369 4 mcatanzaro 2018-09-20 09:11:38 -0700 (In reply to Carlos Garcia Campos from comment #3) > I'm assuming this is available in more systems, no? The syscall was added years before the glibc interface, but so what? It's nicer to use the glibc function, and there is a fallback, after all. syscall() is used in the sandbox code because it's really needed there, but here it's not: there is a fallback. 1462310 5 cgarcia 2018-09-22 00:54:35 -0700 (In reply to Michael Catanzaro from comment #4) > (In reply to Carlos Garcia Campos from comment #3) > > I'm assuming this is available in more systems, no? > > The syscall was added years before the glibc interface, but so what? It's > nicer to use the glibc function, and there is a fallback, after all. > syscall() is used in the sandbox code because it's really needed there, but > here it's not: there is a fallback. I still don't understand why sandbox code can't use SharedMemory. 1475967 6 cgarcia 2018-11-07 06:24:17 -0800 Committed r237922: <https://trac.webkit.org/changeset/237922> 1475969 7 webkit-bug-importer 2018-11-07 06:25:49 -0800 <rdar://problem/45873944> 350101 2018-09-19 03:08:22 -0700 2018-09-19 06:59:25 -0700 Patch wk2-memfd.diff text/plain 5123 cgarcia ZGlmZiAtLWdpdCBhL0NoYW5nZUxvZyBiL0NoYW5nZUxvZwppbmRleCBhMzU3MjRiYjAxNi4uZjQ2 NmQwY2NhMWYgMTAwNjQ0Ci0tLSBhL0NoYW5nZUxvZworKysgYi9DaGFuZ2VMb2cKQEAgLTEsMyAr MSwxNCBAQAorMjAxOC0wOS0xOSAgQ2FybG9zIEdhcmNpYSBDYW1wb3MgIDxjZ2FyY2lhQGlnYWxp YS5jb20+CisKKyAgICAgICAgW0xpbnV4XSBVc2UgbWVtZmRfY3JlYXRlIHdoZW4gYXZhaWxhYmxl IGluIFNoYXJlZE1lbW9yeSBpbXBsZW1lbnRhdGlvbgorICAgICAgICBodHRwczovL2J1Z3Mud2Vi a2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTg5NzQxCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9C T0RZIChPT1BTISkuCisKKyAgICAgICAgQWRkIGluY2x1ZGUgY2hlY2sgZm9yIGxpbnV4L21lbWZk LmggaGVhZGVyLgorCisgICAgICAgICogU291cmNlL2NtYWtlL09wdGlvbnNDb21tb24uY21ha2U6 CisKIDIwMTgtMDktMDYgIEFkcmlhbiBQZXJleiBkZSBDYXN0cm8gIDxhcGVyZXpAaWdhbGlhLmNv bT4KIAogICAgICAgICBVbnJldmlld2VkLiBVcGRhdGUgT3B0aW9uc1dQRS5jbWFrZSBhbmQgTkVX UyBmb3IgMi4yMS45MiByZWxlYXNlLgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdC9DaGFuZ2VM b2cgYi9Tb3VyY2UvV2ViS2l0L0NoYW5nZUxvZwppbmRleCA0YjcwZmRhMTUzMS4uZjg4YjA1Nzhh MDEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJL aXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTgtMDktMTkgIENhcmxvcyBHYXJjaWEg Q2FtcG9zICA8Y2dhcmNpYUBpZ2FsaWEuY29tPgorCisgICAgICAgIFtMaW51eF0gVXNlIG1lbWZk X2NyZWF0ZSB3aGVuIGF2YWlsYWJsZSBpbiBTaGFyZWRNZW1vcnkgaW1wbGVtZW50YXRpb24KKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE4OTc0MQorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIElmIG1lbWZkIGlz IGF2YWlsYWJsZSwgdXNlIGl0IGluc3RlYWQgb2Ygc2htX29wZW4uCisKKyAgICAgICAgKiBQbGF0 Zm9ybS91bml4L1NoYXJlZE1lbW9yeVVuaXguY3BwOgorICAgICAgICAoV2ViS2l0OjpjcmVhdGVT aGFyZWRNZW1vcnkpOiBIZWxwZXIgdG8gY3JlYXRlIHRoZSBzaGFyZWQgbWVtb3J5LCB0cnlpbmcg Zmlyc3Qgd2l0aCBtZW1mZCBhbmQgZmFsbGluZyBiYWNrIHRvCisgICAgICAgIHNobSBpZiBpdCdz IG5vdCBhdmFpbGFibGUuCisgICAgICAgIChXZWJLaXQ6OlNoYXJlZE1lbW9yeTo6Y3JlYXRlKTog VXNlIGNyZWF0ZVNoYXJlZE1lbW9yeSgpIGhlbHBlci4KKwogMjAxOC0wOC0zMSAgSm9obiBXaWxh bmRlciAgPHdpbGFuZGVyQGFwcGxlLmNvbT4KIAogICAgICAgICBTdG9yYWdlIEFjY2VzcyBBUEk6 IE1haW50YWluIGFjY2VzcyB0aHJvdWdoIHNhbWUtc2l0ZSBuYXZpZ2F0aW9ucwpkaWZmIC0tZ2l0 IGEvU291cmNlL1dlYktpdC9QbGF0Zm9ybS91bml4L1NoYXJlZE1lbW9yeVVuaXguY3BwIGIvU291 cmNlL1dlYktpdC9QbGF0Zm9ybS91bml4L1NoYXJlZE1lbW9yeVVuaXguY3BwCmluZGV4IDcxM2Rj MzUyNzBiLi4xZDE3NzY4YmU1MCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9QbGF0Zm9ybS91 bml4L1NoYXJlZE1lbW9yeVVuaXguY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQvUGxhdGZvcm0vdW5p eC9TaGFyZWRNZW1vcnlVbml4LmNwcApAQCAtNDQsNiArNDQsMTEgQEAKICNpbmNsdWRlIDx3dGYv dGV4dC9DU3RyaW5nLmg+CiAjaW5jbHVkZSA8d3RmL3RleHQvV1RGU3RyaW5nLmg+CiAKKyNpZiBI QVZFKExJTlVYX01FTUZEX0gpCisjaW5jbHVkZSA8bGludXgvbWVtZmQuaD4KKyNpbmNsdWRlIDxz eXMvc3lzY2FsbC5oPgorI2VuZGlmCisKIG5hbWVzcGFjZSBXZWJLaXQgewogCiBTaGFyZWRNZW1v cnk6OkhhbmRsZTo6SGFuZGxlKCkKQEAgLTEwNiwxMSArMTExLDI3IEBAIHN0YXRpYyBpbmxpbmUg aW50IGFjY2Vzc01vZGVNTWFwKFNoYXJlZE1lbW9yeTo6UHJvdGVjdGlvbiBwcm90ZWN0aW9uKQog ICAgIHJldHVybiBQUk9UX1JFQUQgfCBQUk9UX1dSSVRFOwogfQogCi1SZWZQdHI8U2hhcmVkTWVt b3J5PiBTaGFyZWRNZW1vcnk6OmNyZWF0ZSh2b2lkKiBhZGRyZXNzLCBzaXplX3Qgc2l6ZSwgUHJv dGVjdGlvbiBwcm90ZWN0aW9uKQorc3RhdGljIGludCBjcmVhdGVTaGFyZWRNZW1vcnkoKQogewot ICAgIENTdHJpbmcgdGVtcE5hbWU7Ci0KKyNpZiBIQVZFKExJTlVYX01FTUZEX0gpCisgICAgc3Rh dGljIGJvb2wgaXNNZW1GZEF2YWlsYWJsZSA9IHRydWU7CiAgICAgaW50IGZpbGVEZXNjcmlwdG9y ID0gLTE7CisgICAgaWYgKGlzTWVtRmRBdmFpbGFibGUpIHsKKyAgICAgICAgZG8geworICAgICAg ICAgICAgZmlsZURlc2NyaXB0b3IgPSBzeXNjYWxsKF9fTlJfbWVtZmRfY3JlYXRlLCAiV2ViS2l0 U2hhcmVkTWVtb3J5IiwgTUZEX0NMT0VYRUMpOworICAgICAgICB9IHdoaWxlIChmaWxlRGVzY3Jp cHRvciA9PSAtMSAmJiBlcnJubyA9PSBFSU5UUik7CisKKyAgICAgICAgaWYgKGZpbGVEZXNjcmlw dG9yICE9IC0xKQorICAgICAgICAgICAgcmV0dXJuIGZpbGVEZXNjcmlwdG9yOworCisgICAgICAg IGlmIChlcnJubyA9PSBFTk9TWVMpCisgICAgICAgICAgICByZXR1cm4gZmlsZURlc2NyaXB0b3I7 CisKKyAgICAgICAgaXNNZW1GZEF2YWlsYWJsZSA9IGZhbHNlOworICAgIH0KKyNlbmRpZgorCisg ICAgQ1N0cmluZyB0ZW1wTmFtZTsKICAgICBmb3IgKGludCB0cmllcyA9IDA7IGZpbGVEZXNjcmlw dG9yID09IC0xICYmIHRyaWVzIDwgMTA7ICsrdHJpZXMpIHsKICAgICAgICAgU3RyaW5nIG5hbWUg PSBTdHJpbmcoIi9XSzJTaGFyZWRNZW1vcnkuIikgKyBTdHJpbmc6Om51bWJlcihzdGF0aWNfY2Fz dDx1bnNpZ25lZD4oV1RGOjpyYW5kb21OdW1iZXIoKSAqIChzdGQ6Om51bWVyaWNfbGltaXRzPHVu c2lnbmVkPjo6bWF4KCkgKyAxLjApKSk7CiAgICAgICAgIHRlbXBOYW1lID0gbmFtZS51dGY4KCk7 CkBAIC0xMTksMTUgKzE0MCwyNCBAQCBSZWZQdHI8U2hhcmVkTWVtb3J5PiBTaGFyZWRNZW1vcnk6 OmNyZWF0ZSh2b2lkKiBhZGRyZXNzLCBzaXplX3Qgc2l6ZSwgUHJvdGVjdGlvbgogICAgICAgICAg ICAgZmlsZURlc2NyaXB0b3IgPSBzaG1fb3Blbih0ZW1wTmFtZS5kYXRhKCksIE9fQ1JFQVQgfCBP X1JEV1IsIFNfSVJVU1IgfCBTX0lXVVNSKTsKICAgICAgICAgfSB3aGlsZSAoZmlsZURlc2NyaXB0 b3IgPT0gLTEgJiYgZXJybm8gPT0gRUlOVFIpOwogICAgIH0KKworICAgIGlmIChmaWxlRGVzY3Jp cHRvciAhPSAtMSkKKyAgICAgICAgc2htX3VubGluayh0ZW1wTmFtZS5kYXRhKCkpOworCisgICAg cmV0dXJuIGZpbGVEZXNjcmlwdG9yOworfQorCitSZWZQdHI8U2hhcmVkTWVtb3J5PiBTaGFyZWRN ZW1vcnk6OmNyZWF0ZSh2b2lkKiBhZGRyZXNzLCBzaXplX3Qgc2l6ZSwgUHJvdGVjdGlvbiBwcm90 ZWN0aW9uKQoreworICAgIGludCBmaWxlRGVzY3JpcHRvciA9IGNyZWF0ZVNoYXJlZE1lbW9yeSgp OwogICAgIGlmIChmaWxlRGVzY3JpcHRvciA9PSAtMSkgewotICAgICAgICBXVEZMb2dBbHdheXMo IkZhaWxlZCB0byBjcmVhdGUgc2hhcmVkIG1lbW9yeSBmaWxlICVzOiAlcyIsIHRlbXBOYW1lLmRh dGEoKSwgc3RyZXJyb3IoZXJybm8pKTsKKyAgICAgICAgV1RGTG9nQWx3YXlzKCJGYWlsZWQgdG8g Y3JlYXRlIHNoYXJlZCBtZW1vcnkgZmlsZTogJXMiLCBzdHJlcnJvcihlcnJubykpOwogICAgICAg ICByZXR1cm4gbnVsbHB0cjsKICAgICB9CiAKICAgICB3aGlsZSAoZnRydW5jYXRlKGZpbGVEZXNj cmlwdG9yLCBzaXplKSA9PSAtMSkgewogICAgICAgICBpZiAoZXJybm8gIT0gRUlOVFIpIHsKICAg ICAgICAgICAgIGNsb3NlV2l0aFJldHJ5KGZpbGVEZXNjcmlwdG9yKTsKLSAgICAgICAgICAgIHNo bV91bmxpbmsodGVtcE5hbWUuZGF0YSgpKTsKICAgICAgICAgICAgIHJldHVybiBudWxscHRyOwog ICAgICAgICB9CiAgICAgfQpAQCAtMTM1LDEyICsxNjUsOSBAQCBSZWZQdHI8U2hhcmVkTWVtb3J5 PiBTaGFyZWRNZW1vcnk6OmNyZWF0ZSh2b2lkKiBhZGRyZXNzLCBzaXplX3Qgc2l6ZSwgUHJvdGVj dGlvbgogICAgIHZvaWQqIGRhdGEgPSBtbWFwKGFkZHJlc3MsIHNpemUsIGFjY2Vzc01vZGVNTWFw KHByb3RlY3Rpb24pLCBNQVBfU0hBUkVELCBmaWxlRGVzY3JpcHRvciwgMCk7CiAgICAgaWYgKGRh dGEgPT0gTUFQX0ZBSUxFRCkgewogICAgICAgICBjbG9zZVdpdGhSZXRyeShmaWxlRGVzY3JpcHRv cik7Ci0gICAgICAgIHNobV91bmxpbmsodGVtcE5hbWUuZGF0YSgpKTsKICAgICAgICAgcmV0dXJu IG51bGxwdHI7CiAgICAgfQogCi0gICAgc2htX3VubGluayh0ZW1wTmFtZS5kYXRhKCkpOwotCiAg ICAgUmVmUHRyPFNoYXJlZE1lbW9yeT4gaW5zdGFuY2UgPSBhZG9wdFJlZihuZXcgU2hhcmVkTWVt b3J5KCkpOwogICAgIGluc3RhbmNlLT5tX2RhdGEgPSBkYXRhOwogICAgIGluc3RhbmNlLT5tX2Zp bGVEZXNjcmlwdG9yID0gZmlsZURlc2NyaXB0b3I7CmRpZmYgLS1naXQgYS9Tb3VyY2UvY21ha2Uv T3B0aW9uc0NvbW1vbi5jbWFrZSBiL1NvdXJjZS9jbWFrZS9PcHRpb25zQ29tbW9uLmNtYWtlCmlu ZGV4IGNkODkzZDMzM2M5Li5kYzFiOGZiMDMwYiAxMDA2NDQKLS0tIGEvU291cmNlL2NtYWtlL09w dGlvbnNDb21tb24uY21ha2UKKysrIGIvU291cmNlL2NtYWtlL09wdGlvbnNDb21tb24uY21ha2UK QEAgLTEyMiw2ICsxMjIsNyBAQCBXRUJLSVRfQ0hFQ0tfSEFWRV9JTkNMVURFKEhBVkVfU1RSSU5H U19IIHN0cmluZ3MuaCkKIFdFQktJVF9DSEVDS19IQVZFX0lOQ0xVREUoSEFWRV9TWVNfUEFSQU1f SCBzeXMvcGFyYW0uaCkKIFdFQktJVF9DSEVDS19IQVZFX0lOQ0xVREUoSEFWRV9TWVNfVElNRV9I IHN5cy90aW1lLmgpCiBXRUJLSVRfQ0hFQ0tfSEFWRV9JTkNMVURFKEhBVkVfU1lTX1RJTUVCX0gg c3lzL3RpbWViLmgpCitXRUJLSVRfQ0hFQ0tfSEFWRV9JTkNMVURFKEhBVkVfTElOVVhfTUVNRkRf SCBsaW51eC9tZW1mZC5oKQogCiAjIENoZWNrIGZvciBmdW5jdGlvbnMKIFdFQktJVF9DSEVDS19I QVZFX0ZVTkNUSU9OKEhBVkVfQUxJR05FRF9NQUxMT0MgX2FsaWduZWRfbWFsbG9jKQo=