Messages in this thread | | | Subject | Re: [RFC PATCH] bpf: Add helpers to read useful task_struct members | From | Alexei Starovoitov <> | Date | Wed, 8 Nov 2017 07:04:02 +0900 |
| |
On 11/8/17 6:47 AM, Y Song wrote: > On Tue, Nov 7, 2017 at 1:39 PM, Alexei Starovoitov <ast@fb.com> wrote: >> On 11/8/17 6:14 AM, Y Song wrote: >>> >>> On Tue, Nov 7, 2017 at 12:37 AM, Naveen N. Rao >>> <naveen.n.rao@linux.vnet.ibm.com> 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.
| |