[OE-core] [PATCH] package: skip strip on signed kernel modules

[Victor Kamensky]

On Fri, 3 Aug 2018, Ocampo Coronado, Omar wrote:

> Yes, we would like to keep the symbols on a signed kernel module.
>
> Andre shared this link:  https://www.kernel.org/doc/html/v4.17/admin-guide/module-signing.html#signed-modules-and-stripping , from conversation topic: Re: [OE-core] Strip kernel modules and signatures

Thank you for the pointer. I did not expect that KLM signing
will be outside of ELF. Too bad that it is so brittle.

Ideally, it would be nice if one could disable KLM signing in
kernel makefile machinery and have mechanism to sign KLMs in OE
itself, just before packaging but after they got stripped.
IMO it would be more practical. I could not imagine if one
would want to ship KLMs with debug symbols inside. But even
if that is implemented, your code would still should stand ok -
if module signed already, it cannot be touched.

> -28 are the last 28 bytes of the file. The same amount of bytes are being read by dracut to check if a module is signed.
> And you are correct Victor, I'm unsure if this would work outside x86 arch.

I've checked that by building mips64 kernel with KLM signing
enabled and I looked at scripts/sign-file.c source, you are
fine: magic_number = "~Module signature appended~\n" will be
always at the end after KLM signing regardless of architecture.

Thanks,
Victor

> Two pending fixes:
>    1) This patch also needs to fix the mode of the file as the original may not be preserved.
>    2)  Seems like 'return' is not accepted by oe.utils.multiprocess, still getting familiar with OE
>
> -----Original Message-----
> From: Victor Kamensky [mailto:kamensky at cisco.com]
> Sent: Friday, August 3, 2018 3:28 PM
> To: Ocampo Coronado, Omar <omar.ocampo.coronado at intel.com>
> Cc: openembedded-core at lists.openembedded.org
> Subject: Re: [OE-core] [PATCH] package: skip strip on signed kernel modules
>
>
>
> On Fri, 3 Aug 2018, omar.ocampo.coronado at intel.com wrote:
>
>> From: foocampo <omar.ocampo.coronado at intel.com>
>>
>> Executing strip action on kernel modules removes the signature.
>> Is not possible to strip and keep the signature, therefore avoid strip
>> signed kernel modules.
>>
>> Signed-off-by: foocampo <omar.ocampo.coronado at intel.com>
>> ---
>> meta/lib/oe/package.py | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py index
>> fa3428ad61..f7d2d3b7c4 100644
>> --- a/meta/lib/oe/package.py
>> +++ b/meta/lib/oe/package.py
>> @@ -24,6 +24,9 @@ def runstrip(arg):
>>
>>     # kernel module
>>     if elftype & 16:
>> +        if is_kernel_module_signed(file):
>> +            bb.debug(1, "Skip strip on signed module %s" % file)
>> +            return
>
> It does not look right to me. Above means that signed KLM will go into image with symbols. Or I don't read code correctly?
>
> Where is signature stored? Is it some kind of an ELF NOTE? In this case you would just need to drop only "--remove-section=.note"
> from strip command. Wondering why .notes were stripped in the first place.
>
>>         stripcmd.extend(["--strip-debug", "--remove-section=.comment",
>>             "--remove-section=.note", "--preserve-dates"])
>
> I suggest split above into two invocations and do second
> stripcmd.extend(["--remove-section=.note"]) only for non signed modules.
> Assuming that signature is in the .note section. If it is not .comment, do that with "--remove-section=.comment" instead.
>
>>     # .so and shared library
>> @@ -46,6 +49,13 @@ def is_kernel_module(path):
>>     with open(path) as f:
>>         return mmap.mmap(f.fileno(), 0,
>> prot=mmap.PROT_READ).find(b"vermagic=") >= 0
>>
>> +# Detect if .ko module is signed
>> +def is_kernel_module_signed(path):
>> +    with open(path, "rb") as f:
>> +        f.seek(-28, 2)
>
> Where magic -28 comes from? Is it true for all cases, all CPU arches?
> I think it could be done more cleanly here.
>
> Thanks,
> Victor
>
>> +        module_tail = f.read()
>> +        return "Module signature appended" in "".join(chr(c) for c in
>> + bytearray(module_tail))
>> +
>> # Return type (bits):
>> # 0 - not elf
>> # 1 - ELF
>> --
>> 2.18.0
>>
>> --
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core at lists.openembedded.org
>> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>>
>