lkml.org 
[lkml]   [2015]   [Jul]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRevisiting patch dependencies
On Thu, Jul 23, 2015 at 12:02:06PM +0800, Minfei Huang wrote:
> On 07/22/15 at 09:40am, Josh Poimboeuf wrote:
> > Is it really safe to assume that there are no dependencies between
> > patches which patch different objects?
> >
>
> I think so.

What about the following scenario:

1. register and enable patch A, which patches vmlinux_func() and changes
its call signature
2. register and enable patch B, which patches a (not yet loaded) module
M so that it will call vmlinux_func() with its new call signature
3. load module M, which is immediately patched by patch B
4. disable patch A. Now the patched module M calls the unpatched
version of vmlinux_func() with the wrong call signature - BOOM

In this case B, a patch to a module, would have an implicit dependency
on A, a patch to vmlinux.

So I don't think the approach in the above patch would work. But I *do*
think we may need to revisit how we handle dependencies...

Note that our current patch stacking protects against unloading out of
order, but it assumes that the user loaded them in the correct order in
the first place. If M and B are loaded before A, then it would still go
boom even with today's code.

So IMO the way we handle dependencies today is incomplete. Some options
for improvement are:

a) Don't allow dependencies between patches. Instead all dependencies
must be contained within the patch itself. So patch A and patch B
are combined into a single patch AB. If, later, a new patch C is
needed, which also depends on A, then create a new cumulative patch
ABC which replaces AB.

Note there's no way to enforce the fact there are no dependencies,
because they can be hidden. So it would just have to be a documented
rule that the patch author must follow, as part of the (yet to be
written) patch creation guidelines. This actually isn't a big deal
because there are several other (still undocumented) rules the patch
author must already follow.

This would mean that klp code can assume there are no dependencies,
and so patch stacking would no longer be necessary. We'd probably
have to rework the ops->func_stack code a bit so that it's ordered by
when the patches were registered instead of when they were enabled,
so that disabling and re-enabling an older patch wouldn't override a
newer cumulative one which replaces it.

b) Create a way for the patch author to specify explicit patch
dependencies.

Note that both options a and b delegate responsibility to the patch
author to ensure that dependencies are handled appropriately.
Ultimately I don't think there's any way around that.

My vote would be option a for now, by removing patch stacking and
documenting the guidelines. With the eventual possibility of adding b
if needed.

--
Josh


\
 
 \ /
  Last update: 2015-07-23 20:21    [W:0.106 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site