lkml.org 
[lkml]   [2016]   [Jan]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RFC PATCH v3 2/6] module: preserve Elf information for livepatch modules
Date
Hi Jessica,

Nice patch series. Minor issues below, but they're really
just nits which can be patched afterwards if you're getting sick of
rebasing :)

> +#ifdef CONFIG_LIVEPATCH
> +/*
> + * copy_module_elf - preserve Elf information about a module
> + *
> + * Copy relevant Elf information from the load_info struct.
> + * Note: not all fields from the original load_info are
> + * copied into mod->info.

This makes me nervous, to have a struct which is half-initialized.

Better would be to have a separate type for this, eg:

struct livepatch_modinfo {
Elf_Ehdr hdr;
Elf_Shdr *sechdrs;
char *secstrings;
/* FIXME: Which of these do you need? */
struct {
unsigned int sym, str, mod, vers, info, pcpu;
} index;
};

This also avoids an extra allocation as hdr is no longer a ptr.

> + /*
> + * Update symtab's sh_addr to point to a valid
> + * symbol table, as the temporary symtab in module
> + * init memory will be freed
> + */
> + mod->info->sechdrs[mod->info->index.sym].sh_addr = (unsigned long)mod->core_symtab;

This comment is a bit misleading: it's actually pointing into the
temporary module copy, which will be discarded. The init section is
slightly different...

> +#ifdef CONFIG_LIVEPATCH
> +static int check_livepatch_modinfo(struct module *mod, struct load_info *info)
> +{
> + mod->klp = get_modinfo(info, "livepatch") ? true : false;
> +
> + return 0;
> +}
> +#else
> +static int check_livepatch_modinfo(struct module *mod, struct load_info *info)
> +{
> + if (get_modinfo(info, "livepatch"))
> + return -EINVAL;
> +
> + return 0;
> +}
> +#endif

The term "check" implies no side-effects; I'd usually call this
int find_livepatch_modinfo(), and make sure you use the error code in
the caller:

> + err = check_livepatch_modinfo(mod, info);
> + if (err)
> + return -ENOEXEC;
> +

Thanks,
Rusty.

\
 
 \ /
  Last update: 2016-01-11 02:41    [W:0.116 / U:0.148 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site