linux-kernel - Re: [PATCH] lkdtm: support llvm-objcopy

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, 13 May 2019 16:50:05 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Nathan Chancellor <natechancellor@...il.com>
Cc:     Kees Cook <keescook@...omium.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        Nathan Chancellor <nathanchance@...il.com>,
        Jordan Rupprect <rupprecht@...gle.com>,
        Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] lkdtm: support llvm-objcopy

On Mon, May 13, 2019 at 4:29 PM Nathan Chancellor
<natechancellor@...il.com> wrote:
>
> On Mon, May 13, 2019 at 03:21:09PM -0700, 'Nick Desaulniers' via Clang Built Linux wrote:
> > With CONFIG_LKDTM=y and make OBJCOPY=llvm-objcopy, llvm-objcopy errors:
> > llvm-objcopy: error: --set-section-flags=.text conflicts with
> > --rename-section=.text=.rodata
> >
> > Rather than support setting flags then renaming sections vs renaming
> > then setting flags, it's simpler to just change both at the same time
> > via --rename-section.
> >
> > This can be verified with:
> > $ readelf -S drivers/misc/lkdtm/rodata_objcopy.o
> > ...
> > Section Headers:
> >   [Nr] Name              Type             Address           Offset
> >        Size              EntSize          Flags  Link  Info  Align
> > ...
> >   [ 1] .rodata           PROGBITS         0000000000000000  00000040
> >        0000000000000004  0000000000000000   A       0     0     4
> > ...
> >
> > Which shows in the Flags field that .text is now renamed .rodata, the
> > append flag A is set, and the section is not flagged as writeable W.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/448
> > Reported-by: Nathan Chancellor <nathanchance@...il.com>
>
> This should be natechancellor@...il.com (although I think I do own that
> email, just haven't been into it for 10+ years...)

Sorry, I should have looked it up.  I'll just fix this, my earlier
mistake, and collect Kee's reviewed by tag in a v2 sent directly to
GKH.

>
> > Suggested-by: Jordan Rupprect <rupprecht@...gle.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@...gle.com>
> > ---
> >  drivers/misc/lkdtm/Makefile | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/misc/lkdtm/Makefile b/drivers/misc/lkdtm/Makefile
> > index 951c984de61a..89dee2a9d88c 100644
> > --- a/drivers/misc/lkdtm/Makefile
> > +++ b/drivers/misc/lkdtm/Makefile
> > @@ -15,8 +15,7 @@ KCOV_INSTRUMENT_rodata.o    := n
> >
> >  OBJCOPYFLAGS :=
> >  OBJCOPYFLAGS_rodata_objcopy.o        := \
> > -                     --set-section-flags .text=alloc,readonly \
> > -                     --rename-section .text=.rodata
> > +                     --rename-section .text=.rodata,alloc,readonly
> >  targets += rodata.o rodata_objcopy.o
> >  $(obj)/rodata_objcopy.o: $(obj)/rodata.o FORCE
> >       $(call if_changed,objcopy)
> > --
> > 2.21.0.1020.gf2820cf01a-goog
> >
>
> I ran this script to see if there was any change for GNU objcopy and it
> looks like .rodata's type gets changed, is this intentional? Otherwise,
> this works for llvm-objcopy like you show.
>
> -----------
>
> 1c1
> < There are 11 section headers, starting at offset 0x240:
> ---
> > There are 11 section headers, starting at offset 0x230:
> 8c8
> <   [ 1] .rodata           PROGBITS         0000000000000000  00000040
> ---
> >   [ 1] .rodata           NOBITS           0000000000000000  00000040
> 10c10

Interesting find.  the .rodata of vmlinux itself is marked PROGBITS,
so its curious that GNU binutils changes the "Type" after the rename.
I doubt the code in question relies on NOBITS for this section.  Kees,
can you clarify?  Jordan, do you know what the differences are between
PROGBITS vs NOBITS?
https://people.redhat.com/mpolacek/src/devconf2012.pdf seems to
suggest NOBITS zero initializes data but I'm not sure that's what this
code wants.

>
> -----------
>
> #!/bin/bash
>
> TMP1=$(mktemp)
> TMP2=$(mktemp)
>
> git checkout next-20190513
>
> make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=out mrproper allyesconfig drivers/misc/lkdtm/
> readelf -S out/drivers/misc/lkdtm/rodata_objcopy.o > ${TMP1}
>
> curl -LSs https://lore.kernel.org/lkml/20190513222109.110020-1-ndesaulniers@google.com/raw | git am -3
>
> make -j$(nproc) ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=out mrproper allyesconfig drivers/misc/lkdtm/
> readelf -S out/drivers/misc/lkdtm/rodata_objcopy.o > ${TMP2}
>
> diff ${TMP1} ${TMP2}



-- 
Thanks,
~Nick Desaulniers