Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [RFC PATCH] bpf: Add helpers to read useful task_struct members
To:     Y Song <ys114321@gmail.com>,
        "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
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>
 <df92faa5-ca2b-a623-e074-e14b7345f8e5@fb.com>
 <1510042830.62prnydrlw.naveen@linux.ibm.com>
 <CAH3MdRX-XehVX5A1O7+vxR+UuG46z7mRSMOer4hsxFYdKjWKUg@mail.gmail.com>
CC:     netdev <netdev@vger.kernel.org>,
        Sandipan Das <sandipan@linux.vnet.ibm.com>,
        Brendan Gregg <brendan.d.gregg@gmail.com>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Martin KaFai Lau <kafai@fb.com>,
        Kees Cook <keescook@chromium.org>,
        <linux-kernel@vger.kernel.org>
From:   Alexei Starovoitov <ast@fb.com>
Message-ID: <fc6e7617-ee4e-b799-7565-bdfc4f7e0159@fb.com>
Date:   Wed, 8 Nov 2017 06:39:28 +0900
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0)
 Gecko/20100101 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <CAH3MdRX-XehVX5A1O7+vxR+UuG46z7mRSMOer4hsxFYdKjWKUg@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Received-SPF: None (protection.outlook.com: fb.com does not designate
 permitted sender hosts)
SpamDiagnosticOutput: 1:99
SpamDiagnosticMetadata: NSPM
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Nov 2017 21:39:47.8517 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: c749c825-67fb-42e1-e5ac-08d526281393
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM3PR15MB0969
X-OriginatorOrg: fb.com
X-Proofpoint-Spam-Reason: safe
X-FB-Internal: Safe
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-11-07_07:,,
 signatures=0
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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.

bcc rewriter issue is odd. I suspect it was broken from day one.
Meaning that bcc didn't support poking into anonymous union and structs.

> I verified that the drawf info with randomized structure config does not
> match randomized structure member offset. Specifically, I tried
> linux/proc_ns.h struct proc_ns_operations,
> dwarf says:
>   field name: offset 0
>   field real_ns_name: offset 8
> But if you print out the real offset at runtime, you get 40 and 16 respectively.

thanks for confirming. It means that gcc randomization plugin is broken
and has to be fixed with regard to adjusting debug info while
randomizing the fields.


From 1583464602098398281@xxx Wed Nov 08 02:52:01 +0000 2017
X-GM-THRID: 1583027203607239623
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread