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@xxxxxxxxxx>
> > Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> > ---
> > 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
--
To unsubscribe from this list: send the line "unsubscribe live-patching" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
