Skip to contentSkip to conversation

lib/crypto: mips: Drop optimized MD5 code

PATCH Applied Eric Biggers 3 participants 6 messages Mar 26 – Apr 19

Subsystems (3)
lore
Share & tools
lore

Eric Biggers

author maintainer
#0 lore raw

PATCH 1/1 lib/crypto: mips: Drop optimized MD5 code

MD5 is obsolete. Continuing to maintain architecture-optimized implementations of MD5 is unnecessary and risky. It diverts resources from the modern algorithms that are actually important.

While there was demand for continuing to maintain the PowerPC optimized MD5 code to accommodate userspace programs that are misusing AF_ALG
(https://lore.kernel.org/linux-crypto/[email protected]/),
no such demand has been seen for the MIPS Cavium Octeon optimized MD5 code. Note that this code runs on only one particular line of SoCs.

Thus, let's drop it and focus effort on the more modern SHA algorithms, which already have optimized code for the same SoCs.

---

This patch is targeting libcrypto-next

 lib/crypto/Kconfig    |  1 -
 lib/crypto/mips/md5.h | 65 -------------------------------------------
 2 files changed, 66 deletions(-)
 delete mode 100644 lib/crypto/mips/md5.h
lib/crypto/Kconfig +0 -1
index 4b6f593dc72f..9f31f03062f0 100644
@@ -132,11 +132,10 @@ config CRYPTO_LIB_MD5
 	  uses any of the functions from <crypto/md5.h>.
 
 config CRYPTO_LIB_MD5_ARCH
 	bool
 	depends on CRYPTO_LIB_MD5 && !UML
-	default y if MIPS && CPU_CAVIUM_OCTEON
 	default y if PPC
 	default y if SPARC64
 
 config CRYPTO_LIB_MLDSA
 	tristate
lib/crypto/mips/md5.h deleted +0 -65

65 lines not on this page · raw message

base-commit: 7ac21b4032e5b9b8a6a312b6f1d54f4ba24d1c16

Signature · 1 line
2.53.0
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 14 lines · Eric Biggers

MD5 is obsolete. Continuing to maintain architecture-optimized implementations of MD5 is unnecessary and risky. It diverts resources from the modern algorithms that are actually important.

While there was demand for continuing to maintain the PowerPC optimized MD5 code to accommodate userspace programs that are misusing AF_ALG
(https://lore.kernel.org/linux-crypto/[email protected]/),
no such demand has been seen for the MIPS Cavium Octeon optimized MD5 code. Note that this code runs on only one particular line of SoCs.

Thus, let's drop it and focus effort on the more modern SHA algorithms, which already have optimized code for the same SoCs.

Quoted 9 lines

---

This patch is targeting libcrypto-next

 lib/crypto/Kconfig    |  1 -
 lib/crypto/mips/md5.h | 65 -------------------------------------------
 2 files changed, 66 deletions(-)
 delete mode 100644 lib/crypto/mips/md5.h
author maintainer Applied
#2 lore raw
In reply to Eric Biggers
Quoted 10 lines · Eric Biggers

MD5 is obsolete. Continuing to maintain architecture-optimized implementations of MD5 is unnecessary and risky. It diverts resources from the modern algorithms that are actually important.

While there was demand for continuing to maintain the PowerPC optimized MD5 code to accommodate userspace programs that are misusing AF_ALG
(https://lore.kernel.org/linux-crypto/[email protected]/),
no such demand has been seen for the MIPS Cavium Octeon optimized MD5 code. Note that this code runs on only one particular line of SoCs.

Quoted 7 lines

Thus, let's drop it and focus effort on the more modern SHA algorithms, which already have optimized code for the same SoCs.

---

This patch is targeting libcrypto-next

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-next

- Eric

#3 lore raw
In reply to Eric Biggers

Hi,

Quoted 4 lines · Eric Biggers

MD5 is obsolete. Continuing to maintain architecture-optimized implementations of MD5 is unnecessary and risky. It diverts resources from the modern algorithms that are actually important.

Quoted 8 lines

While there was demand for continuing to maintain the PowerPC optimized MD5 code to accommodate userspace programs that are misusing AF_ALG
(https://lore.kernel.org/linux-crypto/[email protected]/),
no such demand has been seen for the MIPS Cavium Octeon optimized MD5 code. Note that this code runs on only one particular line of SoCs.

Thus, let's drop it and focus effort on the more modern SHA algorithms, which already have optimized code for the same SoCs.

I don't mind deleting this (I shut down all my MIPS hardware already in new year 2020 to start a "fresh" decade), but just for the record, this will probably downgrade the performance of TCP_MD5SIG which I recall was the original reason this code got added...

Also that PowerPC case about "misusing AF_ALG" is interesting - I often do similar on small systems (just to save binary space and avoid duplicate implementation) - why AF_ALG even allows such use if it's considered a bug?

A.

Quoted 103 lines

---

This patch is targeting libcrypto-next

 lib/crypto/Kconfig    |  1 -
 lib/crypto/mips/md5.h | 65 -------------------------------------------
 2 files changed, 66 deletions(-)
 delete mode 100644 lib/crypto/mips/md5.h
lib/crypto/Kconfig +0 -1
index 4b6f593dc72f..9f31f03062f0 100644
@@ -132,11 +132,10 @@ config CRYPTO_LIB_MD5
 	  uses any of the functions from <crypto/md5.h>.
 
 config CRYPTO_LIB_MD5_ARCH
 	bool
 	depends on CRYPTO_LIB_MD5 && !UML
-	default y if MIPS && CPU_CAVIUM_OCTEON
 	default y if PPC
 	default y if SPARC64
 
 config CRYPTO_LIB_MLDSA
 	tristate
lib/crypto/mips/md5.h deleted +0 -65
deleted file mode 100644
index e08e28aeffa4..000000000000
@@ -1,65 +0,0 @@
-/*
- * Cryptographic API.
- *
- * MD5 Message Digest Algorithm (RFC1321).
- *
- * Adapted for OCTEON by Aaro Koskinen <[email protected]>.
- *
- * Based on crypto/md5.c, which is:
- *
- * Derived from cryptoapi implementation, originally based on the
- * public domain implementation written by Colin Plumb in 1993.
- *
- * Copyright (c) Cryptoapi developers.
- * Copyright (c) 2002 James Morris <[email protected]>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 of the License, or (at your option)
- * any later version.
- */
-
-#include <asm/octeon/crypto.h>
-#include <asm/octeon/octeon.h>
-
-/*
- * We pass everything as 64-bit. OCTEON can handle misaligned data.
- */
-
-static void md5_blocks(struct md5_block_state *state,
-		       const u8 *data, size_t nblocks)
-{
-	struct octeon_cop2_state cop2_state;
-	u64 *state64 = (u64 *)state;
-	unsigned long flags;
-
-	if (!octeon_has_crypto())
-		return md5_blocks_generic(state, data, nblocks);
-
-	cpu_to_le32_array(state->h, ARRAY_SIZE(state->h));
-
-	flags = octeon_crypto_enable(&cop2_state);
-	write_octeon_64bit_hash_dword(state64[0], 0);
-	write_octeon_64bit_hash_dword(state64[1], 1);
-
-	do {
-		const u64 *block = (const u64 *)data;
-
-		write_octeon_64bit_block_dword(block[0], 0);
-		write_octeon_64bit_block_dword(block[1], 1);
-		write_octeon_64bit_block_dword(block[2], 2);
-		write_octeon_64bit_block_dword(block[3], 3);
-		write_octeon_64bit_block_dword(block[4], 4);
-		write_octeon_64bit_block_dword(block[5], 5);
-		write_octeon_64bit_block_dword(block[6], 6);
-		octeon_md5_start(block[7]);
-
-		data += MD5_BLOCK_SIZE;
-	} while (--nblocks);
-
-	state64[0] = read_octeon_64bit_hash_dword(0);
-	state64[1] = read_octeon_64bit_hash_dword(1);
-	octeon_crypto_disable(&cop2_state, flags);
-
-	le32_to_cpu_array(state->h, ARRAY_SIZE(state->h));
-}

base-commit: 7ac21b4032e5b9b8a6a312b6f1d54f4ba24d1c16

Signature · 1 line
2.53.0
author maintainer
#4 lore raw
Quoted 13 lines · Aaro Koskinen

Hi,

Quoted 10 lines · Eric Biggers

MD5 is obsolete. Continuing to maintain architecture-optimized implementations of MD5 is unnecessary and risky. It diverts resources from the modern algorithms that are actually important.

While there was demand for continuing to maintain the PowerPC optimized MD5 code to accommodate userspace programs that are misusing AF_ALG
(https://lore.kernel.org/linux-crypto/[email protected]/),
no such demand has been seen for the MIPS Cavium Octeon optimized MD5 code. Note that this code runs on only one particular line of SoCs.

Quoted 7 lines
Quoted 2 lines

Thus, let's drop it and focus effort on the more modern SHA algorithms, which already have optimized code for the same SoCs.

I don't mind deleting this (I shut down all my MIPS hardware already in new year 2020 to start a "fresh" decade), but just for the record, this will probably downgrade the performance of TCP_MD5SIG which I recall was the original reason this code got added...

Sure, for any removal of optimized code it's always possible to hypothesize a case where it regresses performance. The question is whether it actually matters and is worth keeping around. You mentioned that you did care about this code, but no longer do. I think anyone who may have cared about this in the past is likely to have had a similar experience.

After all, the only line of SoCs that could run this code switched from MIPS to ARM in 2016. Meanwhile, TCP-MD5 itself is insecure and has been superseded by TCP-AO. (Note that TCP-AO supports HMAC-SHA1 and HMAC-SHA256, which are still accelerated on MIPS Cavium Octeon.)

Yes, there are still people using TCP-MD5 anyway (on some platforms, not necessarily this particular one). But a nudge towards upgrading to TCP-AO isn't necessarily a bad thing, either...

Quoted 4 lines

Also that PowerPC case about "misusing AF_ALG" is interesting - I often do similar on small systems (just to save binary space and avoid duplicate implementation) - why AF_ALG even allows such use if it's considered a bug?

It's just a mistake from a long time ago (2010) that still has to be maintained for backwards compatibility. It's not something that would be accepted in the kernel if it was proposed today.

- Eric

author maintainer
#5 lore raw
Quoted 29 lines · Eric Biggers
Quoted 13 lines · Aaro Koskinen

Hi,

Quoted 10 lines · Eric Biggers

MD5 is obsolete. Continuing to maintain architecture-optimized implementations of MD5 is unnecessary and risky. It diverts resources from the modern algorithms that are actually important.

While there was demand for continuing to maintain the PowerPC optimized MD5 code to accommodate userspace programs that are misusing AF_ALG
(https://lore.kernel.org/linux-crypto/[email protected]/),
no such demand has been seen for the MIPS Cavium Octeon optimized MD5 code. Note that this code runs on only one particular line of SoCs.

Quoted 7 lines
Quoted 2 lines

Thus, let's drop it and focus effort on the more modern SHA algorithms, which already have optimized code for the same SoCs.

I don't mind deleting this (I shut down all my MIPS hardware already in new year 2020 to start a "fresh" decade), but just for the record, this will probably downgrade the performance of TCP_MD5SIG which I recall was the original reason this code got added...

Sure, for any removal of optimized code it's always possible to hypothesize a case where it regresses performance. The question is whether it actually matters and is worth keeping around. You mentioned that you did care about this code, but no longer do. I think anyone who may have cared about this in the past is likely to have had a similar experience.

Quoted 8 lines

After all, the only line of SoCs that could run this code switched from MIPS to ARM in 2016. Meanwhile, TCP-MD5 itself is insecure and has been superseded by TCP-AO. (Note that TCP-AO supports HMAC-SHA1 and HMAC-SHA256, which are still accelerated on MIPS Cavium Octeon.)

Yes, there are still people using TCP-MD5 anyway (on some platforms, not necessarily this particular one). But a nudge towards upgrading to TCP-AO isn't necessarily a bad thing, either...

I should also emphasize, again, that we don't actually know whether this code even still worked. It underwent quite a bit refactoring in the last year or so to keep it theoretically working as the crypto subsystem evolved. Yet it has never been testable in QEMU, and there were never any reports of anyone re-testing it on the hardware.

If there were someone who had said, "Yes, I need this legacy code, it passes the MD5 KUnit test, and I'll be re-testing it regularly", that would have been more encouraging. But otherwise we may have been just been keeping broken code around, which is much worse than simply using the well-tested generic code.

- Eric