On Fri, Mar 01, 2019 at 11:13:13AM -0300, Joao Moreira wrote:
> Add a section to Documentation/livepatch/module-elf-format.txt
> describing how klp-convert works for fixing relocations.
>
> Signed-off-by: Joao Moreira <[email protected]>
> ---
> Documentation/livepatch/module-elf-format.txt | 47 ++++++++++++++++++++++++---
> 1 file changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/livepatch/module-elf-format.txt b/Documentation/livepatch/module-elf-format.txt
> index f21a5289a09c..6b0259dfab49 100644
> --- a/Documentation/livepatch/module-elf-format.txt
> +++ b/Documentation/livepatch/module-elf-format.txt
> @@ -2,7 +2,8 @@
> Livepatch module Elf format
> ===========================
>
> [ ... snip ... ]
>
> +--------------------------------------------------
> +4. Automatic conversion of unresolved relocations
> +--------------------------------------------------
> +Sometimes livepatches may operate on symbols which are not self-contained nor
> +exported. When this happens, these symbols remain unresolved in the elf object
> +and will trigger an error during the livepatch instantiation.
> +
> +Whenever possible, the kernel building infrastructure solves this problem
> +automatically. First, a symbol database containing information on all compiled
> +objects is built. Second, this database - a file named Symbols.list, placed in
> +the kernel source root directory - is used to identify targets for unresolved
> +relocations, converting them in the livepatch elf accordingly to the
> +specifications above-described. While the first staged is fully handled by the
^^^^^^
nit: s/staged/stage
> +building system, the second is done by a tool called klp-convert, which can be
> +found in "scripts/livepatch".
> +
> +When an unresolved relocation has as target a symbol whose name is also used by
> +different symbols throughout the kernel, the relocation cannot be resolved
> +automatically. In these cases, the livepatch developer must add annotations to
> +the livepatch, making it possible for the system to identify which is the
> +correct target amongst multiple homonymous symbols. Such annotations must be
> +done through a data structure as follows:
> +
> +struct KLP_MODULE_RELOC(object) data_structure_name[] = {
> + KLP_SYMPOS(symbol, pos)
> +};
> +
> +In the above example, object refers to the object file which contains the
> +symbol, being vmlinux or a module; symbol refers to the symbol name that will
> +be relocated and pos is its position in the object.
> [ ... snip ... ]
Should we be explicit about how position is counted? First = 1, second
= 2, etc? See the off-by-one bug I pointed out in the "livepatch: Add
klp-convert tool" patch earlier.
-- Joe
> > +building system, the second is done by a tool called klp-convert, which can be
> > +found in "scripts/livepatch".
> > +
> > +When an unresolved relocation has as target a symbol whose name is also used by
> > +different symbols throughout the kernel, the relocation cannot be resolved
> > +automatically. In these cases, the livepatch developer must add annotations to
> > +the livepatch, making it possible for the system to identify which is the
> > +correct target amongst multiple homonymous symbols. Such annotations must be
> > +done through a data structure as follows:
> > +
> > +struct KLP_MODULE_RELOC(object) data_structure_name[] = {
> > + KLP_SYMPOS(symbol, pos)
> > +};
> > +
> > +In the above example, object refers to the object file which contains the
> > +symbol, being vmlinux or a module; symbol refers to the symbol name that will
> > +be relocated and pos is its position in the object.
> > [ ... snip ... ]
>
> Should we be explicit about how position is counted? First = 1, second
> = 2, etc? See the off-by-one bug I pointed out in the "livepatch: Add
> klp-convert tool" patch earlier.
We could, but I would add it to a general section somewhere and just add a
reference here.
Documentation/livepatch/livepatch.txt says
"As an optional parameter, the symbol position in the kallsyms database
can be used to disambiguate functions of the same name. This is not the
absolute position in the database, but rather the order it has been found
only for a particular object ( vmlinux or a kernel module )."
We can improve it.
Documentation/livepatch/module-elf-format.txt says
"[D] The position of the symbol in the object (as according to kallsyms)
This is used to differentiate duplicate symbols within the same
object. The symbol position is expressed numerically (0, 1, 2...).
The symbol position of a unique symbol is 0."
It may even confuse someone.
So yes, I'd be for a change here and there.
Miroslav