Adding CCs...
On Wed, 30 Jan 2019, Joao Moreira wrote:
> From: Miroslav Benes <[email protected]>
>
> Currently, livepatch infrastructure in the kernel relies on
> MODULE_INFO(livepatch, "Y") statement in a livepatch module. Then the
> kernel module loader knows a module is indeed livepatch module and can
> behave accordingly.
>
> klp-convert, on the other hand relies on LIVEPATCH_* statement in the
> module's Makefile for exactly the same reason.
>
> Remove dependency on modinfo and generate MODULE_INFO flag
> automatically in modpost when LIVEPATCH_* is defined in the module's
> Makefile. Generate a list of all built livepatch modules based on
> the .livepatch file and store it in (MODVERDIR)/livepatchmods. Give
> this list as an argument for modpost which will use it to identify
> livepatch modules.
>
> As MODULE_INFO is no longer needed, remove it.
>
> [jmoreira:
> * fix modpost.c (add_livepatch_flag) to update module structure with
> livepatch flag and prevent modpost from breaking due to unresolved
> symbols]
>
> Signed-off-by: Miroslav Benes <[email protected]>
> Signed-off-by: Joao Moreira <[email protected]>
> ---
> samples/livepatch/livepatch-sample.c | 1 -
> scripts/Makefile.modpost | 8 +++-
> scripts/mod/modpost.c | 84 +++++++++++++++++++++++++++++++++---
> 3 files changed, 85 insertions(+), 8 deletions(-)
>
> diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c
> index 2d554dd930e2..20a5891ebf7b 100644
> --- a/samples/livepatch/livepatch-sample.c
> +++ b/samples/livepatch/livepatch-sample.c
> @@ -90,4 +90,3 @@ static void livepatch_exit(void)
> module_init(livepatch_init);
> module_exit(livepatch_exit);
> MODULE_LICENSE("GPL");
> -MODULE_INFO(livepatch, "Y");
Again, we shoud do the same for the other samples we have in
samples/livepatch/ directory.
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index da779a185218..0149a72ea7ae 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -65,6 +65,11 @@ MODLISTCMD := find $(MODVERDIR) -name '*.mod' | xargs -r grep -h '\.ko$$' | sort
> __modules := $(shell $(MODLISTCMD))
> modules := $(patsubst %.o,%.ko, $(wildcard $(__modules:.ko=.o)))
>
> +# find all .livepatch files listed in $(MODVERDIR)/
> +ifdef CONFIG_LIVEPATCH
> +$(shell find $(MODVERDIR) -name '*.livepatch' | xargs -r -I{} basename {} .livepatch > $(MODVERDIR)/livepatchmods)
> +endif
> +
> # Stop after building .o files if NOFINAL is set. Makes compile tests quicker
> _modpost: $(if $(KBUILD_MODPOST_NOFINAL), $(modules:.ko:.o),$(modules))
>
> @@ -79,7 +84,8 @@ modpost = scripts/mod/modpost \
> $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
> $(if $(CONFIG_DEBUG_SECTION_MISMATCH),,-S) \
> $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
> - $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
> + $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w) \
> + $(if $(CONFIG_LIVEPATCH),-l $(MODVERDIR)/livepatchmods)
>
> MODPOST_OPT=$(subst -i,-n,$(filter -i,$(MAKEFLAGS)))
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 1dfc34d8b668..2fd1d409ca5a 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1979,10 +1979,6 @@ static void read_symbols(const char *modname)
> license = get_next_modinfo(&info, "license", license);
> }
>
> - /* Livepatch modules have unresolved symbols resolved by klp-convert */
> - if (get_modinfo(info.modinfo, info.modinfo_len, "livepatch"))
> - mod->livepatch = 1;
> -
> for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
> symname = remove_dot(info.strtab + sym->st_name);
>
> @@ -2421,6 +2417,76 @@ static void write_dump(const char *fname)
> free(buf.p);
> }
>
> +struct livepatch_mod_list {
> + struct livepatch_mod_list *next;
> + char *livepatch_mod;
> +};
> +
> +static struct livepatch_mod_list *load_livepatch_mods(const char *fname)
> +{
> + struct livepatch_mod_list *list_iter, *list_start = NULL;
> + unsigned long size, pos = 0;
> + void *file = grab_file(fname, &size);
> + char *line;
> +
> + if (!file)
> + return NULL;
> +
> + while ((line = get_next_line(&pos, file, size))) {
> + list_iter = NOFAIL(malloc(sizeof(*list_iter)));
> + list_iter->next = list_start;
> + list_iter->livepatch_mod = NOFAIL(strdup(line));
> + list_start = list_iter;
> + }
> + release_file(file, size);
> +
> + return list_start;
> +}
> +
> +static void free_livepatch_mods(struct livepatch_mod_list *list_start)
> +{
> + struct livepatch_mod_list *list_iter;
> +
> + while (list_start) {
> + list_iter = list_start->next;
> + free(list_start->livepatch_mod);
> + free(list_start);
> + list_start = list_iter;
> + }
> +}
> +
> +static int is_livepatch_mod(const char *modname,
> + struct livepatch_mod_list *list)
> +{
> + const char *myname;
> +
> + if (!list)
> + return 0;
> +
> + myname = strrchr(modname, '/');
> + if (myname)
> + myname++;
> + else
> + myname = modname;
> +
> + while (list) {
> + if (!strcmp(myname, list->livepatch_mod))
> + return 1;
> + list = list->next;
> + }
> + return 0;
> +}
> +
> +static void add_livepatch_flag(struct buffer *b, struct module *mod,
> + struct livepatch_mod_list *list)
> +{
> + if (is_livepatch_mod(mod->name, list)) {
> + buf_printf(b, "\nMODULE_INFO(livepatch, \"Y\");\n");
> + mod->livepatch = 1;
> + }
> +}
> +
> +
> struct ext_sym_list {
> struct ext_sym_list *next;
> const char *file;
> @@ -2436,8 +2502,9 @@ int main(int argc, char **argv)
> int err;
> struct ext_sym_list *extsym_iter;
> struct ext_sym_list *extsym_start = NULL;
> + struct livepatch_mod_list *livepatch_mods = NULL;
>
> - while ((opt = getopt(argc, argv, "i:I:e:mnsST:o:awE")) != -1) {
> + while ((opt = getopt(argc, argv, "i:I:e:l:mnsST:o:awM:K:E")) != -1) {
> switch (opt) {
> case 'i':
> kernel_read = optarg;
> @@ -2454,6 +2521,9 @@ int main(int argc, char **argv)
> extsym_iter->file = optarg;
> extsym_start = extsym_iter;
> break;
> + case 'l':
> + livepatch_mods = load_livepatch_mods(optarg);
> + break;
> case 'm':
> modversions = 1;
> break;
> @@ -2514,15 +2584,16 @@ int main(int argc, char **argv)
> buf.pos = 0;
>
> err |= check_modname_len(mod);
> - err |= check_exports(mod);
> add_header(&buf, mod);
> add_intree_flag(&buf, !external_module);
> add_retpoline(&buf);
> add_staging_flag(&buf, mod->name);
> + add_livepatch_flag(&buf, mod, livepatch_mods);
> err |= add_versions(&buf, mod);
> add_depends(&buf, mod);
> add_moddevtable(&buf, mod);
> add_srcversion(&buf, mod);
> + err |= check_exports(mod);
>
> sprintf(fname, "%s.mod.c", mod->name);
> write_if_changed(&buf, fname);
> @@ -2541,6 +2612,7 @@ int main(int argc, char **argv)
> "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n");
> }
> }
> + free_livepatch_mods(livepatch_mods);
> free(buf.p);
>
> return err;
> --
> 2.16.4
>
Miroslav
On Wed, Feb 20, 2019 at 04:50:08PM +0100, Miroslav Benes wrote:
> Adding CCs...
Where was this posted? I don't see it on lkml. It would be good to be
able to see the whole series for review.
>
> On Wed, 30 Jan 2019, Joao Moreira wrote:
>
> > From: Miroslav Benes <[email protected]>
> >
> > Currently, livepatch infrastructure in the kernel relies on
> > MODULE_INFO(livepatch, "Y") statement in a livepatch module. Then the
> > kernel module loader knows a module is indeed livepatch module and can
> > behave accordingly.
> >
> > klp-convert, on the other hand relies on LIVEPATCH_* statement in the
> > module's Makefile for exactly the same reason.
> >
> > Remove dependency on modinfo and generate MODULE_INFO flag
> > automatically in modpost when LIVEPATCH_* is defined in the module's
> > Makefile. Generate a list of all built livepatch modules based on
> > the .livepatch file and store it in (MODVERDIR)/livepatchmods. Give
> > this list as an argument for modpost which will use it to identify
> > livepatch modules.
> >
> > As MODULE_INFO is no longer needed, remove it.
> >
> > [jmoreira:
> > * fix modpost.c (add_livepatch_flag) to update module structure with
> > livepatch flag and prevent modpost from breaking due to unresolved
> > symbols]
> >
> > Signed-off-by: Miroslav Benes <[email protected]>
> > Signed-off-by: Joao Moreira <[email protected]>
> > ---
> > samples/livepatch/livepatch-sample.c | 1 -
> > scripts/Makefile.modpost | 8 +++-
> > scripts/mod/modpost.c | 84 +++++++++++++++++++++++++++++++++---
> > 3 files changed, 85 insertions(+), 8 deletions(-)
> >
> > diff --git a/samples/livepatch/livepatch-sample.c b/samples/livepatch/livepatch-sample.c
> > index 2d554dd930e2..20a5891ebf7b 100644
> > --- a/samples/livepatch/livepatch-sample.c
> > +++ b/samples/livepatch/livepatch-sample.c
> > @@ -90,4 +90,3 @@ static void livepatch_exit(void)
> > module_init(livepatch_init);
> > module_exit(livepatch_exit);
> > MODULE_LICENSE("GPL");
> > -MODULE_INFO(livepatch, "Y");
>
> Again, we shoud do the same for the other samples we have in
> samples/livepatch/ directory.
>
> > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> > index da779a185218..0149a72ea7ae 100644
> > --- a/scripts/Makefile.modpost
> > +++ b/scripts/Makefile.modpost
> > @@ -65,6 +65,11 @@ MODLISTCMD := find $(MODVERDIR) -name '*.mod' | xargs -r grep -h '\.ko$$' | sort
> > __modules := $(shell $(MODLISTCMD))
> > modules := $(patsubst %.o,%.ko, $(wildcard $(__modules:.ko=.o)))
> >
> > +# find all .livepatch files listed in $(MODVERDIR)/
> > +ifdef CONFIG_LIVEPATCH
> > +$(shell find $(MODVERDIR) -name '*.livepatch' | xargs -r -I{} basename {} .livepatch > $(MODVERDIR)/livepatchmods)
> > +endif
> > +
> > # Stop after building .o files if NOFINAL is set. Makes compile tests quicker
> > _modpost: $(if $(KBUILD_MODPOST_NOFINAL), $(modules:.ko:.o),$(modules))
> >
> > @@ -79,7 +84,8 @@ modpost = scripts/mod/modpost \
> > $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
> > $(if $(CONFIG_DEBUG_SECTION_MISMATCH),,-S) \
> > $(if $(CONFIG_SECTION_MISMATCH_WARN_ONLY),,-E) \
> > - $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
> > + $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w) \
> > + $(if $(CONFIG_LIVEPATCH),-l $(MODVERDIR)/livepatchmods)
> >
> > MODPOST_OPT=$(subst -i,-n,$(filter -i,$(MAKEFLAGS)))
> >
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 1dfc34d8b668..2fd1d409ca5a 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -1979,10 +1979,6 @@ static void read_symbols(const char *modname)
> > license = get_next_modinfo(&info, "license", license);
> > }
> >
> > - /* Livepatch modules have unresolved symbols resolved by klp-convert */
> > - if (get_modinfo(info.modinfo, info.modinfo_len, "livepatch"))
> > - mod->livepatch = 1;
> > -
> > for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
> > symname = remove_dot(info.strtab + sym->st_name);
> >
> > @@ -2421,6 +2417,76 @@ static void write_dump(const char *fname)
> > free(buf.p);
> > }
> >
> > +struct livepatch_mod_list {
> > + struct livepatch_mod_list *next;
> > + char *livepatch_mod;
> > +};
> > +
> > +static struct livepatch_mod_list *load_livepatch_mods(const char *fname)
> > +{
> > + struct livepatch_mod_list *list_iter, *list_start = NULL;
> > + unsigned long size, pos = 0;
> > + void *file = grab_file(fname, &size);
> > + char *line;
> > +
> > + if (!file)
> > + return NULL;
> > +
> > + while ((line = get_next_line(&pos, file, size))) {
> > + list_iter = NOFAIL(malloc(sizeof(*list_iter)));
> > + list_iter->next = list_start;
> > + list_iter->livepatch_mod = NOFAIL(strdup(line));
> > + list_start = list_iter;
> > + }
> > + release_file(file, size);
> > +
> > + return list_start;
> > +}
> > +
> > +static void free_livepatch_mods(struct livepatch_mod_list *list_start)
> > +{
> > + struct livepatch_mod_list *list_iter;
> > +
> > + while (list_start) {
> > + list_iter = list_start->next;
> > + free(list_start->livepatch_mod);
> > + free(list_start);
> > + list_start = list_iter;
> > + }
> > +}
> > +
> > +static int is_livepatch_mod(const char *modname,
> > + struct livepatch_mod_list *list)
> > +{
> > + const char *myname;
> > +
> > + if (!list)
> > + return 0;
> > +
> > + myname = strrchr(modname, '/');
> > + if (myname)
> > + myname++;
> > + else
> > + myname = modname;
> > +
> > + while (list) {
> > + if (!strcmp(myname, list->livepatch_mod))
> > + return 1;
> > + list = list->next;
> > + }
> > + return 0;
> > +}
> > +
> > +static void add_livepatch_flag(struct buffer *b, struct module *mod,
> > + struct livepatch_mod_list *list)
> > +{
> > + if (is_livepatch_mod(mod->name, list)) {
> > + buf_printf(b, "\nMODULE_INFO(livepatch, \"Y\");\n");
> > + mod->livepatch = 1;
> > + }
> > +}
> > +
> > +
> > struct ext_sym_list {
> > struct ext_sym_list *next;
> > const char *file;
> > @@ -2436,8 +2502,9 @@ int main(int argc, char **argv)
> > int err;
> > struct ext_sym_list *extsym_iter;
> > struct ext_sym_list *extsym_start = NULL;
> > + struct livepatch_mod_list *livepatch_mods = NULL;
> >
> > - while ((opt = getopt(argc, argv, "i:I:e:mnsST:o:awE")) != -1) {
> > + while ((opt = getopt(argc, argv, "i:I:e:l:mnsST:o:awM:K:E")) != -1) {
> > switch (opt) {
> > case 'i':
> > kernel_read = optarg;
> > @@ -2454,6 +2521,9 @@ int main(int argc, char **argv)
> > extsym_iter->file = optarg;
> > extsym_start = extsym_iter;
> > break;
> > + case 'l':
> > + livepatch_mods = load_livepatch_mods(optarg);
> > + break;
> > case 'm':
> > modversions = 1;
> > break;
> > @@ -2514,15 +2584,16 @@ int main(int argc, char **argv)
> > buf.pos = 0;
> >
> > err |= check_modname_len(mod);
> > - err |= check_exports(mod);
> > add_header(&buf, mod);
> > add_intree_flag(&buf, !external_module);
> > add_retpoline(&buf);
> > add_staging_flag(&buf, mod->name);
> > + add_livepatch_flag(&buf, mod, livepatch_mods);
> > err |= add_versions(&buf, mod);
> > add_depends(&buf, mod);
> > add_moddevtable(&buf, mod);
> > add_srcversion(&buf, mod);
> > + err |= check_exports(mod);
> >
> > sprintf(fname, "%s.mod.c", mod->name);
> > write_if_changed(&buf, fname);
> > @@ -2541,6 +2612,7 @@ int main(int argc, char **argv)
> > "Set CONFIG_SECTION_MISMATCH_WARN_ONLY=y to allow them.\n");
> > }
> > }
> > + free_livepatch_mods(livepatch_mods);
> > free(buf.p);
> >
> > return err;
> > --
> > 2.16.4
> >
>
> Miroslav
--
Josh