linux-kernel - Re: [PATCH 7/7] DWARF: add the config option

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]

Date:   Mon, 8 May 2017 20:38:18 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Jiri Slaby <jslaby@...e.cz>,
        Andrew Morton <akpm@...ux-foundation.org>,
        live-patching@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        the arch/x86 maintainers <x86@...nel.org>,
        Jiri Kosina <jikos@...nel.org>
Subject: Re: [PATCH 7/7] DWARF: add the config option

On Mon, May 08, 2017 at 05:21:24PM -0700, Andy Lutomirski wrote:
> On Sun, May 7, 2017 at 9:55 AM, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> >         struct undwarf {
> >                 unsigned int ip;        /* instruction pointer (relative offset from base) */
> >                 unsigned prev_frame:13; /* offset to previous frame from current stack pointer */
> >                 unsigned regs:1;        /* whether prev_frame contains entry regs (regs->ip) */
> >                 unsigned align:2;       /* some details for dealing with gcc stack realignment */
> >         } __packed;
> >
> >         extern struct undwarf undwarves[];
> 
> Some comments in case you're actually planning to do this:
> 
> 'unsigned int ip' is the majority of the size of this thing.  It might
> be worth trying to store a lot fewer bits.  You could split the
> structure into:
> 
> struct undwarf_header {
>   unsigned long start_ip;
>   unsigned align:2;  /* i'm assuming this rarely changes */
>   ...;
>   unsigned int offset_to_details;
> };
> 
> and
> 
> struct undwarf_details {
>   unsigned short ip_offset;
>   unsigned short prev_frame;
> };
> 
> and you'd find the details by first looking up the last header before
> the ip and then finding the details starting at (uintptr_t)header +
> header->offset_to_details.

Good idea.  According to some back-of-a-napkin math, a scheme like this
could reduce the data size from 1.8M down to 1.2M with my kernel config,
a not-too-shabby 600k savings.

> Also, don't you need some indication of which reg is the base from
> which you find previous frame?  After all, sometimes GCC will emit a
> frame pointer even in an otherwise frame-pointer-omitting kernel.

I don't think we *need* to do that.  I believe the base reg can just
always[*] be the stack pointer, even with frame pointers.

That said, it might be beneficial to use the frame pointer as the base
reg where applicable, because it would shrink the data size, especially
in a kernel with frame pointers enabled.  And I think we would only need
an extra bit to store that info.

First I'll just start with the simplest possible scheme (i.e., what I
proposed above).  Even that calculates out to be smaller than the DWARF
.eh_frame stuff.  Then after that we can look at compression techniques
(and their associated tradeoffs).

[*] ignoring rare gcc stack realignments

-- 
Josh