On Tue, Nov 18, 2014 at 03:45:22PM +0100, Miroslav Benes wrote:
>
> On Sun, 16 Nov 2014, Seth Jennings wrote:
>
> [...]
>
> > diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> > new file mode 100644
> > index 0000000..8b68fef
> > --- /dev/null
> > +++ b/include/linux/livepatch.h
> > @@ -0,0 +1,68 @@
> > +/*
> > + * livepatch.h - Live Kernel Patching Core
> > + *
> > + * Copyright (C) 2014 Seth Jennings <sjenn...@redhat.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * as published by the Free Software Foundation; either version 2
> > + * of the License, or (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef _LINUX_LIVEPATCH_H_
> > +#define _LINUX_LIVEPATCH_H_
> > +
> > +#include <linux/module.h>
> > +
>
> I think we need something like
>
> #if IS_ENABLED(CONFIG_LIVE_PATCHING)
>
> here. Otherwise kernel module with live patch itself would be built
> even with live patching support disabled (as the structures and needed
> functions are declared).
What do you think of this (already includes s/lp/klp/ change)?
====
diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
index 0143b73..a9821f3 100644
--- a/include/linux/livepatch.h
+++ b/include/linux/livepatch.h
@@ -21,6 +21,7 @@
#define _LINUX_LIVEPATCH_H_
#include <linux/module.h>
+#include <asm/livepatch.h>
/* TODO: add kernel-doc for structures once agreed upon */
@@ -58,11 +59,20 @@ struct klp_patch {
struct klp_object *objs;
};
-int klp_register_patch(struct klp_patch *);
-int klp_unregister_patch(struct klp_patch *);
-int klp_enable_patch(struct klp_patch *);
-int klp_disable_patch(struct klp_patch *);
+#ifdef CONFIG_LIVE_PATCHING
-#include <asm/livepatch.h>
+extern int klp_register_patch(struct klp_patch *);
+extern int klp_unregister_patch(struct klp_patch *);
+extern int klp_enable_patch(struct klp_patch *);
+extern int klp_disable_patch(struct klp_patch *);
+
+#else /* !CONFIG_LIVE_PATCHING */
+
+static int klp_register_patch(struct klp_patch *k) { return -ENOSYS; }
+static int klp_unregister_patch(struct klp_patch *k) { return -ENOSYS; }
+static int klp_enable_patch(struct klp_patch *k) { return -ENOSYS; }
+static int klp_disable_patch(struct klp_patch *k) { return -ENOSYS; }
+
+#endif
====
This seems to be the way many headers handle this. Patch modules built
against a kernel that doesn't support live patching will build cleanly,
but will always fail to load.
Seth
>
> > +/* TODO: add kernel-doc for structures once agreed upon */
> > +
> > +struct lp_func {
> > + const char *old_name; /* function to be patched */
> > + void *new_func; /* replacement function in patch module */
> > + /*
> > + * The old_addr field is optional and can be used to resolve
> > + * duplicate symbol names in the vmlinux object. If this
> > + * information is not present, the symbol is located by name
> > + * with kallsyms. If the name is not unique and old_addr is
> > + * not provided, the patch application fails as there is no
> > + * way to resolve the ambiguity.
> > + */
> > + unsigned long old_addr;
> > +};
> > +
> > +struct lp_reloc {
> > + unsigned long dest;
> > + unsigned long src;
> > + unsigned long type;
> > + const char *name;
> > + int addend;
> > + int external;
> > +};
> > +
> > +struct lp_object {
> > + const char *name; /* "vmlinux" or module name */
> > + struct lp_func *funcs;
> > + struct lp_reloc *relocs;
> > +};
> > +
> > +struct lp_patch {
> > + struct module *mod; /* module containing the patch */
> > + struct lp_object *objs;
> > +};
> > +
> > +int lp_register_patch(struct lp_patch *);
> > +int lp_unregister_patch(struct lp_patch *);
> > +int lp_enable_patch(struct lp_patch *);
> > +int lp_disable_patch(struct lp_patch *);
> > +
> > +#include <asm/livepatch.h>
>
> and #endif for CONFIG_LIVE_PATCHING here.
>
> > +
> > +#endif /* _LINUX_LIVEPATCH_H_ */
>
> Thanks,
> --
> Miroslav Benes
> SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
