[kpatch] Preparing a patch - pitfalls

[Josh Poimboeuf]

On Mon, Sep 28, 2015 at 04:59:17PM +0300, Evgenii Shatokhin wrote:
> 25.09.2015 21:51, Evgenii Shatokhin пишет:
> >Hi,
> >
> >In the presentation about using Kpatch in RHEL 7, Seth Jennings mentions
> >the following:
> >
> >  "The patches in the upstream kernel to fix CVEs are not designed to be
> >applied at runtime. There are certain pitfalls that need to be checked
> >for when generating a live patch."
> >
> >Could you elaborate? What are these pitfalls that should be checked for
> >when preparing a Kpatch-based patch? What kind of analysis should be done?
> 
> I suppose, one of these pitfalls related to the handling of global data,
> discussed in this ML some time ago. The changes in such data are not
> detected and require workarounds. Stumbled upon this when trying to make a
> Kpatch-based patch from https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=30927520dbae297182990bb21d08762bcc35ce1d.
> 
> Any other pitfalls to consider?

Hi Evgenii,

This is actually a very large and difficult topic.  The short answer is
that you need to fully understand all the implications of the patch and
how it would be applied in the live patching process.  And then you
often have to get creative.

The long answer is a book which unfortunately we haven't written yet.
We will eventually be writing a live patch creation guide to try to
collect and document best practices.  There are many types of cases to
consider and often several ways to solve them.

For the patch you pointed to, you'll need to remove the following hunk
because kpatch intentionally rejects data structure initialization
changes:

@@ -450,6 +465,7 @@ static struct tcp_congestion_ops cubictcp __read_mostly = {
        .cong_avoid     = bictcp_cong_avoid,
        .set_state      = bictcp_state,
        .undo_cwnd      = bictcp_undo_cwnd,
+       .cwnd_event     = bictcp_cwnd_event,
        .pkts_acked     = bictcp_acked,
        .owner          = THIS_MODULE,
        .name           = "cubic",

Instead of that initialization you'll need to write some code which
accomplishes the same task.

One option would be to make a new function which updates the
cubictcp.cwnd_event pointer at runtime.  You can tell kpatch to call
this new function during the patching process using the KPATCH_LOAD_HOOK
macro.  You'd also need to inspect the kernel code to make sure there
are no concurrency issues related to upating this pointer at runtime.

Another option would be to patch the function(s) which call using the
cwnd_event() function pointer.  They could be changed to call
bictcp_cwnd_event() directly when they detect the cubictcp struct.

-- 
Josh