netdev - Re: [PATCH net-next 6/8] tools: bpftool: print all relevant byte opcodes for "load double word"

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAH3iqT5vPnOCEm6wjavbpSyC4Md922Bts3ynxxVCT-1=MWsgKA@mail.gmail.com>
Date:   Fri, 20 Oct 2017 16:50:04 +0100
From:   Quentin Monnet <quentin.monnet@...ronome.com>
To:     David Laight <David.Laight@...lab.com>
Cc:     Jakub Kicinski <jakub.kicinski@...ronome.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "oss-drivers@...ronome.com" <oss-drivers@...ronome.com>
Subject: Re: [PATCH net-next 6/8] tools: bpftool: print all relevant byte
 opcodes for "load double word"

Hi David,

On 20 October 2017 at 10:59, David Laight <David.Laight@...lab.com> wrote:
> From: Jakub Kicinski
>> Sent: 19 October 2017 23:46
>> The eBPF instruction permitting to load double words (8 bytes) into a
>> register need 8-byte long "immediate" field, and thus occupy twice the
>> space of other instructions. bpftool was aware of this and would
>> increment the instruction counter only once on meeting such instruction,
>> but it would only print the first four bytes of the immediate value to
>> load. Make it able to dump the whole 16 byte-long double instruction
>> instead (as would `llvm-objdump -d <program>`).
>
> Guess why most modern instruction sets use a 'load high' instruction
> to generate big constants...
>
> Interestingly, is there anything special in the rest of the
> second instruction in order to make it an identifiable no-op?

The remaining four bytes are taken from the "immediate" field of the second
instruction, which leaves the first four fields (offset, source and destination
registers, and in particular opcode) unused. As far as I know, these fields
remain at zero, and this makes it the only “instruction” to have a null code
(although I am not sure this is a strict requirement, because I did not find
the code in the verifier that would reject a program having a non-null opcode
right after a "load double word immediate" instruction).

>

[…]

> Why not just:
>         for (i = 0; i < len / sizeof(*insn); i += 1 + double_insn) {
>
> ...
>
>         David
>

Yes, we could use that instead, although I am not sure this makes the code
more readable. So I do not believe this is worth a respin, tell me if you think
otherwise.

Thanks for the review!
Quentin