On 8/23/2018 1:04 AM, Jeff King wrote:
On Thu, Aug 23, 2018 at 03:47:07AM +0000, brian m. carlson wrote:
I expect that's going to be the case as well. I have patches that
wire up actual SHA-256 support in my hash-impl branch.
However, having said that, I'm happy to defer to whatever everyone else
thinks is best for 2.19. The assert solution would be fine with me in
this situation, and if we need to pull it out in the future, that's okay
with me.
I don't really have a strong opinion on this either way, so if someone
else does, please say so. I have somewhat more limited availability
over the next couple days, as I'm travelling on business, but I'm happy
to review a patch (and it seems like Peff has one minus the actual
commit message).
I just posted the patch elsewhere in the thread.
Thank you for that!
I think you can safely
ignore the rest of it if you are otherwise occupied. Even if v2.19 ships
without some mitigation, I don't know that it's all that big a deal,
given the numbers I generated (which for some reason are less dramatic
than Stolee's).
My numbers may be more dramatic because my Linux environment is a
virtual machine.
I was thinking that having a mitigation for 2.19 is best, and then we
can focus as part of the 2.20 cycle how we can properly avoid this cost,
especially when 32 is a valid option.
Around the time that my proposed approaches were getting vetoed for
alignment issues, I figured I was out of my depth here. I reached out to
Daniel Lemire (of EWAH bitmap fame) on Twitter [1]. His blog is full of
posts of word-based approaches to different problems, so I thought he
might know something off the top of his head that would be applicable.
His conclusion (after looking only a short time) was to take a 'hasheq'
approach [2] like Peff suggested [3]. Since that requires auditing all
callers of hashcmp to see if hasheq is appropriate, it is not a good
solution for 2.19 but (in my opinion) should be evaluated as part of the
2.20 cycle.
Of course, if someone with knowledge of word-alignment issues across the
platforms we support knows how to enforce an alignment for object_id,
then something word-based like [4] could be reconsidered.
Thanks, everyone!
-Stolee
[1] https://twitter.com/stolee/status/1032312965754748930
[2]
https://lemire.me/blog/2018/08/22/avoid-lexicographical-comparisons-when-testing-for-string-equality/
[3]
https://public-inbox.org/git/20180822030344.GA14684@xxxxxxxxxxxxxxxxxxxxx/
[4]
https://public-inbox.org/git/7ea416cf-b043-1274-e161-85a8780b8e1c@xxxxxxxxx/