[PATCH v2 1/4] livepatch/shadow: Separate code to get or use pre-allocated shadow variable

Marcos Paulo de Souza
2022-10-26T19:41:47Z

From: Petr Mladek <pmladek@suse.com>

Separate code that is used in klp_shadow_get_or_alloc() under klp_mutex.
It splits a long spaghetti function into two. Also it unifies the error
handling. The old used a mix of duplicated code, direct returns,
and goto. The new code has only one unlock, free, and return calls.

It is code refactoring without any functional changes.

Signed-off-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
Changes from v1:
* Change __klp_shadow_get_or_use to __klp_shadow_get_or_add_locked and add a comment
about it's meaning (Petr)
* Add lockdep_assert_held on __klp_shadow_get_or_add_locked
* Reworked the commit message (Petr)
* Added my SoB (Josh)

kernel/livepatch/shadow.c | 78 ++++++++++++++++++++++-----------------
1 file changed, 45 insertions(+), 33 deletions(-)

diff --git a/kernel/livepatch/shadow.c b/kernel/livepatch/shadow.c
index c2e724d97ddf..81ad7cbbd124 100644
--- a/kernel/livepatch/shadow.c
+++ b/kernel/livepatch/shadow.c
@@ -101,41 +101,22 @@ void *klp_shadow_get(void *obj, unsigned long id)
}
EXPORT_SYMBOL_GPL(klp_shadow_get);

-static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id,
- size_t size, gfp_t gfp_flags,
- klp_shadow_ctor_t ctor, void *ctor_data,
- bool warn_on_exist)
+/* Check if the variable exists. Otherwise, add the pre-allocated one. */
+static void *__klp_shadow_get_or_add_locked(void *obj, unsigned long id,
+ struct klp_shadow *new_shadow,
+ klp_shadow_ctor_t ctor, void *ctor_data,
+ bool *exist)
{
- struct klp_shadow *new_shadow;
void *shadow_data;
- unsigned long flags;

- /* Check if the shadow variable already exists */
- shadow_data = klp_shadow_get(obj, id);
- if (shadow_data)
- goto exists;
-
- /*
- * Allocate a new shadow variable. Fill it with zeroes by default.
- * More complex setting can be done by @ctor function. But it is
- * called only when the buffer is really used (under klp_shadow_lock).
- */
- new_shadow = kzalloc(size + sizeof(*new_shadow), gfp_flags);
- if (!new_shadow)
- return NULL;
+ lockdep_assert_held(&klp_shadow_lock);

- /* Look for <obj, id> again under the lock */
- spin_lock_irqsave(&klp_shadow_lock, flags);
shadow_data = klp_shadow_get(obj, id);
if (unlikely(shadow_data)) {
- /*
- * Shadow variable was found, throw away speculative
- * allocation.
- */
- spin_unlock_irqrestore(&klp_shadow_lock, flags);
- kfree(new_shadow);
- goto exists;
+ *exist = true;
+ return shadow_data;
}
+ *exist = false;

new_shadow->obj = obj;
new_shadow->id = id;
@@ -145,8 +126,6 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id,

err = ctor(obj, new_shadow->data, ctor_data);
if (err) {
- spin_unlock_irqrestore(&klp_shadow_lock, flags);
- kfree(new_shadow);
pr_err("Failed to construct shadow variable <%p, %lx> (%d)\n",
obj, id, err);
return NULL;
@@ -156,12 +135,45 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id,
/* No <obj, id> found, so attach the newly allocated one */
hash_add_rcu(klp_shadow_hash, &new_shadow->node,
(unsigned long)new_shadow->obj);
- spin_unlock_irqrestore(&klp_shadow_lock, flags);

return new_shadow->data;
+}
+
+static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id,
+ size_t size, gfp_t gfp_flags,
+ klp_shadow_ctor_t ctor, void *ctor_data,
+ bool warn_on_exist)
+{
+ struct klp_shadow *new_shadow;
+ void *shadow_data;
+ bool exist;
+ unsigned long flags;
+
+ /* Check if the shadow variable already exists */
+ shadow_data = klp_shadow_get(obj, id);
+ if (shadow_data)
+ return shadow_data;
+
+ /*
+ * Allocate a new shadow variable. Fill it with zeroes by default.
+ * More complex setting can be done by @ctor function. But it is
+ * called only when the buffer is really used (under klp_shadow_lock).
+ */
+ new_shadow = kzalloc(size + sizeof(*new_shadow), gfp_flags);
+ if (!new_shadow)
+ return NULL;
+
+ /* Look for <obj, id> again under the lock */
+ spin_lock_irqsave(&klp_shadow_lock, flags);
+ shadow_data = __klp_shadow_get_or_add_locked(obj, id, new_shadow,
+ ctor, ctor_data, &exist);
+ spin_unlock_irqrestore(&klp_shadow_lock, flags);
+
+ /* Throw away unused speculative allocation. */
+ if (!shadow_data || exist)
+ kfree(new_shadow);

-exists:
- if (warn_on_exist) {
+ if (exist && warn_on_exist) {
WARN(1, "Duplicate shadow variable <%p, %lx>\n", obj, id);
return NULL;
}
--
2.37.3