Hi, Petr!
> The work with the elf sections is tricky. I would prefer to add
> .srcversion into struct klp_object, something like:
>
> struct klp_object {
> /* external */
> const char *name;
> + const char *srcversion;
> struct klp_func *funcs;
> struct klp_callbacks callbacks;
> [...]
> }
>
In fact, I have a though in mind that we can easily use the
srcversion in `struct module` like:
struct module {
enum module_state state;
/* Member of list of modules */
struct list_head list;
/* Unique handle for this module */
char name[MODULE_NAME_LEN];
#ifdef CONFIG_STACKTRACE_BUILD_ID
/* Module build ID */
unsigned char build_id[BUILD_ID_SIZE_MAX];
#endif
/* Sysfs stuff. */
struct module_kobject mkobj;
struct module_attribute *modinfo_attrs;
const char *version;
const char *srcversion;
struct kobject *holders_dir;
/* Exported symbols */
const struct kernel_symbol *syms;
const u32 *crcs;
unsigned int num_syms;
becase when we are loading a livepatch module, the syscall `init_module`
will be called and we can check the srcversion here.
What I want to do in the elf section is that I want to add a new section
to store the target module src version inside.
> It would be optional. I made just a quick look. It might be enough
> to check it in klp_find_object_module() and do not set obj->mod
> when obj->srcversion != NULL and does not match mod->srcversion.
>
> Wait! We need to check it also in klp_write_section_relocs().
> Otherwise, klp_apply_section_relocs() might apply the relocations
> for non-compatible module.
>
As previously mentioned, if we can check the srcversion when calling
the syscall `init_module`, refuse to load the module if the livepatch
module have srcversion and the srcversion is not equal to the target
in the system. Can it avoid such relocations problem?
> Another question is whether the same livepatch could support more
> srcversions of the same module. It might be safe when the livepatched
> functions are compatible. But it would be error prone.
>
> If we wanted to allow support for incompatible modules with the same
> name, we would need to encode the srcversion also into the name
> of the special .klp_rela sections so that we could have separate
> relocations for each variant.
>
I am not sure if supporting more srcversions is necessary. Because
I can't find a senario that more than one version of a module are running
in the system at the same time.
>
> Alternative: I think about using "mod->build_id" instead of "srcversion".
> It would be even more strict because it would make dependency
> on a particular build.
>
> An advantage is that it is defined also for vmlinux,
> see vmlinux_build_id. So that we could easily use
> the same approach also for vmlinux.
>
> I do not have strong opinion on this though.
>
Petr, using "mod->build_id" instead of "srcversion" may not be good.
Because livepatch can not only handle the function in vmlinux but also
the function in modules.
