From nobody Sat Oct 4 15:52:46 2025 Received: from mail-qt1-f202.google.com (mail-qt1-f202.google.com [209.85.160.202]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7AAE62FE061 for ; Thu, 14 Aug 2025 12:45:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.160.202 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755175522; cv=none; b=JlNQMGbtk/39FSTxArGecztJI8xUfOk8sHYJCztxwphRb8cpBce2pcNbxD/Y452Pil4CT+z7aZKYCIIHr44f78i6OR6/l4/mQ3qGbIEKtmv6t3tmmj+hV25xfpKG5Iz8OZTsaB8LH3PFVBUGvLMZeEgn4w0h1Jm9GLfwGz+RO20= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755175522; c=relaxed/simple; bh=7ahjSTBeLjUbFf0Bwlancwo9DYFeqGOSSspEEh+y9GU=; h=Date:Mime-Version:Message-ID:Subject:From:To:Cc:Content-Type; b=BjCXWaD58RL4MrHA6TqVvJt71GjI8fOMvCFQYaxR2HQqrJy5TlBhGom/pbzPYjGhnzf4A7BWHSUFn1/2cH6vuxV8StQMQWjUyHTNTGyBp2tE/64Zt5Ia7YToxfM0sfxDaNCauWSatnakVVKrEg1u+6emRebiT1f8XDT1CZHRzSI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--marievic.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=r+6uURxj; arc=none smtp.client-ip=209.85.160.202 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--marievic.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="r+6uURxj" Received: by mail-qt1-f202.google.com with SMTP id d75a77b69052e-4b109912dd8so8410971cf.0 for ; Thu, 14 Aug 2025 05:45:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1755175518; x=1755780318; darn=vger.kernel.org; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=Kbxup2Slgkw+15cQ2AptlaSh/NObK/IYFeU45ShOWVQ=; b=r+6uURxjoVihurCAEdRwc7gKxa/k6NeJyPPQNBAxJu7SlxEiHuD8DjYbLq5tfe7XBR L+BbJJtIQNKRULW2Ke0qjkj9VJjTseQ14w9sV9luEYOg66XFU0z3BEUayxc7KtUpWmGg rlPaUEyOu9hs9t++6qp60qv7VxQtjDyaiKtSFYI58ycYJFn14hKlj7NKp29+9Ha+SNp2 dy6DhqpGJ8VsFXQCPO+buRYa0Cd3h6WfJX/7CBvfg/iohdIVsAhHIkPKyWGeX6jjuFHi hMfONA4mMQ7MV5+EgPiLgyU5y11I33WE72kyz1UOYnq4sUu2pZtSh6jvRUVPz5Pq+Oad OFrg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1755175518; x=1755780318; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=Kbxup2Slgkw+15cQ2AptlaSh/NObK/IYFeU45ShOWVQ=; b=VhNqzueUMOwx91EOjJDKJ5rv7avZma1wLVBGmxc9g8JCw+W+57tWY6i2CNIHAyu4kA kaTV1QTvbsRobgb97wtKnxz/SscB/wZkT56h19IMB0KNZTqRjvpQGW8FgMTheZYwhUrE NG+/UYWtEkK4Z1bfb8gHqI4CZRYlt6NJ+sWNMacBLqbxjNHyHU38ZVEhuxxBE5gk4D69 5G5EC3uk9E3/NaSKSuMIZhxRVQNnvZyhCGT1U/oaSzeZHDX9XeI9oYEQcCIrdwBFpQ1M druBvRwGSDkx3XrmJcE60doS4KZ8vIgA7NrrvLch/CWNdcmcIXIXuQ8Fv/QMdOz/5Y0Q JHaQ== X-Forwarded-Encrypted: i=1; AJvYcCWQ+OZ+z2ZpLpY4yMU79TjsOplqj+USJ6Q5+d02FVQ7oPoCydzmu0u0XCc+7XEVQ/xBKhDyYerx7RwK2Tc=@vger.kernel.org X-Gm-Message-State: AOJu0Yy4S/LsTyVsWGV7RHkeQsQVlxtx2zhhLOjJzc33Tk9ej7DksnYk jcifN9keJxzWaC9JWbHB9GwcZg5BaxHpE80T7KUg1EvjNCkTgTHX4IlYA8IbSU6D/g/z8Vo0VMg xRBJbLwi7W7cunQ== X-Google-Smtp-Source: AGHT+IFQWg3RSRtRCxGs5L40ORJsSddDPfQoU7dkTFdZxUsQqbjLeAOB9hy54Cn68eZ7ybyS2Rk3hI368D3QIA== X-Received: from qtxf4.prod.google.com ([2002:ac8:5d04:0:b0:4ab:7d4c:827c]) (user=marievic job=prod-delivery.src-stubby-dispatcher) by 2002:a05:622a:1887:b0:4a9:cff3:68a2 with SMTP id d75a77b69052e-4b10aa6271bmr45090061cf.37.1755175517877; Thu, 14 Aug 2025 05:45:17 -0700 (PDT) Date: Thu, 14 Aug 2025 12:44:59 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 X-Mailer: git-send-email 2.51.0.rc0.215.g125493bb4a-goog Message-ID: <20250814124459.3484753-1-marievic@google.com> Subject: [RFC PATCH] kunit: Put checking for existing named resource between locks to prevent race conditions in the Resource API From: Marie Zhussupova To: rmoar@google.com, davidgow@google.com, shuah@kernel.org, brendan.higgins@linux.dev Cc: linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com, linux-kernel@vger.kernel.org, Marie Zhussupova Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Named resources attached to a KUnit test must be unique. Currently, the check for existing named resources in kunit_add_named_resource() is performed without a lock. While KUnit is largely single-threaded, a race condition could theoretically lead to multiple resources with the same name being added. To prevent this, the uniqueness check logic has been moved into the locked portion of the __kunit_add_resource() function. This ensures that the check and resource addition are atomic, preventing duplicate named resources. Signed-off-by: Marie Zhussupova --- This patch raises a fundamental question about KUnit's concurrency model. What are the thread-safety guarantees that KUnit should provide? Should these guarantees be formally defined before we introduce changes like this one? It would be great to have everyone's thoughts on this, as it could have implications on how we have handle concurrency in KUnit going forward. --- include/kunit/resource.h | 84 ++++++++++++++++++++++++++++++---------- lib/kunit/resource.c | 12 +++++- 2 files changed, 74 insertions(+), 22 deletions(-) diff --git a/include/kunit/resource.h b/include/kunit/resource.h index 4ad69a2642a5..7b33f332769c 100644 --- a/include/kunit/resource.h +++ b/include/kunit/resource.h @@ -148,12 +148,14 @@ static inline void kunit_put_resource(struct kunit_re= source *res) * If an init function is supplied, @data is passed to it instead. * @free: a user-supplied function to free the resource (if needed). * @res: The resource. + * @name: name of the resource if there is one. * @data: value to pass to init function or set in resource data field. */ int __kunit_add_resource(struct kunit *test, kunit_resource_init_t init, kunit_resource_free_t free, struct kunit_resource *res, + const char *name, void *data); =20 /** @@ -173,7 +175,7 @@ static inline int kunit_add_resource(struct kunit *test, void *data) { res->should_kfree =3D false; - return __kunit_add_resource(test, init, free, res, data); + return __kunit_add_resource(test, init, free, res, NULL, data); } =20 static inline struct kunit_resource * @@ -195,21 +197,14 @@ static inline int kunit_add_named_resource(struct kun= it *test, const char *name, void *data) { - struct kunit_resource *existing; =20 if (!name) return -EINVAL; =20 - existing =3D kunit_find_named_resource(test, name); - if (existing) { - kunit_put_resource(existing); - return -EEXIST; - } - res->name =3D name; res->should_kfree =3D false; =20 - return __kunit_add_resource(test, init, free, res, data); + return __kunit_add_resource(test, init, free, res, name, data); } =20 /** @@ -249,7 +244,7 @@ kunit_alloc_and_get_resource(struct kunit *test, =20 res->should_kfree =3D true; =20 - ret =3D __kunit_add_resource(test, init, free, res, context); + ret =3D __kunit_add_resource(test, init, free, res, NULL, context); if (!ret) { /* * bump refcount for get; kunit_resource_put() should be called @@ -290,7 +285,7 @@ static inline void *kunit_alloc_resource(struct kunit *= test, return NULL; =20 res->should_kfree =3D true; - if (!__kunit_add_resource(test, init, free, res, context)) + if (!__kunit_add_resource(test, init, free, res, NULL, context)) return res->data; =20 return NULL; @@ -314,29 +309,54 @@ static inline bool kunit_resource_name_match(struct k= unit *test, } =20 /** - * kunit_find_resource() - Find a resource using match function/data. + * kunit_find_resource_unlocked() - Find a resource using match function/d= ata + * without locking. * @test: Test case to which the resource belongs. - * @match: match function to be applied to resources/match data. - * @match_data: data to be used in matching. + * @match: Match function to be applied to resources/match data. + * @match_data: Data to be used in matching. + * + * Finds a resource in @test->resources using @match, but does not acquire + * the test's lock. + * + * Note: This function is for specialized use only as it is **NOT thread-s= afe** + * regarding the test's resource list and test reference count. Callers mu= st prevent + * potential race conditions, typically by providing external locking. The= thread-safe + * alternative, kunit_find_resource(), should be used in most situations. */ static inline struct kunit_resource * -kunit_find_resource(struct kunit *test, - kunit_resource_match_t match, - void *match_data) +kunit_find_resource_unlocked(struct kunit *test, + kunit_resource_match_t match, + void *match_data) { struct kunit_resource *res, *found =3D NULL; - unsigned long flags; - - spin_lock_irqsave(&test->lock, flags); =20 list_for_each_entry_reverse(res, &test->resources, node) { - if (match(test, res, (void *)match_data)) { + if (match(test, res, match_data)) { found =3D res; kunit_get_resource(found); break; } } =20 + return found; +} + +/** + * kunit_find_resource() - Find a resource using match function/data. + * @test: Test case to which the resource belongs. + * @match: match function to be applied to resources/match data. + * @match_data: data to be used in matching. + */ +static inline struct kunit_resource * +kunit_find_resource(struct kunit *test, + kunit_resource_match_t match, + void *match_data) +{ + struct kunit_resource *found =3D NULL; + unsigned long flags; + + spin_lock_irqsave(&test->lock, flags); + found =3D kunit_find_resource_unlocked(test, match, match_data); spin_unlock_irqrestore(&test->lock, flags); =20 return found; @@ -355,6 +375,28 @@ kunit_find_named_resource(struct kunit *test, (void *)name); } =20 +/** + * kunit_find_named_resource_unlocked() - Find a resource using match name + * without locking. + * @test: Test case to which the resource belongs. + * @name: Match name. + * + * Finds a resource in @test->resources using its name, but does not acqui= re + * the test's resource lock. + * + * Note: This function is for specialized use only as it is **NOT thread-s= afe** + * regarding the test's resource list and test reference count. Callers mu= st prevent + * potential race conditions, typically by providing external locking. The= thread-safe + * alternative, kunit_find_named_resource(), should be used in most situat= ions. + */ +static inline struct kunit_resource * +kunit_find_named_resource_unlocked(struct kunit *test, + const char *name) +{ + return kunit_find_resource_unlocked(test, kunit_resource_name_match, + (void *)name); +} + /** * kunit_destroy_resource() - Find a kunit_resource and destroy it. * @test: Test case to which the resource belongs. diff --git a/lib/kunit/resource.c b/lib/kunit/resource.c index f0209252b179..dae708f81f97 100644 --- a/lib/kunit/resource.c +++ b/lib/kunit/resource.c @@ -20,10 +20,12 @@ int __kunit_add_resource(struct kunit *test, kunit_resource_init_t init, kunit_resource_free_t free, struct kunit_resource *res, + const char *name, void *data) { int ret =3D 0; unsigned long flags; + struct kunit_resource *existing; =20 res->free =3D free; kref_init(&res->refcount); @@ -37,6 +39,14 @@ int __kunit_add_resource(struct kunit *test, } =20 spin_lock_irqsave(&test->lock, flags); + if (name) { + existing =3D kunit_find_named_resource_unlocked(test, name); + if (existing) { + kunit_put_resource(existing); + spin_unlock_irqrestore(&test->lock, flags); + return -EEXIST; + } + } list_add_tail(&res->node, &test->resources); /* refcount for list is established by kref_init() */ spin_unlock_irqrestore(&test->lock, flags); @@ -107,7 +117,7 @@ int kunit_add_action(struct kunit *test, void (*action)= (void *), void *ctx) =20 action_ctx->res.should_kfree =3D true; /* As init is NULL, this cannot fail. */ - __kunit_add_resource(test, NULL, __kunit_action_free, &action_ctx->res, a= ction_ctx); + __kunit_add_resource(test, NULL, __kunit_action_free, &action_ctx->res, N= ULL, action_ctx); =20 return 0; } --=20 2.51.0.rc0.215.g125493bb4a-goog