Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <806bd4c6-7dff-f740-3625-849282310ce0@meta.com>
Date:   Tue, 17 Jan 2023 09:14:38 -0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.6.1
Subject: Re: [PATCH] tools: bpf: Disable stack protector
Content-Language: en-US
To:     "Jose E. Marchesi" <jose.marchesi@oracle.com>
Cc:     Peter Foley <pefoley2@pefoley.com>,
        Eduard Zingerman <eddyz87@gmail.com>,
        Quentin Monnet <quentin@isovalent.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>,
        Stanislav Fomichev <sdf@google.com>,
        Hao Luo <haoluo@google.com>, Jiri Olsa <jolsa@kernel.org>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Tom Rix <trix@redhat.com>, bpf@vger.kernel.org,
        linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
        david.faust@oracle.com, elena.zannoni@oracle.com
References: <20230114-bpf-v1-1-f836695a8b62@pefoley.com>
 <cb03b745-26b8-706c-de40-80ae991e29fd@isovalent.com>
 <194f38f2dc7d521375e5a660baaf1be31536be9a.camel@gmail.com>
 <CAOFdcFOgAH1z7EKyM=Q4EvzLuKETOWWDMwuqp36SxV-X6PGP5Q@mail.gmail.com>
 <1135125e-6b8a-7b75-5f0b-3208f6b6e8ae@meta.com> <87bkmx9tw8.fsf@oracle.com>
 <87sfg96s30.fsf@oracle.com>
From:   Yonghong Song <yhs@meta.com>
In-Reply-To: <87sfg96s30.fsf@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dEk0ZVBzNUNFSGNoNjh0YlYzRjhldVJmL29seWtmRXlCV1NVbTNtajYxMXEr?=
 =?utf-8?B?NzFNc2xVbE5VQ2pjQ016aFVNQTkyWllFQlNZbEVHSURFUnMrWTlsNFRBK0Rn?=
 =?utf-8?B?a2czOFFHTW1FOGRWVFBNRVFJZm82ZDVmTGwvSzhGWTZ6aFdMWkJ4T0MrVzVU?=
 =?utf-8?B?QVY3aFVxNnJCY0RWbjl1d2Y5M3BiMDNJdHBoMnpjeHV5Mm51TWwzc1cxYTha?=
 =?utf-8?B?MXRnNUJNa3FLVFB2NGRnczZROGJ0YVgwdjhVaHFnRWNsU2paQWppZFZHQTJI?=
 =?utf-8?B?MWY5STdianVJRDhVRnZzRUMwUTBOT0lxWmxnQjlCVW96MmNVelpDU3lkR0xx?=
 =?utf-8?B?WWhBeUJtYnB5T1RKYXJJanh1ZUlGL3RBM3NaZHZvOTNtaHhPS3dDQzlUMHVr?=
 =?utf-8?B?QVdHRC9VN1V5eStZSG1FcjhzS2lyNTBhSnZwVTNObWZabTJycUR2bnhZSzkz?=
 =?utf-8?B?VzdhRFNoRklLaUlXZk1ZZzdWWGhaSHhvZUZHaG93Njk5c2tOQWtuWlNtR2U4?=
 =?utf-8?B?a2pWMnJwZnZYTk1kRjU0TlZKR2I3SFFySXJLcmR1NDhBa3RITHVIb0VkcUFI?=
 =?utf-8?B?UTJqYXdpUEl0U0RGV21NZFNBNFpzcjNmZW1HZkdROFV2czlQaFZmcEFhQUpl?=
 =?utf-8?B?bUZPOTcxWlRmc2hvWTNTTzBDbHhSLzFrTFdjUUFQdHVwdnk3Tk05YUNLUmhU?=
 =?utf-8?B?U2FEWXB0TGdNWXdmSzE3S0JHc0hka05TVnQxNjd2KzBCc2NkN2VvU1dIMXpP?=
 =?utf-8?B?cVMxb2IrL3FOM1FqdEh1UnZiRWFKeTU0WGcwSFJ5TkpuQWVMS0ZTMmd3WUZH?=
 =?utf-8?B?U0dMVUZKVzNJd2RxR25OaGV2ZlZUWjFhaTV5WnBPMndxUDE5Z0h0ZVBnU1BK?=
 =?utf-8?B?dkxvNG8raExZeFhOaTJPMlI5enJ6Sk93d1VDdHpTa1FvSjdRMC9sUkdhSDFy?=
 =?utf-8?B?elZ1UUYvNW9tOTZpQXdINEdjN2tHT2tyR1M3VEJESTdpUlFlTkUrc1U3WUtP?=
 =?utf-8?B?R2lwTFU4WmpuMXZhNnJmczQyem4zSDJpSnVWeEtxdWZteDN4c1NYMWVkT2JZ?=
 =?utf-8?B?UTZxbU9lSG9IVnpDUXhYbDFDMjRKT1F2bkNkMDdzY3lWNEhqNXd0OEhjVkJK?=
 =?utf-8?B?WlpxNnlGclNjWFRaUW9EcGk3eEpwZUdvWkVBWC9vWDZYZVFNQkJlY21FdWdm?=
 =?utf-8?B?NGpaN0p3eFZZNTB2TEVUZWM1SkRTMHp5YThHWnN1RXF0VzlYdU01Rzk1TTZI?=
 =?utf-8?B?UkEzMEFzblBqTjJIakh3NjRhQXJ0dnZxRjB1Q2d2MEtPVnZkRUpEdjMwRE9j?=
 =?utf-8?B?elZqbHM0RTZPQkdIYkFKRXZNYlRHbnJjZlo0bVZsaVA2Z2hpWkQ3Nkd0eStn?=
 =?utf-8?B?eDNsZGN2ZE1nenlEWXNrL2M2MUpaZU95QlZDS1hISFRmR0VyaERJR3VnR3JN?=
 =?utf-8?B?OUN3VjRzZFFqemhBU1NkN1ljWWhtbkNzb0w1LzMwc0xQaEFZTlpBUFdna0lE?=
 =?utf-8?B?YldWeHpXQzdsQTBwakM4RGtPVndldHZBV08rNldEaFRuYlVoRkYxZmxQWWhC?=
 =?utf-8?B?emorSUx4elBsT2dBZkVZaUR1eFAyN2NHU1dXallEWWVZZVFncWZHbjk2RFdn?=
 =?utf-8?B?aXh4RFp6YjF5S3lKMUVBUVVwV0VxU0ZSU28yOC95YlRxaHBGMkowN2xBZnNS?=
 =?utf-8?B?WXJJZ2lUOXVubjE3cWZHU041b3dEeUNmK1lWVkNPMC9hd0ZCQlpTMFdXTXZC?=
 =?utf-8?B?S2UrYi9EbnFydDJKVGVwZUZOaDlGNHd3dGpqVnU0MkwwaHQ1U3RyZll5TzhU?=
 =?utf-8?B?Qk5vdGVPZ3pYUVpKZ0RJTUxRcEdDSkl6TjJEK3NVY0JucngraUM1Z0pUakph?=
 =?utf-8?B?NERtaXd4ajkvTGkvY09KdUNMNFlvbUpOMXZhSmpQcUpwOExKWW1GdnFYTGpt?=
 =?utf-8?B?b2RuTndrQldCT04xYlFNTFQxUFRiTGNla1NOck9UTHU3V3FqVzRyL0taV2xW?=
 =?utf-8?B?Sm8xRkNZMDBVN29PVFpVQXR0S1VvdTFRcWRMVkJEOXR2cDZrTjg4cEZSV2Vt?=
 =?utf-8?B?bEJORnV0TlpiT3NrUk9MNGNGVVdBdXVUZU05cUZBMTdxblltRHVRUkZjWlN5?=
 =?utf-8?B?bWQ0c3BvUzVrb3JETHZtdTJmc29VTWtTRVp6NysxOFUxckJTM2tyd01iSk5j?=
 =?utf-8?B?b0E9PQ==?=
X-OriginatorOrg: meta.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 95ada997-e991-4ac6-74ea-08daf8ae527c
X-MS-Exchange-CrossTenant-AuthSource: SN6PR1501MB2064.namprd15.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jan 2023 17:14:41.9638
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ae927fe-1255-47a7-a2af-5f3a069daaa2
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bxEPx0ZKmHpRrVgSCWlo56EDJd23J8pcW6QMYLZqqUkBcJgyBg+KbmZ/J6eQVsnS
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR15MB3831
X-Proofpoint-ORIG-GUID: su1w1tKbMcKW5dmQ4DDpplXSYOBM94s3
X-Proofpoint-GUID: su1w1tKbMcKW5dmQ4DDpplXSYOBM94s3
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.923,Hydra:6.0.562,FMLib:17.11.122.1
 definitions=2023-01-17_08,2023-01-17_01,2022-06-22_01
X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 1/17/23 8:31 AM, Jose E. Marchesi wrote:
> 
>>> On 1/16/23 2:49 PM, Peter Foley wrote:
>>>> On Mon, Jan 16, 2023 at 4:59 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>>>>>
>>>>> A bit tangential, but since BPF LLVM backend does not support the
>>>>> stack protector (should it?) there is also an option to adjust LLVM
>>>>> to avoid this instrumentation, WDYT?
>>>>>
>>>> That would probably be worth doing, yes.
>>>> But given that won't help already released versions of clang, it
>>>> should probably happen in addition to this patch.
>>>
>>> Peter,
>>>
>>> If I understand correctly (by inspecting clang code), the stack
>>> protector is off by default. Do you have link to Gentoo build
>>> page to show how they enable stack protector? cmake config or
>>> a private patch?
>>>
>>> Jose,
>>>
>>> How gcc-bpf handle stack protector? The compiler just disables
>>> stack protector for bpf target?
>>
>> It doesn't.  -fstack-protector is disabled by default in GCC.  When you
>> use it you get something like:
>>
>>    $ echo 'int foo() { char s[256]; return s[3]; }' | bpf-unknown-none-gcc \
>>      -fstack-protector -S -o foo.s -O2 -xc -
>>    $ cat foo.s
>>    	.file	"<stdin>"
>>    	.text
>>    	.align	3
>>    	.global	foo
>>    	.type	foo, @function
>>    foo:
>>    	lddw	%r1,__stack_chk_guard
>>    	ldxdw	%r0,[%r1+0]
>>    	stxdw	[%fp+-8],%r0
>>    	ldxb	%r0,[%fp+-261]
>>    	lsh	%r0,56
>>    	arsh	%r0,56
>>    	ldxdw	%r2,[%fp+-8]
>>    	ldxdw	%r3,[%r1+0]
>>   	jne	%r2,%r3,.L4
>>    	exit
>>    .L4:
>>    	call	__stack_chk_fail
>>    	.size	foo, .-foo
>>    	.ident	"GCC: (GNU) 12.0.0 20211206 (experimental)"
>>
>> i.e. it pushes a stack canary and checks it upon function exit, calling
>> __stack_chk_fail.
>>
>> If clang has -fstack-protector ON by default and you change the BPF
>> backend in order to ignore the flag, I think we should do the same in
>> GCC.
> 
> I went ahead and pushed the patch below to GCC master.  If
> -fstack-protector is ever considered useful in the architecture, we can
> always stop disabling it.
> 
> I would recommend to change the default for -fstack-protector in clang
> to be off by default when targetting BPF targets, and to emit the same
> or similar note to the user when the option is enabled explicitly with
> -fstack-protector:
> 
>    note: ‘-fstack-protector’ does not work  on this architecture
> 
> WDYT?
> 
>  From 3b81f5c4d8e0d79cbd6927d004185707c14e54b2 Mon Sep 17 00:00:00 2001
> Date: Tue, 17 Jan 2023 17:16:32 +0100
> Subject: [COMMITTED] bpf: disable -fstack-protector in BPF
> 
> The stack protector is not supported in BPF.  This patch disables
> -fstack-protector in bpf-* targets, along with the emission of a note
> indicating that the feature is not supported in this platform.
> 
> Regtested in bpf-unknown-none.
> 
> gcc/ChangeLog:
> 
> 	* config/bpf/bpf.cc (bpf_option_override): Disable
> 	-fstack-protector.
> ---
>   gcc/config/bpf/bpf.cc | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> index 576a1fe8eab..b268801d00c 100644
> --- a/gcc/config/bpf/bpf.cc
> +++ b/gcc/config/bpf/bpf.cc
> @@ -253,6 +253,14 @@ bpf_option_override (void)
>     if (bpf_has_jmp32 == -1)
>       bpf_has_jmp32 = (bpf_isa >= ISA_V3);
>   
> +  /* Disable -fstack-protector as it is not supported in BPF.  */
> +  if (flag_stack_protect)
> +    {
> +      inform (input_location,
> +              "%<-fstack-protector%> does not work "
> +              " on this architecture");
> +      flag_stack_protect = 0;
> +    }
>   }

Thanks, just replied based on a previous email
communication a while back. Yes, clang could
do similar things.

>   
>   #undef TARGET_OPTION_OVERRIDE