Re: [RFC PATCH 0/2] kpatch: dynamic kernel patching

[Josh Poimboeuf <jpoimboe@redhat.com>]

On Thu, May 08, 2014 at 09:08:15AM +0200, Ingo Molnar wrote:
> 
> * David Lang <david@lang.hm> wrote:
> 
> > On Thu, 8 May 2014, Ingo Molnar wrote:
> > 
> > >>>
> > >>>No!
> > >>>
> > >>>A patch to the kernel source is 'safe' if it results in a correctly
> > >>>patched kernel source. Full stop!
> > >>>
> > >>>Live patching does not enter into this question, ever. The correctness
> > >>>of a patch to the source does not depend on 'live patching'
> > >>>considerations in any way, shape or form.
> > >>>
> > >>>Any mechanism that tries to blur these lines is broken by design.
> > >>>
> > >>>My claim is that if a patch is correct/safe in the old fashioned way,
> > >>>then a fundamental principle is that a live patching subsystem must
> > >>>either safely apply, or safely reject the live patching attempt,
> > >>>independently from any user input.
> > >>>
> > >>>It's similar to how kprobes (or ftrace) will safely reject or perform
> > >>>a live patching of the kernel.
> > >>>
> > >>>So for example, there's this recent upstream kernel fix:
> > >>>
> > >>>3ca9e5d36afb agp: info leak in agpioc_info_wrap()
> > >>>
> > >>>which fixes an information leak. The 'patch' is Git commit
> > >>>3ca9e5d36afb (i.e. it patches a very specific incoming kernel source
> > >>>tree that results in a specific outgoing source tree), and we know
> > >>>it's safe and correct.
> > >>>
> > >>>Any live patching subsystem must make sure that if this patch is
> > >>>live-patched, that this attempt is either rejected safely or performed
> > >>>safely.
> > >>>
> > >>>"We think/hope it won't blow up in most cases and we automated some
> > >>>checks halfways" or "the user must know what he is doing" is really
> > >>>not something that I think is a good concept for something as fragile
> > >>>as live patching.
> > >>
> > >>In that case you will have to reject any kernel patch that changes
> > >>any memory structure, because it's impossible as a general rule to
> > >>say that changing memory structures is going to be safe (or even
> > >>possible) to change.
> > >>
> > >>that includes any access to memory that moves around a lock
> > >
> > >Initially restricting it to such patches would be a good beginning -
> > >most of the security fixes are just failed checks, i.e. they don't
> > >typically even change any external (not on stack) memory structure,
> > >right?
> > 
> > in terms of hit-patching kernels you are correct.
> > 
> > but that's a far cry from what it sounded like you were demanding 
> > (that it must handle any kernel patch)
> 
> No, I was not demanding that at all, my suggestion was:
> 
>    > My claim is that if a patch is correct/safe in the old fashioned 
>    > way, then a fundamental principle is that a live patching 
>    > subsystem must either safely apply, or safely reject the live 
>    > patching attempt, independently from any user input.
> 
> Note the 'if'. It could start simple and stupid, and only allow cases 
> where we know the patch must be trivially safe (because it does not do 
> much in terms of disturbing globally visible state). That needs some 
> tooling help, but apparently tooling help is in place already.
> 
> And then we can complicate it from there - but have a reasonably 
> robust starting point that we _know_ works (as long as the 
> implementation is correct).

I really wonder if detecting a "trivially safe" patch is even possible.

Where do you draw the line with the following patches?

- add a call to another function which modifies global data

- add an early return or a goto which changes the way the function
  modifies (or no longer modifies) global data

- touch a local stack variable which results in global data being
  modified later in the function

- return a different value which causes the function's caller to modify
  data

-- 
Josh