Re: [PATCH v2] ppc: remove custom SHA-1 implementation
- From
brian m. carlson <sandals@crustytoothpaste.net>
- Date
- Mar 21, 2022, 21:19 UTC
- Message-ID
- <Yjjr9fkybVmB53M7@camp.crustytoothpaste.net>
- In-Reply-To
- <patch-v2-1.1-e77fd23a824-20220321T170412Z-avarab@gmail.com>
On 2022-03-21 at 17:06:12, Ævar Arnfjörð Bjarmason wrote:
Show 33 quoted lines
> Remove the PPC_SHA1 implementation added in a6ef3518f9a ([PATCH] PPC > assembly implementation of SHA1, 2005-04-22). When this was added > Apple consumer hardware used the PPC architecture, and the > implementation was intended to improve SHA-1 speed there. > > Since it was added we've moved to DC_SHA1 by default, and anyone > wanting hard-rolled non-DC SHA-1 implementation can use OpenSSL's via > the OPENSSL_SHA1 knob. > > I'm unsure if this was ever supposed to work on 64-bit PPC. It clearly > originally targeted 32 bit PPC, but there's some mailing list > references to this being tried on G5 (PPC 970). I can't get it to do > anything but segfault on the BE POWER8 machine in the GCC compile > farm. Anyone caring about speed on PPC these days is likely to be > using IBM's POWER, not PPC 970. > > There have been proposals to entirely remove non-DC_SHA1 > implementations from the tree[1]. I think per [2] that would be a bit > overzealous. I.e. there are various set-ups git's speed is going to be > more important than the relatively implausible SHA-1 collision attack, > or where such attacks are entirely mitigated by other means (e.g. by > incoming objects being checked with DC_SHA1). > > The main reason for doing so at this point is to simplify follow-up > Makefile change. Since PPC_SHA1 included the only in-tree *.S assembly > file we needed to keep around special support for building objects > from it. By getting rid of it we know we'll always build *.o from *.c > files, which makes the build process simpler. > > As an aside the code being removed here was also throwing warnings > with the "-pedantic" flag, but let's remove it instead of fixing it, > as 544d93bc3b4 (block-sha1: remove use of obsolete x86 assembly, > 2022-03-10) did for block-sha1/*.
While I don't agree that we shouldn't remove the other non-DC SHA-1 implementations, I do agree that we should remove this one. Given the testing you've done and the fact that almost everyone desiring speed is using a 64-bit machine these days, I think it's unlikely that anyone is using this in the real world.
-- brian m. carlson (he/him or they/them) Toronto, Ontario, CA