diff --git a/Documentation/livepatch/shadow-vars.txt b/Documentation/livepatch/shadow-vars.txt index 89c66634d600..ecc09a7be5dd 100644 --- a/Documentation/livepatch/shadow-vars.txt +++ b/Documentation/livepatch/shadow-vars.txt @@ -34,9 +34,13 @@ meta-data and shadow-data: - data[] - storage for shadow data It is important to note that the klp_shadow_alloc() and -klp_shadow_get_or_alloc() calls, described below, store a *copy* of the -data that the functions are provided. Callers should provide whatever -mutual exclusion is required of the shadow data. +klp_shadow_get_or_alloc() are zeroing the variable by default. +They also allow to call a custom constructor function when a non-zero +value is needed. Callers should provide whatever mutual exclusion +is required. + +Note that the constructor is called under klp_shadow_lock spinlock. It allows +to do actions that can be done only once when a new variable is allocated. * klp_shadow_get() - retrieve a shadow variable data pointer - search hashtable for pair @@ -47,7 +51,7 @@ mutual exclusion is required of the shadow data. - WARN and return NULL - if doesn't already exist - allocate a new shadow variable - - copy data into the new shadow variable + - initialize the variable using a custom constructor and data when provided - add to the global hashtable * klp_shadow_get_or_alloc() - get existing or alloc a new shadow variable @@ -56,16 +60,20 @@ mutual exclusion is required of the shadow data. - return existing shadow variable - if doesn't already exist - allocate a new shadow variable - - copy data into the new shadow variable + - initialize the variable using a custom constructor and data when provided - add pair to the global hashtable * klp_shadow_free() - detach and free a shadow variable - find and remove a reference from global hashtable - - if found, free shadow variable + - if found + - call destructor function if defined + - free shadow variable * klp_shadow_free_all() - detach and free all <*, id> shadow variables - find and remove any <*, id> references from global hashtable - - if found, free shadow variable + - if found + - call destructor function if defined + - free shadow variable 2. Use cases @@ -107,7 +115,8 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata, sta = kzalloc(sizeof(*sta) + hw->sta_data_size, gfp); /* Attach a corresponding shadow variable, then initialize it */ - ps_lock = klp_shadow_alloc(sta, PS_LOCK, NULL, sizeof(*ps_lock), gfp); + ps_lock = klp_shadow_alloc(sta, PS_LOCK, sizeof(*ps_lock), gfp, + NULL, NULL); if (!ps_lock) goto shadow_fail; spin_lock_init(ps_lock); @@ -131,7 +140,7 @@ variable: void sta_info_free(struct ieee80211_local *local, struct sta_info *sta) { - klp_shadow_free(sta, PS_LOCK); + klp_shadow_free(sta, PS_LOCK, NULL); kfree(sta); ... @@ -148,16 +157,24 @@ shadow variables to parents already in-flight. For commit 1d147bfa6429, a good spot to allocate a shadow spinlock is inside ieee80211_sta_ps_deliver_wakeup(): +int ps_lock_shadow_ctor(void *obj, void *shadow_data, void *ctor_data) +{ + spinlock_t *lock = shadow_data; + + spin_lock_init(lock); + return 0; +} + #define PS_LOCK 1 void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta) { - DEFINE_SPINLOCK(ps_lock_fallback); spinlock_t *ps_lock; /* sync with ieee80211_tx_h_unicast_ps_buf */ ps_lock = klp_shadow_get_or_alloc(sta, PS_LOCK, - &ps_lock_fallback, sizeof(ps_lock_fallback), - GFP_ATOMIC); + sizeof(*ps_lock), GFP_ATOMIC, + ps_lock_shadow_ctor, NULL); + if (ps_lock) spin_lock(ps_lock); ... diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index 4754f01c1abb..aec44b1d9582 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -186,13 +186,20 @@ static inline bool klp_have_reliable_stack(void) IS_ENABLED(CONFIG_HAVE_RELIABLE_STACKTRACE); } +typedef int (*klp_shadow_ctor_t)(void *obj, + void *shadow_data, + void *ctor_data); +typedef void (*klp_shadow_dtor_t)(void *obj, void *shadow_data); + void *klp_shadow_get(void *obj, unsigned long id); -void *klp_shadow_alloc(void *obj, unsigned long id, void *data, - size_t size, gfp_t gfp_flags); -void *klp_shadow_get_or_alloc(void *obj, unsigned long id, void *data, - size_t size, gfp_t gfp_flags); -void klp_shadow_free(void *obj, unsigned long id); -void klp_shadow_free_all(unsigned long id); +void *klp_shadow_alloc(void *obj, unsigned long id, + size_t size, gfp_t gfp_flags, + klp_shadow_ctor_t ctor, void *ctor_data); +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); +void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor); +void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor); #else /* !CONFIG_LIVEPATCH */ diff --git a/kernel/livepatch/shadow.c b/kernel/livepatch/shadow.c index fdac27588d60..83958c814439 100644 --- a/kernel/livepatch/shadow.c +++ b/kernel/livepatch/shadow.c @@ -113,8 +113,10 @@ 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, void *data, - size_t size, gfp_t gfp_flags, bool warn_on_exist) +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; @@ -125,18 +127,15 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id, void *data, if (shadow_data) goto exists; - /* Allocate a new shadow variable for use inside the lock below */ + /* + * 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; - new_shadow->obj = obj; - new_shadow->id = id; - - /* Initialize the shadow variable if data provided */ - if (data) - memcpy(new_shadow->data, data, size); - /* Look for again under the lock */ spin_lock_irqsave(&klp_shadow_lock, flags); shadow_data = klp_shadow_get(obj, id); @@ -150,6 +149,22 @@ static void *__klp_shadow_get_or_alloc(void *obj, unsigned long id, void *data, goto exists; } + new_shadow->obj = obj; + new_shadow->id = id; + + if (ctor) { + int err; + + 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; + } + } + /* No found, so attach the newly allocated one */ hash_add_rcu(klp_shadow_hash, &new_shadow->node, (unsigned long)new_shadow->obj); @@ -170,26 +185,32 @@ exists: * klp_shadow_alloc() - allocate and add a new shadow variable * @obj: pointer to parent object * @id: data identifier - * @data: pointer to data to attach to parent * @size: size of attached data * @gfp_flags: GFP mask for allocation + * @ctor: custom constructor to initialize the shadow data (optional) + * @ctor_data: pointer to any data needed by @ctor (optional) * - * Allocates @size bytes for new shadow variable data using @gfp_flags - * and copies @size bytes from @data into the new shadow variable's own - * data space. If @data is NULL, @size bytes are still allocated, but - * no copy is performed. The new shadow variable is then added to the - * global hashtable. + * Allocates @size bytes for new shadow variable data using @gfp_flags. + * The data are zeroed by default. They are further initialized by @ctor + * function if it is not NULL. The new shadow variable is then added + * to the global hashtable. * - * If an existing shadow variable can be found, this routine - * will issue a WARN, exit early and return NULL. + * If an existing shadow variable can be found, this routine will + * issue a WARN, exit early and return NULL. + * + * This function guarantees that the constructor function is called only when + * the variable did not exist before. The cost is that @ctor is called + * in atomic context under a spin lock. * * Return: the shadow variable data element, NULL on duplicate or * failure. */ -void *klp_shadow_alloc(void *obj, unsigned long id, void *data, - size_t size, gfp_t gfp_flags) +void *klp_shadow_alloc(void *obj, unsigned long id, + size_t size, gfp_t gfp_flags, + klp_shadow_ctor_t ctor, void *ctor_data) { - return __klp_shadow_get_or_alloc(obj, id, data, size, gfp_flags, true); + return __klp_shadow_get_or_alloc(obj, id, size, gfp_flags, + ctor, ctor_data, true); } EXPORT_SYMBOL_GPL(klp_shadow_alloc); @@ -197,37 +218,51 @@ EXPORT_SYMBOL_GPL(klp_shadow_alloc); * klp_shadow_get_or_alloc() - get existing or allocate a new shadow variable * @obj: pointer to parent object * @id: data identifier - * @data: pointer to data to attach to parent * @size: size of attached data * @gfp_flags: GFP mask for allocation + * @ctor: custom constructor to initialize the shadow data (optional) + * @ctor_data: pointer to any data needed by @ctor (optional) * * Returns a pointer to existing shadow data if an shadow * variable is already present. Otherwise, it creates a new shadow * variable like klp_shadow_alloc(). * - * This function guarantees that only one shadow variable exists with - * the given @id for the given @obj. It also guarantees that the shadow - * variable will be initialized by the given @data only when it did not - * exist before. + * This function guarantees that only one shadow variable exists with the given + * @id for the given @obj. It also guarantees that the constructor function + * will be called only when the variable did not exist before. The cost is + * that @ctor is called in atomic context under a spin lock. * * Return: the shadow variable data element, NULL on failure. */ -void *klp_shadow_get_or_alloc(void *obj, unsigned long id, void *data, - size_t size, gfp_t gfp_flags) +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) { - return __klp_shadow_get_or_alloc(obj, id, data, size, gfp_flags, false); + return __klp_shadow_get_or_alloc(obj, id, size, gfp_flags, + ctor, ctor_data, false); } EXPORT_SYMBOL_GPL(klp_shadow_get_or_alloc); +static void klp_shadow_free_struct(struct klp_shadow *shadow, + klp_shadow_dtor_t dtor) +{ + hash_del_rcu(&shadow->node); + if (dtor) + dtor(shadow->obj, shadow->data); + kfree_rcu(shadow, rcu_head); +} + /** * klp_shadow_free() - detach and free a shadow variable * @obj: pointer to parent object * @id: data identifier + * @dtor: custom callback that can be used to unregister the variable + * and/or free data that the shadow variable points to (optional) * * This function releases the memory for this shadow variable * instance, callers should stop referencing it accordingly. */ -void klp_shadow_free(void *obj, unsigned long id) +void klp_shadow_free(void *obj, unsigned long id, klp_shadow_dtor_t dtor) { struct klp_shadow *shadow; unsigned long flags; @@ -239,8 +274,7 @@ void klp_shadow_free(void *obj, unsigned long id) (unsigned long)obj) { if (klp_shadow_match(shadow, obj, id)) { - hash_del_rcu(&shadow->node); - kfree_rcu(shadow, rcu_head); + klp_shadow_free_struct(shadow, dtor); break; } } @@ -252,11 +286,13 @@ EXPORT_SYMBOL_GPL(klp_shadow_free); /** * klp_shadow_free_all() - detach and free all <*, id> shadow variables * @id: data identifier + * @dtor: custom callback that can be used to unregister the variable + * and/or free data that the shadow variable points to (optional) * * This function releases the memory for all <*, id> shadow variable * instances, callers should stop referencing them accordingly. */ -void klp_shadow_free_all(unsigned long id) +void klp_shadow_free_all(unsigned long id, klp_shadow_dtor_t dtor) { struct klp_shadow *shadow; unsigned long flags; @@ -266,10 +302,8 @@ void klp_shadow_free_all(unsigned long id) /* Delete all <*, id> from hash */ hash_for_each(klp_shadow_hash, i, shadow, node) { - if (klp_shadow_match(shadow, shadow->obj, id)) { - hash_del_rcu(&shadow->node); - kfree_rcu(shadow, rcu_head); - } + if (klp_shadow_match(shadow, shadow->obj, id)) + klp_shadow_free_struct(shadow, dtor); } spin_unlock_irqrestore(&klp_shadow_lock, flags); diff --git a/samples/livepatch/livepatch-shadow-fix1.c b/samples/livepatch/livepatch-shadow-fix1.c index 830c55514f9f..49b13553eaae 100644 --- a/samples/livepatch/livepatch-shadow-fix1.c +++ b/samples/livepatch/livepatch-shadow-fix1.c @@ -56,6 +56,21 @@ struct dummy { unsigned long jiffies_expire; }; +/* + * The constructor makes more sense together with klp_shadow_get_or_alloc(). + * In this example, it would be safe to assign the pointer also to the shadow + * variable returned by klp_shadow_alloc(). But we wanted to show the more + * complicated use of the API. + */ +static int shadow_leak_ctor(void *obj, void *shadow_data, void *ctor_data) +{ + void **shadow_leak = shadow_data; + void *leak = ctor_data; + + *shadow_leak = leak; + return 0; +} + struct dummy *livepatch_fix1_dummy_alloc(void) { struct dummy *d; @@ -74,7 +89,8 @@ struct dummy *livepatch_fix1_dummy_alloc(void) * pointer to handle resource release. */ leak = kzalloc(sizeof(int), GFP_KERNEL); - klp_shadow_alloc(d, SV_LEAK, &leak, sizeof(leak), GFP_KERNEL); + klp_shadow_alloc(d, SV_LEAK, sizeof(leak), GFP_KERNEL, + shadow_leak_ctor, leak); pr_info("%s: dummy @ %p, expires @ %lx\n", __func__, d, d->jiffies_expire); @@ -82,9 +98,19 @@ struct dummy *livepatch_fix1_dummy_alloc(void) return d; } +static void livepatch_fix1_dummy_leak_dtor(void *obj, void *shadow_data) +{ + void *d = obj; + void **shadow_leak = shadow_data; + + kfree(*shadow_leak); + pr_info("%s: dummy @ %p, prevented leak @ %p\n", + __func__, d, *shadow_leak); +} + void livepatch_fix1_dummy_free(struct dummy *d) { - void **shadow_leak, *leak; + void **shadow_leak; /* * Patch: fetch the saved SV_LEAK shadow variable, detach and @@ -93,15 +119,10 @@ void livepatch_fix1_dummy_free(struct dummy *d) * was loaded.) */ shadow_leak = klp_shadow_get(d, SV_LEAK); - if (shadow_leak) { - leak = *shadow_leak; - klp_shadow_free(d, SV_LEAK); - kfree(leak); - pr_info("%s: dummy @ %p, prevented leak @ %p\n", - __func__, d, leak); - } else { + if (shadow_leak) + klp_shadow_free(d, SV_LEAK, livepatch_fix1_dummy_leak_dtor); + else pr_info("%s: dummy @ %p leaked!\n", __func__, d); - } kfree(d); } @@ -147,7 +168,7 @@ static int livepatch_shadow_fix1_init(void) static void livepatch_shadow_fix1_exit(void) { /* Cleanup any existing SV_LEAK shadow variables */ - klp_shadow_free_all(SV_LEAK); + klp_shadow_free_all(SV_LEAK, livepatch_fix1_dummy_leak_dtor); WARN_ON(klp_unregister_patch(&patch)); } diff --git a/samples/livepatch/livepatch-shadow-fix2.c b/samples/livepatch/livepatch-shadow-fix2.c index ff9948f0ec00..b34c7bf83356 100644 --- a/samples/livepatch/livepatch-shadow-fix2.c +++ b/samples/livepatch/livepatch-shadow-fix2.c @@ -53,39 +53,42 @@ struct dummy { bool livepatch_fix2_dummy_check(struct dummy *d, unsigned long jiffies) { int *shadow_count; - int count; /* * Patch: handle in-flight dummy structures, if they do not * already have a SV_COUNTER shadow variable, then attach a * new one. */ - count = 0; shadow_count = klp_shadow_get_or_alloc(d, SV_COUNTER, - &count, sizeof(count), - GFP_NOWAIT); + sizeof(*shadow_count), GFP_NOWAIT, + NULL, NULL); if (shadow_count) *shadow_count += 1; return time_after(jiffies, d->jiffies_expire); } +static void livepatch_fix2_dummy_leak_dtor(void *obj, void *shadow_data) +{ + void *d = obj; + void **shadow_leak = shadow_data; + + kfree(*shadow_leak); + pr_info("%s: dummy @ %p, prevented leak @ %p\n", + __func__, d, *shadow_leak); +} + void livepatch_fix2_dummy_free(struct dummy *d) { - void **shadow_leak, *leak; + void **shadow_leak; int *shadow_count; /* Patch: copy the memory leak patch from the fix1 module. */ shadow_leak = klp_shadow_get(d, SV_LEAK); - if (shadow_leak) { - leak = *shadow_leak; - klp_shadow_free(d, SV_LEAK); - kfree(leak); - pr_info("%s: dummy @ %p, prevented leak @ %p\n", - __func__, d, leak); - } else { + if (shadow_leak) + klp_shadow_free(d, SV_LEAK, livepatch_fix2_dummy_leak_dtor); + else pr_info("%s: dummy @ %p leaked!\n", __func__, d); - } /* * Patch: fetch the SV_COUNTER shadow variable and display @@ -95,7 +98,7 @@ void livepatch_fix2_dummy_free(struct dummy *d) if (shadow_count) { pr_info("%s: dummy @ %p, check counter = %d\n", __func__, d, *shadow_count); - klp_shadow_free(d, SV_COUNTER); + klp_shadow_free(d, SV_COUNTER, NULL); } kfree(d); @@ -142,7 +145,7 @@ static int livepatch_shadow_fix2_init(void) static void livepatch_shadow_fix2_exit(void) { /* Cleanup any existing SV_COUNTER shadow variables */ - klp_shadow_free_all(SV_COUNTER); + klp_shadow_free_all(SV_COUNTER, NULL); WARN_ON(klp_unregister_patch(&patch)); }