netdev - Re: Issue accessing task_struct from BPF due to 4.16 stack-protector changes

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: <20180302204201.tugprkgskfb7c7wh@ast-mbp.dhcp.thefacebook.com>
Date:   Fri, 2 Mar 2018 12:42:03 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Gianluca Borello <g.borello@...il.com>
Cc:     Linux Networking Development Mailing List 
        <netdev@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>, keescook@...omium.org
Subject: Re: Issue accessing task_struct from BPF due to 4.16 stack-protector
 changes

On Fri, Mar 02, 2018 at 12:09:57PM -0800, Gianluca Borello wrote:
> Hello,
> 
> While testing bpf-next, I noticed that I was reading garbage when
> accessing some task_struct members, and the issue seems caused by the
> recent commit 2bc2f688fdf8 ("Makefile: move stack-protector
> availability out of Kconfig") which removes CONFIG_CC_STACKPROTECTOR
> from autoconf.h.
> 
> When I compile my BPF program, offsetof(struct task_struct, files),
> which is the member I'm dereferencing, returns 1768 (where the garbage
> is), whereas doing it on 4.15 returns 1776 (where the correct member
> is). I believe when compiling with clang this portion of the
> task_struct doesn't get considered anymore:
> 
> #ifdef CONFIG_CC_STACKPROTECTOR
> /* Canary value for the -fstack-protector GCC feature: */
> unsigned long stack_canary;
> #endif
> 
> I solved it by adding $(KBUILD_CPPFLAGS) to my BPF Makefile (which is
> pretty similar to the one used in samples/bpf/Makefile).
> 
> Two questions:
> 
> 1) Do you confirm this is the proper way to handle this moving
> forward? Or should there be a better way?
> 
> 2) Would you consider useful a simple patch to samples/bpf/Makefile so
> that other developers will not be stuck in a long bisect to figure out
> why they read garbage when dereferencing task_struct? I assume that
> several people use that Makefile as a template to start their project,
> like I did (perhaps I'm assuming wrong though).

good catch!
I wonder why sched.h is using this flag insead of relying on #defines from autoconf.h
It could have been using CONFIG_HAVE_CC_STACKPROTECTOR
instead of CONFIG_CC_STACKPROTECTOR, no ?