mirrors/scst
1
0
Fork 0
mirror of https://github.com/SCST-project/scst.git synced 2026-10-22 06:02:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

scst_mem: Acquire shared pool references only while alive

Browse Source 
Coccinelle api/atomic_as_refcounter.cocci reports:

  scst/src/scst_mem.c:1642:5-24:
  WARNING: atomic_dec_and_test variation before object free at line 1643.

Auditing this ownership counter exposes two bugs in shared pool lookup.
A lookup with a different owner mm returns the existing pool without
taking a reference. Its caller can then release another device's
ownership. Reject that lookup with NULL.

The last put can also reach zero before the destructor acquires
sgv_pools_mutex to unlink the pool. A concurrent lookup can still find
that pool. Use kref_get_unless_zero under the lookup mutex to reject a
pool whose destruction has already started.

Convert sgv_pool_ref to kref, retaining the existing nullable cleanup for
initialization failures separately from the last-reference callback.
The kref API also works on supported kernels predating refcount_t.

Add KUnit cases for same-mm acquisition, different-mm rejection and the
zero-reference state before unlinking. Export the private test header
in generated in-tree patches as well as the out-of-tree build.
This commit is contained in:
Gleb Chesnokov
2026-10-20 18:02:29 +03:00
parent 43ee1a7c5d
commit f175f5408f
6 changed files with 117 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/kernel/generate-kernel-patch
+2 -1
View File
@@ -325,7 +325,8 @@ scst_07_pres="scst/src/scst_pres.h scst/src/scst_pres.c"
scst_08_sysfs="scst/src/scst_sysfs.c \
scst/src/scst_sysfs_test.h"
scst_09_debug="scst/include/scst_debug.h scst/src/scst_debug.c"
scst_10_sgv="scst/include/scst_sgv.h scst/src/scst_mem.h scst/src/scst_mem.c doc/scst_pg.sgml"
scst_10_sgv="scst/include/scst_sgv.h scst/src/scst_mem.h scst/src/scst_mem.c \
scst/src/scst_mem_test.h doc/scst_pg.sgml"
scst_user="scst/include/scst_user.h scst/src/dev_handlers/scst_user_main.c \
scst/src/dev_handlers/scst_user_test.h"
scst_13_vdisk="scst/src/dev_handlers/scst_vdisk_main.c \
scst/src/kunit/lib/suites/scst_kunit_sgv.c
+68
View File
@@ -0,0 +1,68 @@
// SPDX-License-Identifier: GPL-2.0-only
#include "../../scst_kunit.h"
#include <linux/mm.h>
#include <linux/sched.h>
#ifdef INSIDE_KERNEL_TREE
#include <scst/scst.h>
#else
#include "scst.h"
#endif
#include "../../../scst_mem.h"
#include "../../../scst_mem_test.h"
static void sgv_shared_rejects_other_mm(struct kunit *test)
{
struct sgv_pool *pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL);
struct mm_struct *other_mm = kunit_kzalloc(test, sizeof(*other_mm), GFP_KERNEL);
KUNIT_ASSERT_NOT_NULL(test, pool);
KUNIT_ASSERT_NOT_NULL(test, other_mm);
pool->owner_mm = other_mm;
kref_init(&pool->sgv_pool_ref);
/* A failed lookup must not return the original owner's reference. */
KUNIT_EXPECT_PTR_EQ(test, sgv_pool_get_shared(pool), NULL);
KUNIT_EXPECT_EQ(test, kref_read(&pool->sgv_pool_ref), 1);
}
static void sgv_shared_gets_same_mm(struct kunit *test)
{
struct sgv_pool *pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL);
KUNIT_ASSERT_NOT_NULL(test, pool);
pool->owner_mm = current->mm;
kref_init(&pool->sgv_pool_ref);
KUNIT_EXPECT_PTR_EQ(test, sgv_pool_get_shared(pool), pool);
KUNIT_EXPECT_EQ(test, kref_read(&pool->sgv_pool_ref), 2);
}
static void sgv_defer_release(struct kref *ref)
{
/* Keep the fixture alive to model a destructor waiting to unlink it. */
}
static void sgv_shared_rejects_destroying_pool(struct kunit *test)
{
struct sgv_pool *pool = kunit_kzalloc(test, sizeof(*pool), GFP_KERNEL);
KUNIT_ASSERT_NOT_NULL(test, pool);
pool->owner_mm = current->mm;
/* Last put has reached zero; the destructor has not unlinked it yet. */
kref_init(&pool->sgv_pool_ref);
kref_put(&pool->sgv_pool_ref, sgv_defer_release);
KUNIT_EXPECT_PTR_EQ(test, sgv_pool_get_shared(pool), NULL);
KUNIT_EXPECT_EQ(test, kref_read(&pool->sgv_pool_ref), 0);
}
static struct kunit_case scst_sgv_cases[] = {
KUNIT_CASE(sgv_shared_rejects_other_mm),
KUNIT_CASE(sgv_shared_gets_same_mm),
KUNIT_CASE(sgv_shared_rejects_destroying_pool),
{}
};
scst_kunit_suite(scst_sgv_suite, "LIB", "-sgv",
.test_cases = scst_sgv_cases,
);
scst/src/kunit/objects.mk
+1
View File
@@ -11,6 +11,7 @@ scst-kunit-scst := \
kunit/event/suites/scst_kunit_event_delivery.o \
kunit/lib/suites/scst_kunit_capacity.o \
kunit/lib/suites/scst_kunit_lib.o \
kunit/lib/suites/scst_kunit_sgv.o \
kunit/lib/suites/scst_kunit_write_same.o \
kunit/lifecycle/suites/scst_kunit_lifecycle.o \
kunit/lifecycle/suites/scst_kunit_blocking.o \
scst/src/scst_mem.c
+31 -14
View File
@@ -33,6 +33,7 @@
#endif
#include "scst_priv.h"
#include "scst_mem.h"
#include "scst_mem_test.h"
#undef DEFAULT_SYMBOL_NAMESPACE
#define DEFAULT_SYMBOL_NAMESPACE SCST_NAMESPACE
@@ -1386,7 +1387,7 @@ static int sgv_pool_init(struct sgv_pool *pool, const char *name,
}
}
atomic_set(&pool->sgv_pool_ref, 1);
kref_init(&pool->sgv_pool_ref);
spin_lock_init(&pool->sgv_pool_lock);
INIT_LIST_HEAD(&pool->sorted_recycling_list);
for (i = 0; i < pool->max_caches; i++)
@@ -1506,6 +1507,11 @@ out:
TRACE_EXIT();
}
static void sgv_pool_release(struct kref *ref)
{
sgv_pool_destroy(container_of(ref, struct sgv_pool, sgv_pool_ref));
}
/**
* sgv_pool_set_allocator - set custom pages allocator
* @pool: the cache
@@ -1525,6 +1531,23 @@ void sgv_pool_set_allocator(struct sgv_pool *pool,
}
EXPORT_SYMBOL_GPL(sgv_pool_set_allocator);
/* The caller holds sgv_pools_mutex to protect the lookup result. */
SCST_KUNIT_VISIBLE
struct sgv_pool *sgv_pool_get_shared(struct sgv_pool *pool)
{
if (pool->owner_mm != current->mm) {
PRINT_ERROR("Attempt of a shared use of SGV pool %s with different MM",
pool->name);
return NULL;
}
/* Last put can have committed to destruction before taking the mutex. */
if (!kref_get_unless_zero(&pool->sgv_pool_ref))
return NULL;
return pool;
}
/**
* sgv_pool_create_node - creates and initializes an SGV pool
* @name: the name of the SGV pool
@@ -1581,12 +1604,7 @@ struct sgv_pool *sgv_pool_create_node(const char *name, enum sgv_clustering_type
list_for_each_entry(tp, &sgv_pools_list, sgv_pools_list_entry) {
if (strcmp(tp->name, name) == 0) {
if (shared) {
if (tp->owner_mm != current->mm) {
PRINT_ERROR("Attempt of a shared use of SGV pool %s with different MM",
name);
goto out_free;
}
sgv_pool_get(tp);
tp = sgv_pool_get_shared(tp);
goto out_free;
} else {
PRINT_ERROR("SGV pool %s already exists", name);
@@ -1623,9 +1641,9 @@ EXPORT_SYMBOL_GPL(sgv_pool_create_node);
*/
void sgv_pool_get(struct sgv_pool *pool)
{
atomic_inc(&pool->sgv_pool_ref);
TRACE_MEM("Incrementing sgv pool %p ref (new value %d)",
pool, atomic_read(&pool->sgv_pool_ref));
kref_get(&pool->sgv_pool_ref);
TRACE_MEM("Incrementing sgv pool %p ref (new value %u)",
pool, kref_read(&pool->sgv_pool_ref));
}
EXPORT_SYMBOL_GPL(sgv_pool_get);
@@ -1637,10 +1655,9 @@ EXPORT_SYMBOL_GPL(sgv_pool_get);
*/
void sgv_pool_put(struct sgv_pool *pool)
{
TRACE_MEM("Decrementing sgv pool %p ref (new value %d)",
pool, atomic_read(&pool->sgv_pool_ref) - 1);
if (atomic_dec_and_test(&pool->sgv_pool_ref))
sgv_pool_destroy(pool);
TRACE_MEM("Decrementing sgv pool %p ref (new value %u)",
pool, kref_read(&pool->sgv_pool_ref) - 1);
kref_put(&pool->sgv_pool_ref, sgv_pool_release);
}
EXPORT_SYMBOL_GPL(sgv_pool_put);
scst/src/scst_mem.h
+2 -1
View File
@@ -15,6 +15,7 @@
* GNU General Public License for more details.
*/
#include <linux/kref.h>
#include <linux/scatterlist.h>
#include <linux/workqueue.h>
@@ -113,7 +114,7 @@ struct sgv_pool {
atomic_t big_alloc, big_pages, big_merged;
atomic_t other_alloc, other_pages, other_merged;
atomic_t sgv_pool_ref;
struct kref sgv_pool_ref;
int max_caches;
scst/src/scst_mem_test.h
+13
View File
@@ -0,0 +1,13 @@
/* SPDX-License-Identifier: GPL-2.0-only */
#ifndef SCST_MEM_TEST_H
#define SCST_MEM_TEST_H
#include "scst_test.h"
#ifdef SCST_KUNIT_ENABLED
struct sgv_pool;
struct sgv_pool *sgv_pool_get_shared(struct sgv_pool *pool);
#endif
#endif /* SCST_MEM_TEST_H */