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

[Masami Hiramatsu]

(2014/05/08 16:08), Ingo Molnar wrote:
> 
> * David Lang <david@xxxxxxx> 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.

What would you mean "safely apply" and "safely reject"?
Current kpatch checks the target functions are in use and reject safely
if it is in use.

> 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.

Hm, it is possible to check the binary patch itself doesn't refer
the global memory, but it's hard to find it derefers some address
of global memory. Also, there can be a case that the function caller
passes the address of a global memory, and the binary patch just
calls another function with that address. This can change the global
state (but correct way).

> 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 agreed if you mean the starting with a simple "tool" and
complicate it. I doubt kernel module can check the patch logic....

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@xxxxxxxxxxx


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/