Remove decoder memory allocations based on untrusted data (sizes) in the stream; related changes
Created attachment 426923 [details] Patch
*** Bug 209270 has been marked as a duplicate of this bug. ***
Comment on attachment 426923 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426923&action=review > Source/WebKit/Platform/IPC/ArgumentCoders.h:-517 > HashMapType hashMap; > - hashMap.reserveInitialCapacity(hashMapSize); Should we consider doing the trick we do for Vector here where if the key and value are both fixed size we can check that the buffer is big enough to contain size * (sizeof(key type) + sizeof(value type)) and then reserve? I don't have a good feeling about what type of HashMaps/HashSets we use so I am not sure if that is a useful optimization.
Comment on attachment 426923 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426923&action=review >> Source/WebKit/Platform/IPC/ArgumentCoders.h:-517 >> - hashMap.reserveInitialCapacity(hashMapSize); > > Should we consider doing the trick we do for Vector here where if the key and value are both fixed size we can check that the buffer is big enough to contain size * (sizeof(key type) + sizeof(value type)) and then reserve? I don't have a good feeling about what type of HashMaps/HashSets we use so I am not sure if that is a useful optimization. Not sure what you suggest can be done, but I’m sure we can do some kind of reality check about the remaining size of the decoding stream and reserve if it passes that check. But I don’t see this in VectorArgumentCoder. Where is the code for that trick you are referring to?
Oh, I see now. That first bool in the template!
(In reply to Darin Adler from comment #5) > Oh, I see now. That first bool in the template! That’s a surprisingly narrow optimization.
(In reply to Darin Adler from comment #6) > (In reply to Darin Adler from comment #5) > > Oh, I see now. That first bool in the template! > > That’s a surprisingly narrow optimization. It’s a more specific optimization for a smaller set of types. It’s "treat this as a single buffer and read it all at once using decodeFixedLengthData" when it’s a simple scalar. It does seem like there are conditions where we can sufficiently vet the size and use it to preallocate space in the hash table. Using the relationship between the size of the remaining data and the size that we pre-allocate. But, like you, Sam, I don’t know how important and valuable this optimization is. It saves memory (very important) on hash tables that we send across IPC (wait, what?, why?).
Committed r276510 (236967@main): <https://commits.webkit.org/236967@main>
<rdar://problem/77110020>