Messages in this thread | | | Subject | Re: [PATCH 0/3] livepatch: add shadow variable API | From | Joe Lawrence <> | Date | Thu, 1 Jun 2017 16:23:44 -0400 |
| |
On 06/01/2017 04:05 PM, Jiri Kosina wrote: > On Thu, 1 Jun 2017, Joe Lawrence wrote: > >> This patchset is a simplified livepatch port of kpatch's "shadow" >> variable API [1]. The kpatch project has successfully employed such >> shadow variables to implement patches that have extended data structure >> elements. This API provides livepatch a means of associating new, >> shadow data fields with existing data structures. >> >> See the first patch for the implementation, the second for further >> documentation (API, conccurency notes, use-case code snippets) and the >> third patch for an update to the sample livepatch module using shadow >> variables. > > Thanks a lot for initiating this. > > The only issue I've spotted so far -- is there any reason, why the API > completely ignores task_struct->patch_state, and always returns the 'new' > value? > > This basically offloads the responsibility for deciding between old/new to > each and every caller, and that feels much more error prone compared to > having this automatically done by klp_shadow_get(). >
Hi Jiri,
I'm a little confused about the question. Maybe this clarifies a few things:
* klp_shadow_get() is only returning a pointer to the shadow data, the additional storage that klp_shadow_attach() has associated with the original data structure. Callers will have to handle this shadow structure accordingly, ie, not through old_struct->new_value, but rather *new_value).
* the intention is that only livepatched code will be calling klp_shadow_*, so it can assume that the current task is patched
* callers might need to verify klp_shadow_get() is returning non-NULL if it's possible that some data-structures don't have a shadow var attached
If you are referring to stacking livepatches ... to be honest I hadn't thought of that scenario. In that case, we might be able to get away with pushing something like this into the hash:
klp #1: klp_shadow_attach(ptr, "shadow_var", ...) klp #2: klp_shadow_attach(ptr, "shadow_var_v2", ...)
... but that's just off the top of my head :) I was hoping to handle the easy case first.
Maybe I misunderstood the question... if so, I can update the documentation file to better describe what's going on.
Regards,
-- Joe
| |