227274 2021-06-22 17:13:13 -0700 Fix incorrect indexing / out of bounds access in getRestartIndices 2021-06-24 03:13:14 -0700 1 1 1 Unclassified WebKit ANGLE WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 kpiddington kpiddington dino ews-watchlist kbr kkinnunen kondapallykalyan webkit-bug-importer oldest_to_newest 1771828 0 kpiddington 2021-06-22 17:13:13 -0700 Fix incorrect indexing / out of bounds access in getRestartIndices 1771830 1 432008 kpiddington 2021-06-22 17:14:49 -0700 Created attachment 432008 Patch 1771831 2 kpiddington 2021-06-22 17:14:51 -0700 <rdar://problem/79244789> 1771832 3 ews-watchlist 2021-06-22 17:16:01 -0700 Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE 1771835 4 432008 dino 2021-06-22 17:29:26 -0700 Comment on attachment 432008 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432008&action=review > Source/ThirdParty/ANGLE/ChangeLog:8 > + In the case where a restart index is present in the last element of an index array, we can accidently > + access out of bounds. Fix this by checking boundaries, and fixing other indexing errors Do we need a new test, or did the existing tests pick this up? 1772029 5 kpiddington 2021-06-23 12:14:13 -0700 (In reply to Dean Jackson from comment #4) > Comment on attachment 432008 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=432008&action=review > > > Source/ThirdParty/ANGLE/ChangeLog:8 > > + In the case where a restart index is present in the last element of an index array, we can accidently > > + access out of bounds. Fix this by checking boundaries, and fixing other indexing errors > > Do we need a new test, or did the existing tests pick this up? The existing tests should hit this path, but would probably only trigger on ASAN builds. 1772197 6 ews-feeder 2021-06-23 23:53:02 -0700 Committed r279213 (239099@main): <https://commits.webkit.org/239099@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 432008. 1772234 7 432008 kkinnunen 2021-06-24 03:13:14 -0700 Comment on attachment 432008 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=432008&action=review > Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm:418 > T *bufferData = (T*)(idxBuffer->mapReadOnly(ctx)); do you need unmap? > Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm:420 > + for(int i = 0; i < numIndices; i++) I think the algorithm creates many 1 length ranges for contents like F,F,F,F IndexRange is inclusive and as such cannot represent zero length ranges. > Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm:436 > } template<typename T> static void calculateRestartRanges(ContextMtl* ctx, mtl::BufferRef idxBuffer, std::vector<IndexRange> * ranges) { ranges->clear(); T *bufferData = reinterpret_cast<T*>(idxBuffer->mapReadOnly(ctx)); constexpr size_t indexCount = idxBuffer->size() / sizeof(T) constexpr size_t restartMarker = std::numeric_limits<T>::max(); for(size_t i = 0; i < indexCount;) { if (bufferData[i] == restartMarker) { ++i; continue; } size_t restartBegin = i; ++i; while (i < indexCount && bufferData[i] != restartMarker) { ++i; } size_t restartEnd = i - 1; ranges->emplace_back(restartBegin, restartEnd); } idxBuffer->unmap(); } I'd write something like that, but I'm the last person to correct anybody's off-by-ones. 432008 2021-06-22 17:14:49 -0700 2021-06-23 23:53:03 -0700 Patch bug-227274-20210622171448.patch text/plain 2530 kpiddington U3VidmVyc2lvbiBSZXZpc2lvbjogMjc5MTM3CmRpZmYgLS1naXQgYS9Tb3VyY2UvVGhpcmRQYXJ0 eS9BTkdMRS9DaGFuZ2VMb2cgYi9Tb3VyY2UvVGhpcmRQYXJ0eS9BTkdMRS9DaGFuZ2VMb2cKaW5k ZXggNTFiNzNiMzNjYWEwNWMwNWMxYzZlYWUyMzc2ODc1NTU1MDFhOTI5Mi4uNTJkZjhmNGM5NzIw OWIyOThlZDdiZWNhYTk1MGI4OWE0MDQ1YjNmZCAxMDA2NDQKLS0tIGEvU291cmNlL1RoaXJkUGFy dHkvQU5HTEUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9UaGlyZFBhcnR5L0FOR0xFL0NoYW5nZUxv ZwpAQCAtMSwzICsxLDE5IEBACisyMDIxLTA2LTIyICBLeWxlIFBpZGRpbmd0b24gIDxrcGlkZGlu Z3RvbkBhcHBsZS5jb20+CisKKyAgICAgICAgRml4IGluY29ycmVjdCBpbmRleGluZyAvIG91dCBv ZiBib3VuZHMgYWNjZXNzIGluIGdldFJlc3RhcnRJbmRpY2VzCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMjcyNzQKKyAgICAgICAgPHJkYXI6Ly83OTI0 NDc4OT4KKworICAgICAgICBJbiB0aGUgY2FzZSB3aGVyZSBhIHJlc3RhcnQgaW5kZXggaXMgcHJl c2VudCBpbiB0aGUgbGFzdCBlbGVtZW50IG9mIGFuIGluZGV4IGFycmF5LCB3ZSBjYW4gYWNjaWRl bnRseQorICAgICAgICBhY2Nlc3Mgb3V0IG9mIGJvdW5kcy4gRml4IHRoaXMgYnkgY2hlY2tpbmcg Ym91bmRhcmllcywgYW5kIGZpeGluZyBvdGhlciBpbmRleGluZyBlcnJvcnMKKworICAgICAgICBU ZXN0czogUmFuIGRlcXAvZnVuY3Rpb25hbC9nbGVzMy9wcmltaXRpdmVyZXN0YXJ0IHN1aXRlLiBQ YXNzZWQgOCB0ZXN0cy4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKwor ICAgICAgICAqIHNyYy9saWJBTkdMRS9yZW5kZXJlci9tZXRhbC9CdWZmZXJNdGwubW06CisgICAg ICAgIChyeDo6Y2FsY3VsYXRlUmVzdGFydFJhbmdlcyk6CisKIDIwMjEtMDYtMTcgIEt5bGUgUGlk ZGluZ3RvbiAgPGtwaWRkaW5ndG9uQGFwcGxlLmNvbT4KIAogICAgICAgICBbTWV0YWwgQU5HTEVd IFNoYWRlcnMgd2l0aCByZXNlcnZlZCBtZXRhbCBrZXl3b3JkcyBkbyBub3QgdHJhbnNsYXRlLCBu b3IgZG8gc2hhZGVycyB3aXRoIHN0cnVjdCBhbmQgdmFyaWFibGUgbmFtZXMgdGhhdCBhcmUgdGhl IHNhbWUgZXhjZXB0IHByZWZpeGVkIGJ5IGFuIHVuZGVyc2NvcmUKZGlmZiAtLWdpdCBhL1NvdXJj ZS9UaGlyZFBhcnR5L0FOR0xFL3NyYy9saWJBTkdMRS9yZW5kZXJlci9tZXRhbC9CdWZmZXJNdGwu bW0gYi9Tb3VyY2UvVGhpcmRQYXJ0eS9BTkdMRS9zcmMvbGliQU5HTEUvcmVuZGVyZXIvbWV0YWwv QnVmZmVyTXRsLm1tCmluZGV4IDM3OTQ2MzJmNzM0YWNjMjdhNmMzNjE1NWE2ODRmZmYzOWU0ZWM5 YzYuLjA2ZjhkNTc0ZDYxNGU1ZTRhYTI4YzczYjQ3ODE4NWM3OWE4ODZiNjcgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9UaGlyZFBhcnR5L0FOR0xFL3NyYy9saWJBTkdMRS9yZW5kZXJlci9tZXRhbC9CdWZm ZXJNdGwubW0KKysrIGIvU291cmNlL1RoaXJkUGFydHkvQU5HTEUvc3JjL2xpYkFOR0xFL3JlbmRl cmVyL21ldGFsL0J1ZmZlck10bC5tbQpAQCAtNDE2LDcgKzQxNiw4IEBAIHN0YXRpYyB2b2lkIGNh bGN1bGF0ZVJlc3RhcnRSYW5nZXMoQ29udGV4dE10bCogY3R4LCBtdGw6OkJ1ZmZlclJlZiBpZHhC dWZmZXIsIHN0CiB7CiAgICAgcmFuZ2VzLT5jbGVhcigpOwogICAgIFQgKmJ1ZmZlckRhdGEgPSAo VCopKGlkeEJ1ZmZlci0+bWFwUmVhZE9ubHkoY3R4KSk7Ci0gICAgZm9yKGludCBpID0gMDsgaSA8 IGlkeEJ1ZmZlci0+c2l6ZSgpL3NpemVvZihUKTsgaSsrKQorICAgIGNvbnN0IHNpemVfdCBudW1J bmRpY2VzID0gaWR4QnVmZmVyLT5zaXplKCkvc2l6ZW9mKFQpOworICAgIGZvcihpbnQgaSA9IDA7 IGkgPCBudW1JbmRpY2VzOyBpKyspCiAgICAgewogICAgICAgICBUIHZhbHVlID0gYnVmZmVyRGF0 YVtpXTsKICAgICAgICAgaWYodmFsdWUgPT0gc3RkOjpudW1lcmljX2xpbWl0czxUPjo6bWF4KCkp CkBAIC00MjcsOCArNDI4LDkgQEAgc3RhdGljIHZvaWQgY2FsY3VsYXRlUmVzdGFydFJhbmdlcyhD b250ZXh0TXRsKiBjdHgsIG10bDo6QnVmZmVyUmVmIGlkeEJ1ZmZlciwgc3QKICAgICAgICAgICAg IGRvCiAgICAgICAgICAgICB7CiAgICAgICAgICAgICAgICAgKytpOwotICAgICAgICAgICAgICAg IHZhbHVlID0gYnVmZmVyRGF0YVtpXTsKLSAgICAgICAgICAgIH13aGlsZSAoaSA8IGlkeEJ1ZmZl ci0+c2l6ZSgpICYmIHZhbHVlID09IHN0ZDo6bnVtZXJpY19saW1pdHM8VD46Om1heCgpKTsKKyAg ICAgICAgICAgICAgICBpZihpIDwgbnVtSW5kaWNlcykKKyAgICAgICAgICAgICAgICAgICAgdmFs dWUgPSBidWZmZXJEYXRhW2ldOworICAgICAgICAgICAgfXdoaWxlIChpIDwgbnVtSW5kaWNlcyAm JiB2YWx1ZSA9PSBzdGQ6Om51bWVyaWNfbGltaXRzPFQ+OjptYXgoKSk7CiAgICAgICAgICAgICBu ZXdSYW5nZS5yZXN0YXJ0RW5kID0gaS0xOwogICAgICAgICAgICAgcmFuZ2VzLT5wdXNoX2JhY2so bmV3UmFuZ2UpOwogICAgICAgICB9Cg==