On Thu, Jun 01, 2017 at 02:25:24PM -0400, Joe Lawrence wrote: > Add three exported API for livepatch modules: > > void *klp_shadow_attach(void *obj, char *var, gfp_t gfp, void *data); > void klp_shadow_detach(void *obj, char *var); > void *klp_shadow_get(void *obj, char *var); > > that implement "shadow" variables, which allow callers to associate new > shadow fields to existing data structures. > > Signed-off-by: Joe Lawrence <joe.lawrence@xxxxxxxxxx> Overall the patch looks good to me. It's a simple API but we've found it to be very useful for certain patches. One comment below: > +void *klp_shadow_attach(void *obj, char *var, gfp_t gfp, void *data) > +{ > + unsigned long flags; > + struct klp_shadow *shadow; > + > + shadow = kmalloc(sizeof(*shadow), gfp); > + if (!shadow) > + return NULL; > + > + shadow->obj = obj; > + > + shadow->var = kstrdup(var, gfp); > + if (!shadow->var) { > + kfree(shadow); > + return NULL; > + } > + > + shadow->data = data; > + > + spin_lock_irqsave(&klp_shadow_lock, flags); > + hash_add_rcu(klp_shadow_hash, &shadow->node, (unsigned long)obj); > + spin_unlock_irqrestore(&klp_shadow_lock, flags); > + > + return shadow->data; > +} > +EXPORT_SYMBOL_GPL(klp_shadow_attach); I wonder if we should worry about people misusing the API by calling klp_shadow_attach() for a shadow variable that already exists. Maybe we should add a check and return NULL if it already exists. -- Josh -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html