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 <[email protected]> > > Signed-off-by: Josh Poimboeuf <[email protected]> > > --- > > 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
