linux-kernel - Re: [PATCH v2 2/8] kbuild: Support for Symbols.list creation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <651eb328-b13a-9f31-d4da-9d2e965914e3@redhat.com>
Date:   Wed, 3 Apr 2019 15:10:19 -0400
From:   Joe Lawrence <joe.lawrence@...hat.com>
To:     Miroslav Benes <mbenes@...e.cz>
Cc:     Joao Moreira <jmoreira@...e.de>, live-patching@...r.kernel.org,
        pmladek@...e.cz, jikos@...e.cz, nstange@...e.de,
        jpoimboe@...hat.com, khlebnikov@...dex-team.ru, jeyu@...nel.org,
        matz@...e.de, linux-kernel@...r.kernel.org,
        yamada.masahiro@...ionext.com, linux-kbuild@...r.kernel.org,
        michal.lkml@...kovi.net
Subject: Re: [PATCH v2 2/8] kbuild: Support for Symbols.list creation

On 4/3/19 8:48 AM, Miroslav Benes wrote:
> 
>> and it looks like the combined KLP_MODULE_RELOC still contains the two
>> unique symbol position values (2 and 3):
>>
>>    % objcopy -O binary --only-section=.klp.module_relocs.vmlinux lib/livepatch/test_klp_convert.klp.o >(hexdump -C)
>>    00000000  00 00 00 00 00 00 00 00  02 00 00 00 00 00 00 00  |................|
>>    00000010  00 00 00 00 00 00 00 00  03 00 00 00              |............|
>>    0000001c
> 
> Nice :/
>   
>> Maybe we can work around this by modifying the annotation macros and/or
>> klp-convert, or live with this for now.
> 
> The question is (and I'll check later. I cannot wrap my head around it
> now) if it at least works if there are two references of the same symbol
> in two different .o. It would be same state_show in this case and not two
> different ones. If it works then I think we can live with it for a while,
> because after all duplicate symbols are quite rare in the kernel.

Possibly, but in testing that scenario I found another issue.  Check out what
happens to the combined .klp.module_relocs.vmlinux section for:

  test_klp_convert_a.c
  KLP_MODULE_RELOC(vmlinux) vmlinux_relocs_a[] = {
          KLP_SYMPOS(state_show, 2)
          KLP_SYMPOS(joe, 10)
          KLP_SYMPOS(joe2, 11)
  };

  test_klp_convert_b.c
  KLP_MODULE_RELOC(vmlinux) vmlinux_relocs_b[] = {
          KLP_SYMPOS(state_show, 2)
  };

The second file's klp_module_reloc are not aligned with the first,
so I think there is additional padding to push the second set to a
word boundary:

  % objcopy -O binary --only-section=.klp.module_relocs.vmlinux lib/livepatch/test_klp_convert.klp.o >(hexdump -C)
  00000000  00 00 00 00 00 00 00 00  02 00 00 00 00 00 00 00
            |-*sym----------------|  |--sympos-| |-*sym-----

  00000010  00 00 00 00 0a 00 00 00  00 00 00 00 00 00 00 00
            ----------| |--sympos-|  |-*sym----------------|

  00000020  0b 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
            |-sympos--|              |-*sym----------------|

                        ^^^^^^^^^^^
                        padding
  00000030  02 00 00 00
            |-sympos--|


in this case, klp-convert thought the last symbol's sympos was
incorrectly 0 and not 2.

If the packed attribute is merely a space optimization, can we
simply pull that (or can we specify slightly looser alignment to
account for the padding)?

I'll continue working on putting together v3 and add this new item
to the TODO list.

Thanks,

-- Joe