From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754456AbdKHAaT (ORCPT ); Tue, 7 Nov 2017 19:30:19 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:44596 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750985AbdKHAaS (ORCPT ); Tue, 7 Nov 2017 19:30:18 -0500 Reply-To: atish.patra@oracle.com Subject: Re: [RFC PATCH] bpf: Add helpers to read useful task_struct members References: <20171103065833.8076-1-sandipan@linux.vnet.ibm.com> <94a4761f-1b51-8b70-fb7f-3cea91c69717@fb.com> <1509815348.8zu63uatdo.naveen@linux.ibm.com> <4acdc081-341d-ee91-a591-b1d331a8c8d5@fb.com> <1509982000.092la4257a.naveen@linux.ibm.com> <1510042830.62prnydrlw.naveen@linux.ibm.com> <328c2dff-7d6d-0dc0-46cf-ae715845903e@fb.com> To: Y Song , Alexei Starovoitov Cc: "Naveen N. Rao" , netdev , Sandipan Das , Brendan Gregg , Daniel Borkmann , Martin KaFai Lau , Kees Cook , linux-kernel@vger.kernel.org Newsgroups: gmane.linux.kernel,gmane.linux.network From: Atish Patra Organization: Oracle Message-ID: <0b17e570-387b-2a4e-48c4-be87e6b2c4a4@oracle.com> Date: Tue, 7 Nov 2017 18:29:49 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/07/2017 04:42 PM, Y Song wrote: > On Tue, Nov 7, 2017 at 2:04 PM, Alexei Starovoitov wrote: >> On 11/8/17 6:47 AM, Y Song wrote: >>> >>> On Tue, Nov 7, 2017 at 1:39 PM, Alexei Starovoitov wrote: >>>> >>>> On 11/8/17 6:14 AM, Y Song wrote: >>>>> >>>>> >>>>> On Tue, Nov 7, 2017 at 12:37 AM, Naveen N. Rao >>>>> wrote: >>>>>> >>>>>> >>>>>> Alexei Starovoitov wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On 11/7/17 12:55 AM, Naveen N. Rao wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> I thought such struct shouldn't change layout. >>>>>>>>> If it is we need to fix include/linux/compiler-clang.h to do that >>>>>>>>> anon struct as well. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> We considered that, but it looked to be very dependent on the version >>>>>>>> of >>>>>>>> gcc used to build the kernel. But, this may be a simpler approach for >>>>>>>> the shorter term. >>>>>>>> >>>>>>> >>>>>>> why it would depend on version of gcc? >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> From what I can see, randomized_struct_fields_start is defined only for >>>>>> gcc >>>>>>> >>>>>>> >>>>>>> = 4.6. For older versions, it does not get mapped to an anonymous >>>>>> >>>>>> >>>>>> structure. We may not care for older gcc versions, but.. >>>>>> >>>>>> The other issue was that __randomize_layout maps to __designated_init >>>>>> when >>>>>> randstruct plugin is not enabled, which is in turn an attribute on gcc >>>>>>> = >>>>>> v5.1, but not otherwise. >>>>>> >>>>>>> We just need this, no? >>>>>>> >>>>>>> diff --git a/include/linux/compiler-clang.h >>>>>>> b/include/linux/compiler-clang.h >>>>>>> index de179993e039..4e29ab6187cb 100644 >>>>>>> --- a/include/linux/compiler-clang.h >>>>>>> +++ b/include/linux/compiler-clang.h >>>>>>> @@ -15,3 +15,6 @@ >>>>>>> * with any version that can compile the kernel >>>>>>> */ >>>>>>> #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), >>>>>>> __COUNTER__) >>>>>>> + >>>>>>> +#define randomized_struct_fields_start struct { >>>>>>> +#define randomized_struct_fields_end }; >>>>>>> >>>>>>> since offsets are mandated by C standard. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Yes, this is what we're testing with and is probably sufficient for our >>>>>> purposes. >>>>> >>>>> >>>>> >>>>> Just tested this with bcc. bcc actually complains. the rewriter >>>>> is not able to rewrite prev->pid where prev is "struct task_struct >>>>> *prev". >>>>> I will change bcc rewriter to see whether the field value is correct or >>>>> not. >>>>> >>>>> Not sure my understanding is correct or not, but I am afraid that >>>>> the above approach for clang compiler change may not work. >>>>> If clang calculates the field offset based on header file, the offset >>>>> may not be the same as kernel one.... >>>> >>>> >>>> >>>> why is that? >>>> When randomization is off both gcc and clang must generate the same >>>> offsets, since it's C standard. >>> >>> >>> The patch changed compiler-clang.h, so gcc still do randomization. >> >> >> gcc_plugins are off by default and randomization will not be >> turned on for any sane distro or datacenter that cares about >> performance and stability. >> So imo above compiler-clang.h patch together with bcc fix would >> be enough. > > Agree that short time the suggested fix should be enough. > Long time, disto could become "insane" someday :-) > Yup it works with clang compiler & bcc hack. Thanks :). I verified it with task_switch.py & runqlat.py. Are you going to push the bcc hacks to github repo ? Regards, Atish