lkml.org 
[lkml]   [2016]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] livepatch/module: make TAINT_LIVEPATCH module-specific
    On Thu, Aug 25, 2016 at 09:46:06AM +0200, Petr Mladek wrote:
    > Hi,
    >
    > I have spent some time to understand the change. I hope that the
    > comments below would help others.
    >
    > On Wed 2016-08-24 16:33:00, Josh Poimboeuf wrote:
    > > There's no reliable way to determine which module tainted the kernel
    > > with CONFIG_LIVEPATCH. For example, /sys/module/<klp module>/taint
    > > doesn't report it. Neither does the "mod -t" command in the crash tool.
    > >
    > > Make it crystal clear who the guilty party is by converting
    > > CONFIG_LIVEPATCH to a module taint flag.
    >
    > The above paragraph is a bit confusing. The patch adds TAINT_LIVEPATCH into the
    > list of module taint flags.
    >
    > > This changes the behavior a bit: now the the flag gets set when the
    > > module is loaded, rather than when it's enabled.
    > >
    > > Reviewed-by: Chunyu Hu <chuhu@redhat.com>
    > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
    > > ---
    > > kernel/livepatch/core.c | 3 ---
    > > kernel/module.c | 35 ++++++++++++-----------------------
    > > 2 files changed, 12 insertions(+), 26 deletions(-)
    > >
    > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
    > > index 5fbabe0..af46438 100644
    > > --- a/kernel/livepatch/core.c
    > > +++ b/kernel/livepatch/core.c
    > > @@ -545,9 +545,6 @@ static int __klp_enable_patch(struct klp_patch *patch)
    > > list_prev_entry(patch, list)->state == KLP_DISABLED)
    > > return -EBUSY;
    > >
    > > - pr_notice_once("tainting kernel with TAINT_LIVEPATCH\n");
    > > - add_taint(TAINT_LIVEPATCH, LOCKDEP_STILL_OK);
    >
    > The first important thing is that add_taint() is replaced with
    > add_taint_module(). The other function sets also mod->taints.
    >
    > It is a module taint flag, so it really makes sense to call it
    > when the module is loaded.
    >
    > > -
    > > pr_notice("enabling patch '%s'\n", patch->mod->name);
    > >
    > > klp_for_each_object(patch, obj) {
    > > diff --git a/kernel/module.c b/kernel/module.c
    > > index 529efae..fd5f95b 100644
    > > --- a/kernel/module.c
    > > +++ b/kernel/module.c
    > > @@ -1149,6 +1149,8 @@ static size_t module_flags_taint(struct module *mod, char *buf)
    > > buf[l++] = 'C';
    > > if (mod->taints & (1 << TAINT_UNSIGNED_MODULE))
    > > buf[l++] = 'E';
    > > + if (mod->taints & (1 << TAINT_LIVEPATCH))
    > > + buf[l++] = 'K';
    >
    > This is the second important part of the change. It shows the flag
    > in /sys/module/<klp module>/taint.
    >
    > The rest is just reshufling of the code. But it has problems
    > as already reported by kbuild test robot.
    >
    > The change looks good to me. We just need to fix the compilation
    > problem by adding some #ifdefs.

    This patch was a bit sloppy. I was doing too many things at once. :-)

    I'll fix it up and improve the description.

    --
    Josh

    \
     
     \ /
      Last update: 2016-09-17 09:57    [W:3.042 / U:0.124 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site