Re: [RFC PATCH] bpf: Add helpers to read useful task_struct members

On Tue, Nov 7, 2017 at 2:04 PM, Alexei Starovoitov [off-list ref] wrote:

On 11/8/17 6:47 AM, Y Song wrote:

quoted

On Tue, Nov 7, 2017 at 1:39 PM, Alexei Starovoitov [off-list ref] wrote:

quoted

On 11/8/17 6:14 AM, Y Song wrote:

quoted

On Tue, Nov 7, 2017 at 12:37 AM, Naveen N. Rao
[off-list ref] wrote:

quoted

Alexei Starovoitov wrote:

quoted

On 11/7/17 12:55 AM, Naveen N. Rao wrote:

quoted

quoted

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

quoted

= 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

quoted

=

v5.1, but not otherwise.

quoted

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 :-)