From: Scott Dial <scott@scottdial.com>

Date: Fri, 24 Apr 2020 18:51:08 -0400

Subject: net: macsec: preserve ingress frame ordering

Git-commit: ab046a5d4be4c90a3952a0eae75617b49c0cb01b

Patch-mainline: 5.7-rc5

References: networking-stable-20_05_12

MACsec decryption always occurs in a softirq context. Since

the FPU may not be usable in the softirq context, the call to

decrypt may be scheduled on the cryptd work queue. The cryptd

work queue does not provide ordering guarantees. Therefore,

preserving order requires masking out ASYNC implementations

of gcm(aes).

For instance, an Intel CPU with AES-NI makes available the

generic-gcm-aesni driver from the aesni_intel module to

implement gcm(aes). However, this implementation requires

the FPU, so it is not always available to use from a softirq

context, and will fallback to the cryptd work queue, which

does not preserve frame ordering. With this change, such a

system would select gcm_base(ctr(aes-aesni),ghash-generic).

While the aes-aesni implementation prefers to use the FPU, it

will fallback to the aes-asm implementation if unavailable.

By using a synchronous version of gcm(aes), the decryption

will complete before returning from crypto_aead_decrypt().

Therefore, the macsec_decrypt_done() callback will be called

before returning from macsec_decrypt(). Thus, the order of

calls to macsec_post_decrypt() for the frames is preserved.

While it's presumable that the pure AES-NI version of gcm(aes)

is more performant, the hybrid solution is capable of gigabit

speeds on modest hardware. Regardless, preserving the order

of frames is paramount for many network protocols (e.g.,

triggering TCP retries). Within the MACsec driver itself, the

replay protection is tripped by the out-of-order frames, and

can cause frames to be dropped.

This bug has been present in this code since it was added in

v4.6, however it may not have been noticed since not all CPUs

have FPU offload available. Additionally, the bug manifests

as occasional out-of-order packets that are easily

misattributed to other network phenomena.

When this code was added in v4.6, the crypto/gcm.c code did

not restrict selection of the ghash function based on the

ASYNC flag. For instance, x86 CPUs with PCLMULQDQ would

select the ghash-clmulni driver instead of ghash-generic,

which submits to the cryptd work queue if the FPU is busy.

However, this bug was was corrected in v4.8 by commit

b30bdfa86431afbafe15284a3ad5ac19b49b88e3, and was backported

all the way back to the v3.14 stable branch, so this patch

should be applicable back to the v4.6 stable branch.

Signed-off-by: Scott Dial <scott@scottdial.com>

Signed-off-by: David S. Miller <davem@davemloft.net>

Signed-off-by: Jiri Slaby <jslaby@suse.cz>

---

 drivers/net/macsec.c |    3 ++-

 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/drivers/net/macsec.c

+++ b/drivers/net/macsec.c

@@ -1312,7 +1312,8 @@ static struct crypto_aead *macsec_alloc_

 	struct crypto_aead *tfm;

 	int ret;

-	tfm = crypto_alloc_aead("gcm(aes)", 0, 0);

+	/* Pick a sync gcm(aes) cipher to ensure order is preserved. */

+	tfm = crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC);

 	if (IS_ERR(tfm))

 		return tfm;