Use timingsafe_bcmp() in WTF::constantTimeMemcmp() when available, instead of our own custom implementation.
Created attachment 429795 [details] Patch
Comment on attachment 429795 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429795&action=review > Source/WTF/wtf/CryptographicUtilities.cpp:36 > + return timingsafe_bcmp(voidA, voidB, length); Why not timingsafe_memcmp, or should we rename constantTimeMemcmp to constantTimeBCmp?
(In reply to Alex Christensen from comment #2) > Comment on attachment 429795 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=429795&action=review > > > Source/WTF/wtf/CryptographicUtilities.cpp:36 > > + return timingsafe_bcmp(voidA, voidB, length); > > Why not timingsafe_memcmp, or should we rename constantTimeMemcmp to > constantTimeBCmp? I didn't choose how the system API is called :) No, I cannot rename the system API to timingsafe_memcmp. We could rename the WTF:: constantTimeMemcmp to match BSD's naming but I personally prefer the memcmp naming.
Comment on attachment 429795 [details] Patch Oh, I was looking at the bsd documentation, which has both. memcmp implies a directional return value that can be used in sorting, but this doesn't have that property. Our use only checks whether it's zero, though, so it's fine so far.
Comment on attachment 429795 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=429795&action=review > Source/WTF/wtf/CryptographicUtilities.cpp:33 > NEVER_INLINE int constantTimeMemcmp(const void* voidA, const void* voidB, size_t length) This wrapper that's in a different dylib and forces no inlining is slightly wasteful when calling the system API. Perhaps implement the system API path in the header?
Committed r278140 (238185@main): <https://commits.webkit.org/238185@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 429795 [details].
<rdar://problem/78542643>
(In reply to Alexey Proskuryakov from comment #5) > Comment on attachment 429795 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=429795&action=review > > > Source/WTF/wtf/CryptographicUtilities.cpp:33 > > NEVER_INLINE int constantTimeMemcmp(const void* voidA, const void* voidB, size_t length) > > This wrapper that's in a different dylib and forces no inlining is slightly > wasteful when calling the system API. Perhaps implement the system API path > in the header? Sure, I can do that. I actually had hesitated to do that. I'll follow-up.
Reopening to attach new patch.
Created attachment 429821 [details] Patch
Committed r278144 (238188@main): <https://commits.webkit.org/238188@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 429821 [details].
This should probably use a new HAVE macro.
(In reply to Sam Weinig from comment #12) > This should probably use a new HAVE macro. It's there, don't you see it? http://trac.webkit.org/r278178 :D
(In reply to Chris Dumez from comment #13) > (In reply to Sam Weinig from comment #12) > > This should probably use a new HAVE macro. > > It's there, don't you see it? http://trac.webkit.org/r278178 :D Great!