livepatch: Initialize shadow variables safely by a custom callback

The existing API allows to pass a sample data to initialize the shadow
data. It works well when the data are position independent. But it fails
miserably when we need to set a pointer to the shadow structure itself.

Unfortunately, we might need to initialize the pointer surprisingly
often because of struct list_head. It is even worse because the list
might be hidden in other common structures, for example, struct mutex,
struct wait_queue_head.

For example, this was needed to fix races in ALSA sequencer. It required
to add mutex into struct snd_seq_client. See commit b3defb791b
("ALSA: seq: Make ioctls race-free") and commit d15d662e89
("ALSA: seq: Fix racy pool initializations")

This patch makes the API more safe. A custom constructor function and data
are passed to klp_shadow_*alloc() functions instead of the sample data.

Note that ctor_data are no longer a template for shadow->data. It might
point to any data that might be necessary when the constructor is called.

Also note that the constructor is called under klp_shadow_lock. It is
an internal spin_lock that synchronizes alloc() vs. get() operations,
see klp_shadow_get_or_alloc(). On one hand, this adds a risk of ABBA
deadlocks. On the other hand, it allows to do some operations safely.
For example, we could add the new structure into an existing list.
This must be done only once when the structure is allocated.

Reported-by: Nicolai Stange <nstange@suse.de>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Josh Poimboeuf <jpoimboe@redhat.com>
Acked-by: Miroslav Benes <mbenes@suse.cz>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
This commit is contained in:
Petr Mladek 2018-04-16 13:36:46 +02:00 committed by Jiri Kosina
parent e1c70f3238
commit e91c2518a5
5 changed files with 104 additions and 47 deletions

View file

@ -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 <obj, id> pair
@ -47,7 +51,7 @@ mutual exclusion is required of the shadow data.
- WARN and return NULL
- if <obj, id> 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 <obj, id> to the global hashtable
* klp_shadow_get_or_alloc() - get existing or alloc a new shadow variable
@ -56,7 +60,7 @@ mutual exclusion is required of the shadow data.
- return existing shadow variable
- if <obj, id> 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 <obj, id> pair to the global hashtable
* klp_shadow_free() - detach and free a <obj, id> shadow variable
@ -107,7 +111,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);
@ -148,16 +153,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);
...