From: Petr Mladek <pmladek@xxxxxxxx> 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@xxxxxxxx> Signed-off-by: Marcos Paulo de Souza <mpdesouza@xxxxxxxx> --- 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