Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1664705 > unrolled thread

Re: [PATCH 0/3] livepatch: add shadow variable API

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 0/3] livepatch: add shadow variable API Miroslav Benes <mbenes@suse.cz> - 2017-06-13 12:20 +0200
    Re: [PATCH 0/3] livepatch: add shadow variable API Joe Lawrence <joe.lawrence@redhat.com> - 2017-06-13 19:30 +0200
      Re: [PATCH 0/3] livepatch: add shadow variable API Miroslav Benes <mbenes@suse.cz> - 2017-06-14 10:10 +0200

#1664705 — Re: [PATCH 0/3] livepatch: add shadow variable API

> 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", ...)

I thought this was the reason to have a string there. Otherwise, a 
pointer to original data would be enough, wouldn't it?

Miroslav

[toc] | [next] | [standalone]

#1665041

On 06/13/2017 06:19 AM, Miroslav Benes wrote:
>> 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", ...)
> 
> I thought this was the reason to have a string there. Otherwise, a 
> pointer to original data would be enough, wouldn't it?

Well, one could attach multiple shadow variables to the same data
structure, ie, one for each new data element.  In the stacking case, you
might add a spinlock in patch 1, then a linked-list in patch 2.  Patched
codepaths would then use klp_shadow_get(obj, "spinlock") or
klp_shadow_get(obj, "list") as needed.

Versioning shadow variables would be a bit more involved.  You'd have to
figure out if you A) convert existing shadow variables to the new format
on livepatch module load, or B) convert on the fly, or C) handle none,
v1, and v2 instances of the shadow variables.  /head spins

To be honest, I don't think we've never needed anything beyond basic
shadow variables in kpatch, so I'm only speculating about their
potential (ab)uses :)  That said, since this patchset is introducing the
API, it would be good to be reasonably flexible.

-- Joe

[toc] | [prev] | [next] | [standalone]

#1665546

On Tue, 13 Jun 2017, Joe Lawrence wrote:

> On 06/13/2017 06:19 AM, Miroslav Benes wrote:
> >> 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", ...)
> > 
> > I thought this was the reason to have a string there. Otherwise, a 
> > pointer to original data would be enough, wouldn't it?
> 
> Well, one could attach multiple shadow variables to the same data
> structure, ie, one for each new data element.  In the stacking case, you
> might add a spinlock in patch 1, then a linked-list in patch 2.  Patched
> codepaths would then use klp_shadow_get(obj, "spinlock") or
> klp_shadow_get(obj, "list") as needed.

Ok, I mixed two different things into one. Yes, this is a valid use case.

> Versioning shadow variables would be a bit more involved.  You'd have to
> figure out if you A) convert existing shadow variables to the new format
> on livepatch module load, or B) convert on the fly, or C) handle none,
> v1, and v2 instances of the shadow variables.  /head spins

I'm gonna pretend I didn't read this.

> To be honest, I don't think we've never needed anything beyond basic
> shadow variables in kpatch, so I'm only speculating about their
> potential (ab)uses :)  That said, since this patchset is introducing the
> API, it would be good to be reasonably flexible.

I'd worry about that later. If we ever come upon that.

Thanks,
Miroslav

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web