* [PATCH RESEND bpf 0/2] x86/bpf: Fixes for the BPF JIT with retbleed=stuff @ 2024-03-29 9:46 Uros Bizjak 2024-03-29 9:46 ` [PATCH RESEND bpf 1/2] x86/bpf: Fix IP after emitting call depth accounting Uros Bizjak 2024-03-29 9:46 ` [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating " Uros Bizjak 0 siblings, 2 replies; 12+ messages in thread From: Uros Bizjak @ 2024-03-29 9:46 UTC (permalink / raw) To: x86, bpf, netdev, linux-kernel Cc: Uros Bizjak, Alexei Starovoitov, Daniel Borkmann, Joan Bruguera Micó From: Joan Bruguera Micó <joanbrugueram@gmail.com> Fixes two issues that cause kernels panic when using the BPF JIT with the call depth tracking / stuffing mitigation for Skylake processors (`retbleed=stuff`). Both issues can be triggered by running simple BPF programs (e.g. running the test suite should trigger both). The first (resubmit) fixes a trivial issue related to calculating the destination IP for call instructions with call depth tracking. The second is related to using the correct IP for relocations, related to the recently introduced %rip-relative addressing for PER_CPU_VAR. Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: Joan Bruguera Micó <joanbrugueram@gmail.com> Joan Bruguera Micó (2): x86/bpf: Fix IP after emitting call depth accounting x86/bpf: Fix IP for relocating call depth accounting arch/x86/include/asm/alternative.h | 4 ++-- arch/x86/kernel/callthunks.c | 4 ++-- arch/x86/net/bpf_jit_comp.c | 22 ++++++++++------------ 3 files changed, 14 insertions(+), 16 deletions(-) -- 2.44.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RESEND bpf 1/2] x86/bpf: Fix IP after emitting call depth accounting 2024-03-29 9:46 [PATCH RESEND bpf 0/2] x86/bpf: Fixes for the BPF JIT with retbleed=stuff Uros Bizjak @ 2024-03-29 9:46 ` Uros Bizjak 2024-03-29 21:26 ` Alexei Starovoitov 2024-03-29 9:46 ` [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating " Uros Bizjak 1 sibling, 1 reply; 12+ messages in thread From: Uros Bizjak @ 2024-03-29 9:46 UTC (permalink / raw) To: x86, bpf, netdev, linux-kernel Cc: Joan Bruguera Micó, Alexei Starovoitov, Daniel Borkmann From: Joan Bruguera Micó <joanbrugueram@gmail.com> Adjust the IP passed to `emit_patch` so it calculates the correct offset for the CALL instruction if `x86_call_depth_emit_accounting` emits code. Otherwise we will skip some instructions and most likely crash. Fixes: b2e9dfe54be4 ("x86/bpf: Emit call depth accounting if required") Link: https://lore.kernel.org/lkml/20230105214922.250473-1-joanbrugueram@gmail.com/ Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> --- arch/x86/net/bpf_jit_comp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index a7ba8e178645..09f7dc9d4d65 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/arch/x86/net/bpf_jit_comp.c @@ -479,9 +479,10 @@ static int emit_call(u8 **pprog, void *func, void *ip) static int emit_rsb_call(u8 **pprog, void *func, void *ip) { + void *adjusted_ip; OPTIMIZER_HIDE_VAR(func); - x86_call_depth_emit_accounting(pprog, func); - return emit_patch(pprog, func, ip, 0xE8); + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); + return emit_patch(pprog, func, adjusted_ip, 0xE8); } static int emit_jump(u8 **pprog, void *func, void *ip) -- 2.44.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND bpf 1/2] x86/bpf: Fix IP after emitting call depth accounting 2024-03-29 9:46 ` [PATCH RESEND bpf 1/2] x86/bpf: Fix IP after emitting call depth accounting Uros Bizjak @ 2024-03-29 21:26 ` Alexei Starovoitov 2024-03-29 21:26 ` Alexei Starovoitov 0 siblings, 1 reply; 12+ messages in thread From: Alexei Starovoitov @ 2024-03-29 21:26 UTC (permalink / raw) To: Uros Bizjak Cc: X86 ML, bpf, Network Development, LKML, Joan Bruguera Micó, Alexei Starovoitov, Daniel Borkmann On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > Adjust the IP passed to `emit_patch` so it calculates the correct offset > for the CALL instruction if `x86_call_depth_emit_accounting` emits code. > Otherwise we will skip some instructions and most likely crash. > > Fixes: b2e9dfe54be4 ("x86/bpf: Emit call depth accounting if required") > Link: https://lore.kernel.org/lkml/20230105214922.250473-1-joanbrugueram@gmail.com/ > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > --- > arch/x86/net/bpf_jit_comp.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index a7ba8e178645..09f7dc9d4d65 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -479,9 +479,10 @@ static int emit_call(u8 **pprog, void *func, void *ip) > > static int emit_rsb_call(u8 **pprog, void *func, void *ip) > { > + void *adjusted_ip; > OPTIMIZER_HIDE_VAR(func); > - x86_call_depth_emit_accounting(pprog, func); > - return emit_patch(pprog, func, ip, 0xE8); > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); Why not just ip += x86_call_depth_emit_accounting(pprog, func); ? > + return emit_patch(pprog, func, adjusted_ip, 0xE8); > } > > static int emit_jump(u8 **pprog, void *func, void *ip) > -- > 2.44.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND bpf 1/2] x86/bpf: Fix IP after emitting call depth accounting 2024-03-29 21:26 ` Alexei Starovoitov @ 2024-03-29 21:26 ` Alexei Starovoitov 0 siblings, 0 replies; 12+ messages in thread From: Alexei Starovoitov @ 2024-03-29 21:26 UTC (permalink / raw) To: Uros Bizjak Cc: X86 ML, bpf, Network Development, LKML, Joan Bruguera Micó, Alexei Starovoitov, Daniel Borkmann [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 36387 bytes --] On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > Adjust the IP passed to `emit_patch` so it calculates the correct offset > for the CALL instruction if `x86_call_depth_emit_accounting` emits code. > Otherwise we will skip some instructions and most likely crash. > > Fixes: b2e9dfe54be4 ("x86/bpf: Emit call depth accounting if required") > Link: https://lore.kernel.org/lkml/20230105214922.250473-1-joanbrugueram@gmail.com/ > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > --- > arch/x86/net/bpf_jit_comp.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index a7ba8e178645..09f7dc9d4d65 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -479,9 +479,10 @@ static int emit_call(u8 **pprog, void *func, void *ip) > > static int emit_rsb_call(u8 **pprog, void *func, void *ip) > { > + void *adjusted_ip; > OPTIMIZER_HIDE_VAR(func); > - x86_call_depth_emit_accounting(pprog, func); > - return emit_patch(pprog, func, ip, 0xE8); > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); Why not just ip += x86_call_depth_emit_accounting(pprog, func); ? > + return emit_patch(pprog, func, adjusted_ip, 0xE8); > } > > static int emit_jump(u8 **pprog, void *func, void *ip) > -- > 2.44.0 > X-sender: <netdev+bounces-83464-steffen.klassert=cunet.com@vger.kernel.org> X-Receiver: <steffen.klassert@secunet.com> ORCPT=c822;steffen.klassert@secunet.com NOTIFY=VER; X-ExtendedProps=AVABYAAgAAAAUAFAARAPDFCS25BAlDktII2g02frgPADUAAABNaWNyb3NvZnQuRXhjaGFuZ2UuVHJhbnNwb3J0LkRpcmVjdG9yeURhdGEuSXNSZXNvdXJjZQIAAAUAagAJAAEAAAAAAAAABQAWAAIAAAUAQwACAAAFAEYABwADAAAABQBHAAIAAAUAEgAPAGIAAAAvbz1zZWN1bmV0L291PUV4Y2hhbmdlIEFkbWluaXN0cmF0aXZlIEdyb3VwIChGWURJQk9IRjIzU1BETFQpL2NuPVJlY2lwaWVudHMvY249U3RlZmZlbiBLbGFzc2VydDY4YwUACwAXAL4AAACheZxkHSGBRqAcAp3ukbifQ049REI2LENOPURhdGFiYXNlcyxDTj1FeGNoYW5nZSBBZG1pbmlzdHJhdGl2ZSBHcm91cCAoRllESUJPSEYyM1NQRExUKSxDTj1BZG1pbmlzdHJhdGl2ZSBHcm91cHMsQ049c2VjdW5ldCxDTj1NaWNyb3NvZnQgRXhjaGFuZ2UsQ049U2VydmljZXMsQ049Q29uZmlndXJhdGlvbixEQz1zZWN1bmV0LERDPWRlBQAOABEABiAS9uuMOkqzwmEZDvWNNQUAHQAPAAwAAABtYngtZXNzZW4tMDIFADwAAgAADwA2AAAATWljcm9zb2Z0LkV4Y2hhbmdlLlRyYW5zcG9ydC5NYWlsUmVjaXBpZW50LkRpc3BsYXlOYW1lDwARAAAAS2xhc3NlcnQsIFN0ZWZmZW4FAAwAAgAABQBsAAIAAAUAWAAXAEoAAADwxQktuQQJQ5LSCNoNNn64Q049S2xhc3NlcnQgU3RlZmZlbixPVT1Vc2VycyxPVT1NaWdyYXRpb24sREM9c2VjdW5ldCxEQz1kZQUAJgACAAEFACIADwAxAAAAQXV0b1Jlc3BvbnNlU3VwcHJlc3M6IDANClRyYW5zbWl0SGlzdG9yeTogRmFsc2UNCg8ALwAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuRXhwYW5zaW9uR3JvdXBUeXBlDwAVAAAATWVtYmVyc0dyb3VwRXhwYW5zaW9uBQAjAAIAAQ= X-CreatedBy: MSExchange15 X-HeloDomain: a.mx.secunet.com X-ExtendedProps: BQBjAAoAi5Hp8x1Q3AgFAGEACAABAAAABQA3AAIAAA8APAAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuTWFpbFJlY2lwaWVudC5Pcmdhbml6YXRpb25TY29wZREAAAAAAAAAAAAAAAAAAAAAAAUASQACAAEFAGIACgBqAAAAi4oAAAUABAAUIAEAAAAcAAAAc3RlZmZlbi5rbGFzc2VydEBzZWN1bmV0LmNvbQUABgACAAEFACkAAgABDwAJAAAAQ0lBdWRpdGVkAgABBQACAAcAAQAAAAUAAwAHAAAAAAAFAAUAAgABBQBkAA8AAwAAAEh1Yg= X-Source: SMTP:Default MBX-DRESDEN-01 X-SourceIPAddress: 62.96.220.36 X-EndOfInjectedXHeaders: 15806 Received: from cas-essen-01.secunet.de (10.53.40.201) by mbx-dresden-01.secunet.de (10.53.40.199) with Microsoft SMTP Server (version=S1_2, cipher=S_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.37; Fri, 29 Mar 2024 22:27:05 +0100 Received: from a.mx.secunet.com (62.96.220.36) by cas-essen-01.secunet.de (10.53.40.201) with Microsoft SMTP Server (version=S1_2, cipher=S_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Fri, 29 Mar 2024 22:27:05 +0100 Received: from localhost (localhost [127.0.0.1]) by a.mx.secunet.com (Postfix) with ESMTP id D2D3020897 for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 22:27:05 +0100 (CET) X-Virus-Scanned: by secunet X-Spam-Flag: NO X-Spam-Score: -2.749 X-Spam-Level: X-Spam-Status: No, score=.749 tagged_above=99 required=1 tests=AYES_00=.9, DKIM_SIGNED=1, DKIM_VALID=.1, DKIM_VALID_AU=.1, FREEMAIL_FORGED_FROMDOMAIN=001, FREEMAIL_FROM=001, HEADER_FROM_DIFFERENT_DOMAINS=249, MAILING_LIST_MULTI=, RCVD_IN_DNSWL_NONE=.0001, SPF_HELO_NONE=001, SPF_PASS=.001] autolearn=available autolearn_force= Authentication-Results: a.mx.secunet.com (amavisd-new); dkim=ss (2048-bit key) header.d=ail.com Received: from a.mx.secunet.com ([127.0.0.1]) by localhost (a.mx.secunet.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id loUNJzs71Z7j for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 22:27:04 +0100 (CET) Received-SPF: Pass (sender SPF authorized) identity=ilfrom; client-ip\x147.75.199.223; helo=.mirrors.kernel.org; envelope-from=tdev+bounces-83464-steffen.klassert=cunet.com@vger.kernel.org; receiver=effen.klassert@secunet.com DKIM-Filter: OpenDKIM Filter v2.11.0 a.mx.secunet.com 8242220892 Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org [147.75.199.223]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by a.mx.secunet.com (Postfix) with ESMTPS id 8242220892 for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 22:27:04 +0100 (CET) Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 5FB5A1C20AE8 for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 21:27:03 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id B79EE13BC09; Fri, 29 Mar 2024 21:26:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=02OONrK" X-Original-To: netdev@vger.kernel.org Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 910E138FAD; Fri, 29 Mar 2024 21:26:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=ne smtp.client-ip 9.85.221.54 ARC-Seal: i= a=a-sha256; d=bspace.kernel.org; s=c-20240116; t\x1711747613; cv=ne; b=hT6rBlhYRPp3nILeYSgLH5jgw3/ahNcI+sDsQW7a6Oq02j3c5prNzS0n5vJx9TVbZQcqIgvdV9HTstLuoPtW/jT0vcYIvofBPyJUl7qtv3N1ZcCALaUGLdrgZBAPv1JUH+kyrmT9ybQHjLzeMASpgNwI0hFSXpPucdmpDSanwARC-Message-Signature: i= a=a-sha256; d=bspace.kernel.org; s=c-20240116; t\x1711747613; c=laxed/simple; bh=2kYFst4z/jzDluAZKuRs3VnNLG/lJw8nm0VcFYikM= h=ME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=WQ2A0CseIsL9FgYjN6zPufVIpQqiRzxpxfWWir2S2bRrkalHeIYKni2SW9MnB9+iKThkvwKO+Zn5Zssl7qqHveQk51ff9EJpUn5NhPHVmY4QJ0TF4i9BP87FJsd1aA9g47SAdGsUrEEJEsLIDX4ANMZG8S1Gjnd1qw8tZdsRMARC-Authentication-Results: i= smtp.subspace.kernel.org; dmarc=ss (p=ne dis=ne) header.from=ail.com; spf=ss smtp.mailfrom=ail.com; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=2OONrK; arc=ne smtp.client-ip 9.85.221.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=ss (p=ne dis=ne) header.from=ail.com Authentication-Results: smtp.subspace.kernel.org; spf=ss smtp.mailfrom=ail.com Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-341c9926f98so1441462f8f.1; Fri, 29 Mar 2024 14:26:51 -0700 (PDT) DKIM-Signature: v= a=a-sha256; c=laxed/relaxed; d=ail.com; s 230601; t\x1711747610; x\x1712352410; darn=er.kernel.org; h=ntent-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=v9NiziDyCYu9Ssy1MqwcYz3Obg+5qR7kJAYV5rLhc= b=2OONrKn+4k88m3UEDllV8o8JUuB2NguuTGHZzCXHNiOAa9yBQIiaGmfQsGVV86q3 vwTm5pN9qCEJWhO3/1dihonK9iCD1MV6zVd13EikzldfB4dxhYg+3Wg6vO0czEsr1HAc Xnut8+4zSVKD/nOdEua3WuEO6t2FYuiIILMfvrA145eDAhyL+bLHHiztPM0IEukc3eMT QwkIybVK8O22yg5D4IKFhCt9QnTfLrqtSCdTICpGqqxHghVj4Ift5u/IfjSrR2EkEVNe CwHVBQEan1clULUfxVR4imy+rZpP4IILFDkrBGL8rfHg7ZbgGF/FY9ZH05/vNFFyN9i4 L+ZQ= X-Google-DKIM-Signature: v= a=a-sha256; c=laxed/relaxed; d\x1e100.net; s 230601; t\x1711747610; x\x1712352410; h=ntent-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=v9NiziDyCYu9Ssy1MqwcYz3Obg+5qR7kJAYV5rLhc= b=ml1tLtFlyP9+6qzsr/Y/Kp5igFrK2oUC/WIz//Zmu/5fSRzhVpw37aRWeKK6hdX/ FxsJBUEiOwP7rnH9cVh0eWtHbz2qaashwfXDE6sVeXzEDIGZGpGu23nNdVQXdmSsPoym +exTdbi8fY0pCW6bB8Bl9QHeCnAfsqlPKJyaAjnrOHw5Qw9qdI1S9gC2eq0WbnVy6iHQ 7tLKPnU50UXa4ii7btWPm2rqB3S45MtzAeOOLaaKgubovEhMNhC+usRZ5ijfKdVYpIhu bTcuciA022kGsoViz3poqnsJ0p8W9ojwHusnFV0FwDvrqrw9MTAV+LH+DM//g3kcbyjr T4ig= X-Forwarded-Encrypted: i= AJvYcCUms6Y8mQXobn6Doc4LggzSXZfoQDIm/IKPAkO+/bgLih/BXEZjjKJJx2V38SsUeCaJy1R/1BisLsDktGAF092Q8+cQmdOHd3i65roTYjtnFDPo2/M4ZlO8bb6wJr5fOR5UJWeLxlciGCxFLtE/mIj3WRxJMuCKajUX X-Gm-Message-State: AOJu0YwDLEfVDuSKNNImIfCehbMu01XecTBUg/BxEe3Dr2BLV02Sld8L HnQdjTLXkZXA3IHu11Z7n/x+G0xL6V5UIl2p3WAyQ6nodkCzPQbShE13cVc4xK07Mw0KAcUpPfT hd4UbQzqEaNrmK52fddVQ5NxZI1IX-Google-Smtp-Source: AGHT+IEhPG1B/8mDvGNpkDj4PWCUpLjQ708pGc8G6bha6o2gM1Lwf8UNLZI5EJ57kTgt1wZPIajCKtTg1SeDSsMj41AX-Received: by 2002:a05:6000:24a:b0:33e:4238:8615 with SMTP id m10-20020a056000024a00b0033e42388615mr1774070wrz.40.1711747609718; Fri, 29 Mar 2024 14:26:49 -0700 (PDT) Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: <netdev.vger.kernel.org> List-Subscribe: <mailto:netdev+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:netdev+unsubscribe@vger.kernel.org> MIME-Version: 1.0 References: <20240329094906.18147-1-ubizjak@gmail.com> <20240329094906.18147-2-ubizjak@gmail.com> In-Reply-To: <20240329094906.18147-2-ubizjak@gmail.com> From: Alexei Starovoitov <alexei.starovoitov@gmail.com> Date: Fri, 29 Mar 2024 14:26:38 -0700 Message-ID: <CAADnVQLZnkm8psPvmUOS1FDacXdJPxQ79rQJ33F00dkS9czw1Q@mail.gmail.com> Subject: Re: [PATCH RESEND bpf 1/2] x86/bpf: Fix IP after emitting call depth accounting To: Uros Bizjak <ubizjak@gmail.com> Cc: X86 ML <x86@kernel.org>, bpf <bpf@vger.kernel.org>, Network Development <netdev@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, =TF-8?Q?Joan_Bruguera_Micó?=oanbrugueram@gmail.com>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net> Content-Type: text/plain; charset=TF-8" Content-Transfer-Encoding: quoted-printable Return-Path: netdev+bounces-83464-steffen.klassert=cunet.com@vger.kernel.org X-MS-Exchange-Organization-OriginalArrivalTime: 29 Mar 2024 21:27:05.8796 (UTC) X-MS-Exchange-Organization-Network-Message-Id: 6a725642-594b-4e0f-2297-08dc5036fb91 X-MS-Exchange-Organization-OriginalClientIPAddress: 62.96.220.36 X-MS-Exchange-Organization-OriginalServerIPAddress: 10.53.40.201 X-MS-Exchange-Organization-Cross-Premises-Headers-Processed: cas-essen-01.secunet.de X-MS-Exchange-Organization-OrderedPrecisionLatencyInProgress: LSRV=x-dresden-01.secunet.de:TOTAL-HUB=406|SMR=353(SMRDE=050|SMRC=302(SMRCL=102|X-SMRCR=303))|CAT=052(CATOS=011 (CATSM=011(CATSM-Malware Agent=011))|CATRESL=020(CATRESLP2R=001)|CATORES=018 (CATRS=018(CATRS-Index Routing Agent=017)));2024-03-29T21:27:06.276Z X-MS-Exchange-Forest-ArrivalHubServer: mbx-dresden-01.secunet.de X-MS-Exchange-Organization-AuthSource: cas-essen-01.secunet.de X-MS-Exchange-Organization-AuthAs: Anonymous X-MS-Exchange-Organization-FromEntityHeader: Internet X-MS-Exchange-Organization-OriginalSize: 9939 X-MS-Exchange-Organization-HygienePolicy: Standard X-MS-Exchange-Organization-MessageLatency: SRVÊs-essen-01.secunet.de:TOTAL-FE=009|SMR=008(SMRPI=006(SMRPI-FrontendProxyAgent=006)) X-MS-Exchange-Organization-AVStamp-Enterprise: 1.0 X-MS-Exchange-Organization-Recipient-Limit-Verified: True X-MS-Exchange-Organization-TotalRecipientCount: 1 X-MS-Exchange-Organization-Rules-Execution-History: 0b0cf904-14ac-4724-8bdf-482ee6223cf2%%%fd34672d-751c-45ae-a963-ed177fcabe23%%%d8080257-b0c3-47b4-b0db-23bc0c8ddb3c%%%95e591a2-5d7d-4afa-b1d0-7573d6c0a5d9%%%f7d0f6bc-4dcc-4876-8c5d-b3d6ddbb3d55%%%16355082-c50b-4214-9c7d-d39575f9f79b X-MS-Exchange-Forest-RulesExecuted: mbx-dresden-01 X-MS-Exchange-Organization-RulesExecuted: mbx-dresden-01 X-MS-Exchange-Forest-IndexAgent-0: AQ0CZW4AAWoEAAAPAAADH4sIAAAAAAAEAJVU227bRhAd6i5Zkls3Rd GXYpAnX3Q3fZHqpLYTF3Vhw0GStkBf5BW5ktaiSHa5iu1egP5RH/rY x/5Jv6SzS8o24MhOhAU1mjlz5szMUv+Uz3z8VooanjKJnW4NO62OjU xhp2d3//vzr4NT/EEGER6KXy/YBPdmA2Psj6ZMeA0nmD7HSxko3quU ntMhrmDaw+8D5uOhnI1mXDI8Fc6/f+PeBTkHiW96hyDJPHAvZpFCNe Z4/ApDFkXcRRXgOZ8K1Q+ZcsbnGAUoFDrMc2YeUzwycCeQkjsKg+Ew 4kpzDQNpIi8OTk5Q+JGSM0eJwEcxxPOr3e0+MXh9l4dq3Df0zHGCma +EPzpH7YiI1OUNzXVGRPJSRBwv6QjPw2giQlIy5XepI2S+i9OAWvDE hHvX6EgWjRvzuYgrHvVw0OFdd8i37AG3cfUpSWkOwmEPj6ZxWx4aUX irR0uW/JeZkNx9uqapToQ/6eFYqTDqNZteIHljwqXPvUYgR01vMvWa tMTNVru11Wnb3U6n0dlq2Tub9XZ9wQqamvaNGPncrdMQ64Prj18hvn B6eODxKy7wjWIyeBcIFbzDPRap/Vt9N9CXzBfcw8NATqbM93HPNY59 EYw4k4PgquFzZdD1el1/IZPOuKknRgE9tf4FLY6qhw0Hf8ct3NjYSJ BtHAqP7sWY+SPu1nBTL4pLs6bVjTW65DRmj8e/62vJilwxHFKxEW2C NR8qNngoqpmE7/IrZDsDtsvbO7vb9laj0eoOd1yn69ru9ha2W61t20 6ae7iaBlFnjxfd38e6vdOtdXFDf7Vb2hMppoRDgpS51uber852cX09 DGUwqiFtycX14cx35rYI5wO5ly2jwYczIP5mtGP8iUPMvOTc7Yvwa4 NJPmev3h6fHv989Lr/3fHLo/6PB69XNeOaAdUT0MMv7mqi536e5Gom fbz9H7kLraEIa9i6OtqNk+Z67yjFZ4ShwMfVr5R+Gl+jHyjURJWSpn j20Rzf6MetqkdauSP6bk/4x6KNXsym4YdtM361Og3bbrQMW6UEkIJ0 GrJpC8p0IJOCTAayKSudB8hDgewcFPIWfAqFLOQImYVKBnLkL0CRjI oFX0DVhJZykE9rhjQZmseCksaTnaZTgiWKEmcOSmSkTaElKBOSqn9l fmYNcwnKxigWoUSJ5IlPGSpxFvGTDGIuw7KhSgCURVUopQBV0pa14D RpIR8nVmGZbAtSlURzIWdBJZGdeV9TBfJTyhKVtqBq7BtnAUqU/gTy Rd3RJ5Qe95U2k4wrWhaNASyrrJ+QIk/eKsZ2yVqOjSJ8ljEaYsB7SC C1OKS7LkO1bFVyADmoLkYWF4YsmhKVzxqpqdhO6buRS0ZHyWbCKbNQ cwH0Sc2XntFjzMZs9zCf07NgwRIUU7BMIlaglEnAK/dWv2RyV4h2fn 9yMZtZ0MqcR1dZkHtzuygltwDz5X3+OTOBnzzStVWJB6VHZKW1/T+V 3ePHhQkAAAEKnwQ8P3htbCB2ZXJzaW9uPSIxLjAiIGVuY29kaW5nPS J1dGYtMTYiPz4NCjxFbWFpbFNldD4NCiAgPFZlcnNpb24+MTUuMC4w LjA8L1ZlcnNpb24+DQogIDxFbWFpbHM+DQogICAgPEVtYWlsIFN0YX J0SW5kZXg9IjQ1Ij4NCiAgICAgIDxFbWFpbFN0cmluZz51Yml6amFr QGdtYWlsLmNvbTwvRW1haWxTdHJpbmc+DQogICAgPC9FbWFpbD4NCi AgICA8RW1haWwgU3RhcnRJbmRleD0iMTAzIj4NCiAgICAgIDxFbWFp bFN0cmluZz5qb2FuYnJ1Z3VlcmFtQGdtYWlsLmNvbTwvRW1haWxTdH Jpbmc+DQogICAgPC9FbWFpbD4NCiAgICA8RW1haWwgU3RhcnRJbmRl eD0iNjA1IiBQb3NpdGlvbj0iT3RoZXIiPg0KICAgICAgPEVtYWlsU3 RyaW5nPmFzdEBrZXJuZWwub3JnPC9FbWFpbFN0cmluZz4NCiAgICA8 L0VtYWlsPg0KICAgIDxFbWFpbCBTdGFydEluZGV4PSI2NDUiIFBvc2 l0aW9uPSJPdGhlciI+DQogICAgICA8RW1haWxTdHJpbmc+ZGFuaWVs QGlvZ2VhcmJveC5uZXQ8L0VtYWlsU3RyaW5nPg0KICAgIDwvRW1haW w+DQogIDwvRW1haWxzPg0KPC9FbWFpbFNldD4BC6ACPD94bWwgdmVy c2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTE2Ij8+DQo8VXJsU2V0Pg 0KICA8VmVyc2lvbj4xNS4wLjAuMDwvVmVyc2lvbj4NCiAgPFVybHM+ DQogICAgPFVybCBTdGFydEluZGV4PSI0MzciIFBvc2l0aW9uPSJPdG hlciIgVHlwZT0iVXJsIj4NCiAgICAgIDxVcmxTdHJpbmc+aHR0cHM6 Ly9sb3JlLmtlcm5lbC5vcmcvbGttbC8yMDIzMDEwNTIxNDkyMi4yNT A0NzMtMS1qb2FuYnJ1Z3VlcmFtQGdtYWlsLmNvbS88L1VybFN0cmlu Zz4NCiAgICA8L1VybD4NCiAgPC9VcmxzPg0KPC9VcmxTZXQ+AQzjBj w/eG1sIHZlcnNpb249IjEuMCIgZW5jb2Rpbmc9InV0Zi0xNiI/Pg0K PENvbnRhY3RTZXQ+DQogIDxWZXJzaW9uPjE1LjAuMC4wPC9WZXJzaW 9uPg0KICA8Q29udGFjdHM+DQogICAgPENvbnRhY3QgU3RhcnRJbmRl eD0iMzIiPg0KICAgICAgPFBlcnNvbiBTdGFydEluZGV4PSIzMiI+DQ ogICAgICAgIDxQZXJzb25TdHJpbmc+VXJvcyBCaXpqYWs8L1BlcnNv blN0cmluZz4NCiAgICAgIDwvUGVyc29uPg0KICAgICAgPEVtYWlscz 4NCiAgICAgICAgPEVtYWlsIFN0YXJ0SW5kZXg9IjQ1Ij4NCiAgICAg ICAgICA8RW1haWxTdHJpbmc+dWJpempha0BnbWFpbC5jb208L0VtYW lsU3RyaW5nPg0KICAgICAgICA8L0VtYWlsPg0KICAgICAgPC9FbWFp bHM+DQogICAgICA8Q29udGFjdFN0cmluZz5Vcm9zIEJpemphayAmbH Q7dWJpempha0BnbWFpbC5jb208L0NvbnRhY3RTdHJpbmc+DQogICAg PC9Db250YWN0Pg0KICAgIDxDb250YWN0IFN0YXJ0SW5kZXg9IjgzIj 4NCiAgICAgIDxQZXJzb24gU3RhcnRJbmRleD0iODMiPg0KICAgICAg ICA8UGVyc29uU3RyaW5nPkpvYW4gQnJ1Z3VlcmE8L1BlcnNvblN0cm luZz4NCiAgICAgIDwvUGVyc29uPg0KICAgICAgPEVtYWlscz4NCiAg ICAgICAgPEVtYWlsIFN0YXJ0SW5kZXg9IjEwMyI+DQogICAgICAgIC AgPEVtYWlsU3RyaW5nPmpvYW5icnVndWVyYW1AZ21haWwuY29tPC9F bWFpbFN0cmluZz4NCiAgICAgICAgPC9FbWFpbD4NCiAgICAgIDwvRW 1haWxzPg0KICAgICAgPENvbnRhY3RTdHJpbmc+Sm9hbiBCcnVndWVy YSBNaWPDsyAmbHQ7am9hbmJydWd1ZXJhbUBnbWFpbC5jb208L0Nvbn RhY3RTdHJpbmc+DQogICAgPC9Db250YWN0Pg0KICA8L0NvbnRhY3Rz Pg0KPC9Db250YWN0U2V0PgEOzgFSZXRyaWV2ZXJPcGVyYXRvciwxMC wxO1JldHJpZXZlck9wZXJhdG9yLDExLDE7UG9zdERvY1BhcnNlck9w ZXJhdG9yLDEwLDA7UG9zdERvY1BhcnNlck9wZXJhdG9yLDExLDA7UG 9zdFdvcmRCcmVha2VyRGlhZ25vc3RpY09wZXJhdG9yLDEwLDA7UG9z dFdvcmRCcmVha2VyRGlhZ25vc3RpY09wZXJhdG9yLDExLDA7VHJhbn Nwb3J0V3JpdGVyUHJvZHVjZXIsMjAsNw= X-MS-Exchange-Forest-IndexAgent: 1 3061 X-MS-Exchange-Forest-EmailMessageHash: 35AF81FD X-MS-Exchange-Forest-Language: en X-MS-Exchange-Organization-Processed-By-Journaling: Journal Agent On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > Adjust the IP passed to `emit_patch` so it calculates the correct offset > for the CALL instruction if `x86_call_depth_emit_accounting` emits code. > Otherwise we will skip some instructions and most likely crash. > > Fixes: b2e9dfe54be4 ("x86/bpf: Emit call depth accounting if required") > Link: https://lore.kernel.org/lkml/20230105214922.250473-1-joanbrugueram@gmail.com/ > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > --- > arch/x86/net/bpf_jit_comp.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index a7ba8e178645..09f7dc9d4d65 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -479,9 +479,10 @@ static int emit_call(u8 **pprog, void *func, void *ip) > > static int emit_rsb_call(u8 **pprog, void *func, void *ip) > { > + void *adjusted_ip; > OPTIMIZER_HIDE_VAR(func); > - x86_call_depth_emit_accounting(pprog, func); > - return emit_patch(pprog, func, ip, 0xE8); > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); Why not just ip += x86_call_depth_emit_accounting(pprog, func); ? > + return emit_patch(pprog, func, adjusted_ip, 0xE8); > } > > static int emit_jump(u8 **pprog, void *func, void *ip) > -- > 2.44.0 > X-sender: <linux-kernel+bounces-125445-steffen.klassert=cunet.com@vger.kernel.org> X-Receiver: <steffen.klassert@secunet.com> ORCPT=c822;steffen.klassert@secunet.com NOTIFY=VER; X-ExtendedProps=AVABYAAgAAAAUAFAARAPDFCS25BAlDktII2g02frgPADUAAABNaWNyb3NvZnQuRXhjaGFuZ2UuVHJhbnNwb3J0LkRpcmVjdG9yeURhdGEuSXNSZXNvdXJjZQIAAAUAagAJAAEAAAAAAAAABQAWAAIAAAUAQwACAAAFAEYABwADAAAABQBHAAIAAAUAEgAPAGIAAAAvbz1zZWN1bmV0L291PUV4Y2hhbmdlIEFkbWluaXN0cmF0aXZlIEdyb3VwIChGWURJQk9IRjIzU1BETFQpL2NuPVJlY2lwaWVudHMvY249U3RlZmZlbiBLbGFzc2VydDY4YwUACwAXAL4AAACheZxkHSGBRqAcAp3ukbifQ049REI2LENOPURhdGFiYXNlcyxDTj1FeGNoYW5nZSBBZG1pbmlzdHJhdGl2ZSBHcm91cCAoRllESUJPSEYyM1NQRExUKSxDTj1BZG1pbmlzdHJhdGl2ZSBHcm91cHMsQ049c2VjdW5ldCxDTj1NaWNyb3NvZnQgRXhjaGFuZ2UsQ049U2VydmljZXMsQ049Q29uZmlndXJhdGlvbixEQz1zZWN1bmV0LERDPWRlBQAOABEABiAS9uuMOkqzwmEZDvWNNQUAHQAPAAwAAABtYngtZXNzZW4tMDIFADwAAgAADwA2AAAATWljcm9zb2Z0LkV4Y2hhbmdlLlRyYW5zcG9ydC5NYWlsUmVjaXBpZW50LkRpc3BsYXlOYW1lDwARAAAAS2xhc3NlcnQsIFN0ZWZmZW4FAAwAAgAABQBsAAIAAAUAWAAXAEoAAADwxQktuQQJQ5LSCNoNNn64Q049S2xhc3NlcnQgU3RlZmZlbixPVT1Vc2VycyxPVT1NaWdyYXRpb24sREM9c2VjdW5ldCxEQz1kZQUAJgACAAEFACIADwAxAAAAQXV0b1Jlc3BvbnNlU3VwcHJlc3M6IDANClRyYW5zbWl0SGlzdG9yeTogRmFsc2UNCg8ALwAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuRXhwYW5zaW9uR3JvdXBUeXBlDwAVAAAATWVtYmVyc0dyb3VwRXhwYW5zaW9uBQAjAAIAAQ= X-CreatedBy: MSExchange15 X-HeloDomain: b.mx.secunet.com X-ExtendedProps: BQBjAAoAtpHp8x1Q3AgFAGEACAABAAAABQA3AAIAAA8APAAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuTWFpbFJlY2lwaWVudC5Pcmdhbml6YXRpb25TY29wZREAAAAAAAAAAAAAAAAAAAAAAAUASQACAAEFAGIACgBsAAAAi4oAAAUABAAUIAEAAAAcAAAAc3RlZmZlbi5rbGFzc2VydEBzZWN1bmV0LmNvbQUABgACAAEFACkAAgABDwAJAAAAQ0lBdWRpdGVkAgABBQACAAcAAQAAAAUAAwAHAAAAAAAFAAUAAgABBQBkAA8AAwAAAEh1Yg= X-Source: SMTP:Default MBX-DRESDEN-01 X-SourceIPAddress: 62.96.220.37 X-EndOfInjectedXHeaders: 15923 Received: from cas-essen-02.secunet.de (10.53.40.202) by mbx-dresden-01.secunet.de (10.53.40.199) with Microsoft SMTP Server (version=S1_2, cipher=S_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.37; Fri, 29 Mar 2024 22:27:17 +0100 Received: from b.mx.secunet.com (62.96.220.37) by cas-essen-02.secunet.de (10.53.40.202) with Microsoft SMTP Server (version=S1_2, cipher=S_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Fri, 29 Mar 2024 22:27:17 +0100 Received: from localhost (localhost [127.0.0.1]) by b.mx.secunet.com (Postfix) with ESMTP id A01F52032C for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 22:27:17 +0100 (CET) X-Virus-Scanned: by secunet X-Spam-Flag: NO X-Spam-Score: -2.749 X-Spam-Level: X-Spam-Status: No, score=.749 tagged_above=99 required=1 tests=AYES_00=.9, DKIM_SIGNED=1, DKIM_VALID=.1, DKIM_VALID_AU=.1, FREEMAIL_FORGED_FROMDOMAIN=001, FREEMAIL_FROM=001, HEADER_FROM_DIFFERENT_DOMAINS=249, MAILING_LIST_MULTI=, RCVD_IN_DNSWL_NONE=.0001, SPF_HELO_NONE=001, SPF_PASS=.001] autolearn=m autolearn_force= Authentication-Results: a.mx.secunet.com (amavisd-new); dkim=ss (2048-bit key) header.d=ail.com Received: from b.mx.secunet.com ([127.0.0.1]) by localhost (a.mx.secunet.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id s3774IfGRhS4 for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 22:27:13 +0100 (CET) Received-SPF: Pass (sender SPF authorized) identity=ilfrom; client-ip\x147.75.80.249; helo=.mirrors.kernel.org; envelope-from=nux-kernel+bounces-125445-steffen.klassert=cunet.com@vger.kernel.org; receiver=effen.klassert@secunet.com DKIM-Filter: OpenDKIM Filter v2.11.0 b.mx.secunet.com AC91D200BB Authentication-Results: b.mx.secunet.com; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=02OONrK" Received: from am.mirrors.kernel.org (am.mirrors.kernel.org [147.75.80.249]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by b.mx.secunet.com (Postfix) with ESMTPS id AC91D200BB for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 22:27:13 +0100 (CET) Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 4FEB71F24381 for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 21:27:13 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 7AC5013CAAE; Fri, 29 Mar 2024 21:26:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=02OONrK" Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 910E138FAD; Fri, 29 Mar 2024 21:26:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=ne smtp.client-ip 9.85.221.54 ARC-Seal: i= a=a-sha256; d=bspace.kernel.org; s=c-20240116; t\x1711747613; cv=ne; b=hT6rBlhYRPp3nILeYSgLH5jgw3/ahNcI+sDsQW7a6Oq02j3c5prNzS0n5vJx9TVbZQcqIgvdV9HTstLuoPtW/jT0vcYIvofBPyJUl7qtv3N1ZcCALaUGLdrgZBAPv1JUH+kyrmT9ybQHjLzeMASpgNwI0hFSXpPucdmpDSanwARC-Message-Signature: i= a=a-sha256; d=bspace.kernel.org; s=c-20240116; t\x1711747613; c=laxed/simple; bh=2kYFst4z/jzDluAZKuRs3VnNLG/lJw8nm0VcFYikM= h=ME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=WQ2A0CseIsL9FgYjN6zPufVIpQqiRzxpxfWWir2S2bRrkalHeIYKni2SW9MnB9+iKThkvwKO+Zn5Zssl7qqHveQk51ff9EJpUn5NhPHVmY4QJ0TF4i9BP87FJsd1aA9g47SAdGsUrEEJEsLIDX4ANMZG8S1Gjnd1qw8tZdsRMARC-Authentication-Results: i= smtp.subspace.kernel.org; dmarc=ss (p=ne dis=ne) header.from=ail.com; spf=ss smtp.mailfrom=ail.com; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=2OONrK; arc=ne smtp.client-ip 9.85.221.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=ss (p=ne dis=ne) header.from=ail.com Authentication-Results: smtp.subspace.kernel.org; spf=ss smtp.mailfrom=ail.com Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-341c9926f98so1441462f8f.1; Fri, 29 Mar 2024 14:26:51 -0700 (PDT) DKIM-Signature: v= a=a-sha256; c=laxed/relaxed; d=ail.com; s 230601; t\x1711747610; x\x1712352410; darn=er.kernel.org; h=ntent-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=v9NiziDyCYu9Ssy1MqwcYz3Obg+5qR7kJAYV5rLhc= b=2OONrKn+4k88m3UEDllV8o8JUuB2NguuTGHZzCXHNiOAa9yBQIiaGmfQsGVV86q3 vwTm5pN9qCEJWhO3/1dihonK9iCD1MV6zVd13EikzldfB4dxhYg+3Wg6vO0czEsr1HAc Xnut8+4zSVKD/nOdEua3WuEO6t2FYuiIILMfvrA145eDAhyL+bLHHiztPM0IEukc3eMT QwkIybVK8O22yg5D4IKFhCt9QnTfLrqtSCdTICpGqqxHghVj4Ift5u/IfjSrR2EkEVNe CwHVBQEan1clULUfxVR4imy+rZpP4IILFDkrBGL8rfHg7ZbgGF/FY9ZH05/vNFFyN9i4 L+ZQ= X-Google-DKIM-Signature: v= a=a-sha256; c=laxed/relaxed; d\x1e100.net; s 230601; t\x1711747610; x\x1712352410; h=ntent-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=v9NiziDyCYu9Ssy1MqwcYz3Obg+5qR7kJAYV5rLhc= b=ml1tLtFlyP9+6qzsr/Y/Kp5igFrK2oUC/WIz//Zmu/5fSRzhVpw37aRWeKK6hdX/ FxsJBUEiOwP7rnH9cVh0eWtHbz2qaashwfXDE6sVeXzEDIGZGpGu23nNdVQXdmSsPoym +exTdbi8fY0pCW6bB8Bl9QHeCnAfsqlPKJyaAjnrOHw5Qw9qdI1S9gC2eq0WbnVy6iHQ 7tLKPnU50UXa4ii7btWPm2rqB3S45MtzAeOOLaaKgubovEhMNhC+usRZ5ijfKdVYpIhu bTcuciA022kGsoViz3poqnsJ0p8W9ojwHusnFV0FwDvrqrw9MTAV+LH+DM//g3kcbyjr T4ig= X-Forwarded-Encrypted: i= AJvYcCUms6Y8mQXobn6Doc4LggzSXZfoQDIm/IKPAkO+/bgLih/BXEZjjKJJx2V38SsUeCaJy1R/1BisLsDktGAF092Q8+cQmdOHd3i65roTYjtnFDPo2/M4ZlO8bb6wJr5fOR5UJWeLxlciGCxFLtE/mIj3WRxJMuCKajUX X-Gm-Message-State: AOJu0YwDLEfVDuSKNNImIfCehbMu01XecTBUg/BxEe3Dr2BLV02Sld8L HnQdjTLXkZXA3IHu11Z7n/x+G0xL6V5UIl2p3WAyQ6nodkCzPQbShE13cVc4xK07Mw0KAcUpPfT hd4UbQzqEaNrmK52fddVQ5NxZI1IX-Google-Smtp-Source: AGHT+IEhPG1B/8mDvGNpkDj4PWCUpLjQ708pGc8G6bha6o2gM1Lwf8UNLZI5EJ57kTgt1wZPIajCKtTg1SeDSsMj41AX-Received: by 2002:a05:6000:24a:b0:33e:4238:8615 with SMTP id m10-20020a056000024a00b0033e42388615mr1774070wrz.40.1711747609718; Fri, 29 Mar 2024 14:26:49 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 References: <20240329094906.18147-1-ubizjak@gmail.com> <20240329094906.18147-2-ubizjak@gmail.com> In-Reply-To: <20240329094906.18147-2-ubizjak@gmail.com> From: Alexei Starovoitov <alexei.starovoitov@gmail.com> Date: Fri, 29 Mar 2024 14:26:38 -0700 Message-ID: <CAADnVQLZnkm8psPvmUOS1FDacXdJPxQ79rQJ33F00dkS9czw1Q@mail.gmail.com> Subject: Re: [PATCH RESEND bpf 1/2] x86/bpf: Fix IP after emitting call depth accounting To: Uros Bizjak <ubizjak@gmail.com> Cc: X86 ML <x86@kernel.org>, bpf <bpf@vger.kernel.org>, Network Development <netdev@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, =TF-8?Q?Joan_Bruguera_Micó?=oanbrugueram@gmail.com>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net> Content-Type: text/plain; charset=TF-8" Content-Transfer-Encoding: quoted-printable Return-Path: linux-kernel+bounces-125445-steffen.klassert=cunet.com@vger.kernel.org X-MS-Exchange-Organization-OriginalArrivalTime: 29 Mar 2024 21:27:17.6856 (UTC) X-MS-Exchange-Organization-Network-Message-Id: cbecf88b-6e62-4b3a-91cd-08dc5037029a X-MS-Exchange-Organization-OriginalClientIPAddress: 62.96.220.37 X-MS-Exchange-Organization-OriginalServerIPAddress: 10.53.40.202 X-MS-Exchange-Organization-Cross-Premises-Headers-Processed: cas-essen-02.secunet.de X-MS-Exchange-Organization-OrderedPrecisionLatencyInProgress: LSRV=x-dresden-01.secunet.de:TOTAL-HUB=424|SMR=371(SMRDE=050|SMRC=320(SMRCL=102|X-SMRCR=320))|CAT=051(CATOS=011 (CATSM=011(CATSM-Malware Agent=010))|CATRESL=020(CATRESLP2R=001)|CATORES=018 (CATRS=018(CATRS-Index Routing Agent=017)));2024-03-29T21:27:18.099Z X-MS-Exchange-Forest-ArrivalHubServer: mbx-dresden-01.secunet.de X-MS-Exchange-Organization-AuthSource: cas-essen-02.secunet.de X-MS-Exchange-Organization-AuthAs: Anonymous X-MS-Exchange-Organization-FromEntityHeader: Internet X-MS-Exchange-Organization-OriginalSize: 10057 X-MS-Exchange-Organization-HygienePolicy: Standard X-MS-Exchange-Organization-MessageLatency: SRVÊs-essen-02.secunet.de:TOTAL-FE=007|SMR=007(SMRPI=005(SMRPI-FrontendProxyAgent=005)) X-MS-Exchange-Organization-AVStamp-Enterprise: 1.0 X-MS-Exchange-Organization-Recipient-Limit-Verified: True X-MS-Exchange-Organization-TotalRecipientCount: 1 X-MS-Exchange-Organization-Rules-Execution-History: 0b0cf904-14ac-4724-8bdf-482ee6223cf2%%%fd34672d-751c-45ae-a963-ed177fcabe23%%%d8080257-b0c3-47b4-b0db-23bc0c8ddb3c%%%95e591a2-5d7d-4afa-b1d0-7573d6c0a5d9%%%f7d0f6bc-4dcc-4876-8c5d-b3d6ddbb3d55%%%16355082-c50b-4214-9c7d-d39575f9f79b X-MS-Exchange-Forest-RulesExecuted: mbx-dresden-01 X-MS-Exchange-Organization-RulesExecuted: mbx-dresden-01 X-MS-Exchange-Forest-IndexAgent-0: AQ0CZW4AAWoEAAAPAAADH4sIAAAAAAAEAJVU227bRhAd6i5Zkls3Rd GXYpAnX3Q3fZHqpLYTF3Vhw0GStkBf5BW5ktaiSHa5iu1egP5RH/rY x/5Jv6SzS8o24MhOhAU1mjlz5szMUv+Uz3z8VooanjKJnW4NO62OjU xhp2d3//vzr4NT/EEGER6KXy/YBPdmA2Psj6ZMeA0nmD7HSxko3quU ntMhrmDaw+8D5uOhnI1mXDI8Fc6/f+PeBTkHiW96hyDJPHAvZpFCNe Z4/ApDFkXcRRXgOZ8K1Q+ZcsbnGAUoFDrMc2YeUzwycCeQkjsKg+Ew 4kpzDQNpIi8OTk5Q+JGSM0eJwEcxxPOr3e0+MXh9l4dq3Df0zHGCma +EPzpH7YiI1OUNzXVGRPJSRBwv6QjPw2giQlIy5XepI2S+i9OAWvDE hHvX6EgWjRvzuYgrHvVw0OFdd8i37AG3cfUpSWkOwmEPj6ZxWx4aUX irR0uW/JeZkNx9uqapToQ/6eFYqTDqNZteIHljwqXPvUYgR01vMvWa tMTNVru11Wnb3U6n0dlq2Tub9XZ9wQqamvaNGPncrdMQ64Prj18hvn B6eODxKy7wjWIyeBcIFbzDPRap/Vt9N9CXzBfcw8NATqbM93HPNY59 EYw4k4PgquFzZdD1el1/IZPOuKknRgE9tf4FLY6qhw0Hf8ct3NjYSJ BtHAqP7sWY+SPu1nBTL4pLs6bVjTW65DRmj8e/62vJilwxHFKxEW2C NR8qNngoqpmE7/IrZDsDtsvbO7vb9laj0eoOd1yn69ru9ha2W61t20 6ae7iaBlFnjxfd38e6vdOtdXFDf7Vb2hMppoRDgpS51uber852cX09 DGUwqiFtycX14cx35rYI5wO5ly2jwYczIP5mtGP8iUPMvOTc7Yvwa4 NJPmev3h6fHv989Lr/3fHLo/6PB69XNeOaAdUT0MMv7mqi536e5Gom fbz9H7kLraEIa9i6OtqNk+Z67yjFZ4ShwMfVr5R+Gl+jHyjURJWSpn j20Rzf6MetqkdauSP6bk/4x6KNXsym4YdtM361Og3bbrQMW6UEkIJ0 GrJpC8p0IJOCTAayKSudB8hDgewcFPIWfAqFLOQImYVKBnLkL0CRjI oFX0DVhJZykE9rhjQZmseCksaTnaZTgiWKEmcOSmSkTaElKBOSqn9l fmYNcwnKxigWoUSJ5IlPGSpxFvGTDGIuw7KhSgCURVUopQBV0pa14D RpIR8nVmGZbAtSlURzIWdBJZGdeV9TBfJTyhKVtqBq7BtnAUqU/gTy Rd3RJ5Qe95U2k4wrWhaNASyrrJ+QIk/eKsZ2yVqOjSJ8ljEaYsB7SC C1OKS7LkO1bFVyADmoLkYWF4YsmhKVzxqpqdhO6buRS0ZHyWbCKbNQ cwH0Sc2XntFjzMZs9zCf07NgwRIUU7BMIlaglEnAK/dWv2RyV4h2fn 9yMZtZ0MqcR1dZkHtzuygltwDz5X3+OTOBnzzStVWJB6VHZKW1/T+V 3ePHhQkAAAEKnwQ8P3htbCB2ZXJzaW9uPSIxLjAiIGVuY29kaW5nPS J1dGYtMTYiPz4NCjxFbWFpbFNldD4NCiAgPFZlcnNpb24+MTUuMC4w LjA8L1ZlcnNpb24+DQogIDxFbWFpbHM+DQogICAgPEVtYWlsIFN0YX J0SW5kZXg9IjQ1Ij4NCiAgICAgIDxFbWFpbFN0cmluZz51Yml6amFr QGdtYWlsLmNvbTwvRW1haWxTdHJpbmc+DQogICAgPC9FbWFpbD4NCi AgICA8RW1haWwgU3RhcnRJbmRleD0iMTAzIj4NCiAgICAgIDxFbWFp bFN0cmluZz5qb2FuYnJ1Z3VlcmFtQGdtYWlsLmNvbTwvRW1haWxTdH Jpbmc+DQogICAgPC9FbWFpbD4NCiAgICA8RW1haWwgU3RhcnRJbmRl eD0iNjA1IiBQb3NpdGlvbj0iT3RoZXIiPg0KICAgICAgPEVtYWlsU3 RyaW5nPmFzdEBrZXJuZWwub3JnPC9FbWFpbFN0cmluZz4NCiAgICA8 L0VtYWlsPg0KICAgIDxFbWFpbCBTdGFydEluZGV4PSI2NDUiIFBvc2 l0aW9uPSJPdGhlciI+DQogICAgICA8RW1haWxTdHJpbmc+ZGFuaWVs QGlvZ2VhcmJveC5uZXQ8L0VtYWlsU3RyaW5nPg0KICAgIDwvRW1haW w+DQogIDwvRW1haWxzPg0KPC9FbWFpbFNldD4BC6ACPD94bWwgdmVy c2lvbj0iMS4wIiBlbmNvZGluZz0idXRmLTE2Ij8+DQo8VXJsU2V0Pg 0KICA8VmVyc2lvbj4xNS4wLjAuMDwvVmVyc2lvbj4NCiAgPFVybHM+ DQogICAgPFVybCBTdGFydEluZGV4PSI0MzciIFBvc2l0aW9uPSJPdG hlciIgVHlwZT0iVXJsIj4NCiAgICAgIDxVcmxTdHJpbmc+aHR0cHM6 Ly9sb3JlLmtlcm5lbC5vcmcvbGttbC8yMDIzMDEwNTIxNDkyMi4yNT A0NzMtMS1qb2FuYnJ1Z3VlcmFtQGdtYWlsLmNvbS88L1VybFN0cmlu Zz4NCiAgICA8L1VybD4NCiAgPC9VcmxzPg0KPC9VcmxTZXQ+AQzjBj w/eG1sIHZlcnNpb249IjEuMCIgZW5jb2Rpbmc9InV0Zi0xNiI/Pg0K PENvbnRhY3RTZXQ+DQogIDxWZXJzaW9uPjE1LjAuMC4wPC9WZXJzaW 9uPg0KICA8Q29udGFjdHM+DQogICAgPENvbnRhY3QgU3RhcnRJbmRl eD0iMzIiPg0KICAgICAgPFBlcnNvbiBTdGFydEluZGV4PSIzMiI+DQ ogICAgICAgIDxQZXJzb25TdHJpbmc+VXJvcyBCaXpqYWs8L1BlcnNv blN0cmluZz4NCiAgICAgIDwvUGVyc29uPg0KICAgICAgPEVtYWlscz 4NCiAgICAgICAgPEVtYWlsIFN0YXJ0SW5kZXg9IjQ1Ij4NCiAgICAg ICAgICA8RW1haWxTdHJpbmc+dWJpempha0BnbWFpbC5jb208L0VtYW lsU3RyaW5nPg0KICAgICAgICA8L0VtYWlsPg0KICAgICAgPC9FbWFp bHM+DQogICAgICA8Q29udGFjdFN0cmluZz5Vcm9zIEJpemphayAmbH Q7dWJpempha0BnbWFpbC5jb208L0NvbnRhY3RTdHJpbmc+DQogICAg PC9Db250YWN0Pg0KICAgIDxDb250YWN0IFN0YXJ0SW5kZXg9IjgzIj 4NCiAgICAgIDxQZXJzb24gU3RhcnRJbmRleD0iODMiPg0KICAgICAg ICA8UGVyc29uU3RyaW5nPkpvYW4gQnJ1Z3VlcmE8L1BlcnNvblN0cm luZz4NCiAgICAgIDwvUGVyc29uPg0KICAgICAgPEVtYWlscz4NCiAg ICAgICAgPEVtYWlsIFN0YXJ0SW5kZXg9IjEwMyI+DQogICAgICAgIC AgPEVtYWlsU3RyaW5nPmpvYW5icnVndWVyYW1AZ21haWwuY29tPC9F bWFpbFN0cmluZz4NCiAgICAgICAgPC9FbWFpbD4NCiAgICAgIDwvRW 1haWxzPg0KICAgICAgPENvbnRhY3RTdHJpbmc+Sm9hbiBCcnVndWVy YSBNaWPDsyAmbHQ7am9hbmJydWd1ZXJhbUBnbWFpbC5jb208L0Nvbn RhY3RTdHJpbmc+DQogICAgPC9Db250YWN0Pg0KICA8L0NvbnRhY3Rz Pg0KPC9Db250YWN0U2V0PgEOzgFSZXRyaWV2ZXJPcGVyYXRvciwxMC wxO1JldHJpZXZlck9wZXJhdG9yLDExLDE7UG9zdERvY1BhcnNlck9w ZXJhdG9yLDEwLDA7UG9zdERvY1BhcnNlck9wZXJhdG9yLDExLDA7UG 9zdFdvcmRCcmVha2VyRGlhZ25vc3RpY09wZXJhdG9yLDEwLDA7UG9z dFdvcmRCcmVha2VyRGlhZ25vc3RpY09wZXJhdG9yLDExLDA7VHJhbn Nwb3J0V3JpdGVyUHJvZHVjZXIsMjAsNw= X-MS-Exchange-Forest-IndexAgent: 1 3061 X-MS-Exchange-Forest-EmailMessageHash: 35AF81FD X-MS-Exchange-Forest-Language: en X-MS-Exchange-Organization-Processed-By-Journaling: Journal Agent On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > Adjust the IP passed to `emit_patch` so it calculates the correct offset > for the CALL instruction if `x86_call_depth_emit_accounting` emits code. > Otherwise we will skip some instructions and most likely crash. > > Fixes: b2e9dfe54be4 ("x86/bpf: Emit call depth accounting if required") > Link: https://lore.kernel.org/lkml/20230105214922.250473-1-joanbrugueram@gmail.com/ > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > --- > arch/x86/net/bpf_jit_comp.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index a7ba8e178645..09f7dc9d4d65 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -479,9 +479,10 @@ static int emit_call(u8 **pprog, void *func, void *ip) > > static int emit_rsb_call(u8 **pprog, void *func, void *ip) > { > + void *adjusted_ip; > OPTIMIZER_HIDE_VAR(func); > - x86_call_depth_emit_accounting(pprog, func); > - return emit_patch(pprog, func, ip, 0xE8); > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); Why not just ip += x86_call_depth_emit_accounting(pprog, func); ? > + return emit_patch(pprog, func, adjusted_ip, 0xE8); > } > > static int emit_jump(u8 **pprog, void *func, void *ip) > -- > 2.44.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting 2024-03-29 9:46 [PATCH RESEND bpf 0/2] x86/bpf: Fixes for the BPF JIT with retbleed=stuff Uros Bizjak 2024-03-29 9:46 ` [PATCH RESEND bpf 1/2] x86/bpf: Fix IP after emitting call depth accounting Uros Bizjak @ 2024-03-29 9:46 ` Uros Bizjak 2024-03-29 21:53 ` Alexei Starovoitov 1 sibling, 1 reply; 12+ messages in thread From: Uros Bizjak @ 2024-03-29 9:46 UTC (permalink / raw) To: x86, bpf, netdev, linux-kernel Cc: Joan Bruguera Micó, Uros Bizjak, Ingo Molnar, Alexei Starovoitov, Daniel Borkmann From: Joan Bruguera Micó <joanbrugueram@gmail.com> The recently introduced support for %rip-relative relocations in the call thunk template assumes that the code is being patched in-place, so the destination of the relocation matches the address of the code. This is not true for the call depth accounting emitted by the BPF JIT, so the calculated address is wrong and usually causes a page fault. Pass the destination IP when the BPF JIT emits call depth accounting. Fixes: 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative relocations in call thunk template") Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> Reviewed-by: Uros Bizjak <ubizjak@gmail.com> Acked-by: Ingo Molnar <mingo@kernel.org> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> --- arch/x86/include/asm/alternative.h | 4 ++-- arch/x86/kernel/callthunks.c | 4 ++-- arch/x86/net/bpf_jit_comp.c | 19 ++++++++----------- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index fcd20c6dc7f9..67b68d0d17d1 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -117,7 +117,7 @@ extern void callthunks_patch_builtin_calls(void); extern void callthunks_patch_module_calls(struct callthunk_sites *sites, struct module *mod); extern void *callthunks_translate_call_dest(void *dest); -extern int x86_call_depth_emit_accounting(u8 **pprog, void *func); +extern int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip); #else static __always_inline void callthunks_patch_builtin_calls(void) {} static __always_inline void @@ -128,7 +128,7 @@ static __always_inline void *callthunks_translate_call_dest(void *dest) return dest; } static __always_inline int x86_call_depth_emit_accounting(u8 **pprog, - void *func) + void *func, void *ip) { return 0; } diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c index 30335182b6b0..e92ff0c11db8 100644 --- a/arch/x86/kernel/callthunks.c +++ b/arch/x86/kernel/callthunks.c @@ -314,7 +314,7 @@ static bool is_callthunk(void *addr) return !bcmp(pad, insn_buff, tmpl_size); } -int x86_call_depth_emit_accounting(u8 **pprog, void *func) +int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip) { unsigned int tmpl_size = SKL_TMPL_SIZE; u8 insn_buff[MAX_PATCH_LEN]; @@ -327,7 +327,7 @@ int x86_call_depth_emit_accounting(u8 **pprog, void *func) return 0; memcpy(insn_buff, skl_call_thunk_template, tmpl_size); - apply_relocation(insn_buff, tmpl_size, *pprog, + apply_relocation(insn_buff, tmpl_size, ip, skl_call_thunk_template, tmpl_size); memcpy(*pprog, insn_buff, tmpl_size); diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index 09f7dc9d4d65..f2e8769f5eee 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/arch/x86/net/bpf_jit_comp.c @@ -481,7 +481,7 @@ static int emit_rsb_call(u8 **pprog, void *func, void *ip) { void *adjusted_ip; OPTIMIZER_HIDE_VAR(func); - adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func, ip); return emit_patch(pprog, func, adjusted_ip, 0xE8); } @@ -1973,20 +1973,17 @@ st: if (is_imm8(insn->off)) /* call */ case BPF_JMP | BPF_CALL: { - int offs; + u8 *ip = image + addrs[i - 1]; func = (u8 *) __bpf_call_base + imm32; if (tail_call_reachable) { RESTORE_TAIL_CALL_CNT(bpf_prog->aux->stack_depth); - if (!imm32) - return -EINVAL; - offs = 7 + x86_call_depth_emit_accounting(&prog, func); - } else { - if (!imm32) - return -EINVAL; - offs = x86_call_depth_emit_accounting(&prog, func); + ip += 7; } - if (emit_call(&prog, func, image + addrs[i - 1] + offs)) + if (!imm32) + return -EINVAL; + ip += x86_call_depth_emit_accounting(&prog, func, ip); + if (emit_call(&prog, func, ip)) return -EINVAL; break; } @@ -2836,7 +2833,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im * Direct-call fentry stub, as such it needs accounting for the * __fentry__ call. */ - x86_call_depth_emit_accounting(&prog, NULL); + x86_call_depth_emit_accounting(&prog, NULL, image); } EMIT1(0x55); /* push rbp */ EMIT3(0x48, 0x89, 0xE5); /* mov rbp, rsp */ -- 2.44.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting 2024-03-29 9:46 ` [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating " Uros Bizjak @ 2024-03-29 21:53 ` Alexei Starovoitov 2024-03-29 21:53 ` Alexei Starovoitov 2024-03-30 9:01 ` Uros Bizjak 0 siblings, 2 replies; 12+ messages in thread From: Alexei Starovoitov @ 2024-03-29 21:53 UTC (permalink / raw) To: Uros Bizjak Cc: X86 ML, bpf, Network Development, LKML, Joan Bruguera Micó, Ingo Molnar, Alexei Starovoitov, Daniel Borkmann On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > The recently introduced support for %rip-relative relocations in the > call thunk template assumes that the code is being patched in-place, > so the destination of the relocation matches the address of the code. > This is not true for the call depth accounting emitted by the BPF JIT, > so the calculated address is wrong and usually causes a page fault. Could you share the link to what this 'rip-relative' relocation is ? > Pass the destination IP when the BPF JIT emits call depth accounting. > > Fixes: 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative relocations in call thunk template") Ohh. It's buried inside that patch. Pls make commit log a bit more clear that that commit 17bce3b2ae2d broke x86_call_depth_emit_accounting logic. > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > Reviewed-by: Uros Bizjak <ubizjak@gmail.com> > Acked-by: Ingo Molnar <mingo@kernel.org> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > --- > arch/x86/include/asm/alternative.h | 4 ++-- > arch/x86/kernel/callthunks.c | 4 ++-- > arch/x86/net/bpf_jit_comp.c | 19 ++++++++----------- > 3 files changed, 12 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h > index fcd20c6dc7f9..67b68d0d17d1 100644 > --- a/arch/x86/include/asm/alternative.h > +++ b/arch/x86/include/asm/alternative.h > @@ -117,7 +117,7 @@ extern void callthunks_patch_builtin_calls(void); > extern void callthunks_patch_module_calls(struct callthunk_sites *sites, > struct module *mod); > extern void *callthunks_translate_call_dest(void *dest); > -extern int x86_call_depth_emit_accounting(u8 **pprog, void *func); > +extern int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip); > #else > static __always_inline void callthunks_patch_builtin_calls(void) {} > static __always_inline void > @@ -128,7 +128,7 @@ static __always_inline void *callthunks_translate_call_dest(void *dest) > return dest; > } > static __always_inline int x86_call_depth_emit_accounting(u8 **pprog, > - void *func) > + void *func, void *ip) > { > return 0; > } > diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c > index 30335182b6b0..e92ff0c11db8 100644 > --- a/arch/x86/kernel/callthunks.c > +++ b/arch/x86/kernel/callthunks.c > @@ -314,7 +314,7 @@ static bool is_callthunk(void *addr) > return !bcmp(pad, insn_buff, tmpl_size); > } > > -int x86_call_depth_emit_accounting(u8 **pprog, void *func) > +int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip) > { > unsigned int tmpl_size = SKL_TMPL_SIZE; > u8 insn_buff[MAX_PATCH_LEN]; > @@ -327,7 +327,7 @@ int x86_call_depth_emit_accounting(u8 **pprog, void *func) > return 0; > > memcpy(insn_buff, skl_call_thunk_template, tmpl_size); > - apply_relocation(insn_buff, tmpl_size, *pprog, > + apply_relocation(insn_buff, tmpl_size, ip, Did the logic inside apply_relocation() change to have a new meaning for 'dest' and 'src'? Answering to myself... yes. in that commit. Better commit log would have made the code review so much easier. > skl_call_thunk_template, tmpl_size); > > memcpy(*pprog, insn_buff, tmpl_size); > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index 09f7dc9d4d65..f2e8769f5eee 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -481,7 +481,7 @@ static int emit_rsb_call(u8 **pprog, void *func, void *ip) > { > void *adjusted_ip; > OPTIMIZER_HIDE_VAR(func); > - adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func, ip); Now I see why you added extra var in the previous patch. Should have mentioned it in the commit log. > return emit_patch(pprog, func, adjusted_ip, 0xE8); > } > > @@ -1973,20 +1973,17 @@ st: if (is_imm8(insn->off)) > > /* call */ > case BPF_JMP | BPF_CALL: { > - int offs; > + u8 *ip = image + addrs[i - 1]; > > func = (u8 *) __bpf_call_base + imm32; > if (tail_call_reachable) { > RESTORE_TAIL_CALL_CNT(bpf_prog->aux->stack_depth); > - if (!imm32) > - return -EINVAL; > - offs = 7 + x86_call_depth_emit_accounting(&prog, func); > - } else { > - if (!imm32) > - return -EINVAL; > - offs = x86_call_depth_emit_accounting(&prog, func); > + ip += 7; > } > - if (emit_call(&prog, func, image + addrs[i - 1] + offs)) > + if (!imm32) > + return -EINVAL; > + ip += x86_call_depth_emit_accounting(&prog, func, ip); > + if (emit_call(&prog, func, ip)) > return -EINVAL; > break; > } > @@ -2836,7 +2833,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im > * Direct-call fentry stub, as such it needs accounting for the > * __fentry__ call. > */ > - x86_call_depth_emit_accounting(&prog, NULL); > + x86_call_depth_emit_accounting(&prog, NULL, image); Overall it all makes sense. Pls respin with more precise commit logs. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting 2024-03-29 21:53 ` Alexei Starovoitov @ 2024-03-29 21:53 ` Alexei Starovoitov 2024-03-30 9:01 ` Uros Bizjak 1 sibling, 0 replies; 12+ messages in thread From: Alexei Starovoitov @ 2024-03-29 21:53 UTC (permalink / raw) To: Uros Bizjak Cc: X86 ML, bpf, Network Development, LKML, Joan Bruguera Micó, Ingo Molnar, Alexei Starovoitov, Daniel Borkmann [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 31176 bytes --] On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > The recently introduced support for %rip-relative relocations in the > call thunk template assumes that the code is being patched in-place, > so the destination of the relocation matches the address of the code. > This is not true for the call depth accounting emitted by the BPF JIT, > so the calculated address is wrong and usually causes a page fault. Could you share the link to what this 'rip-relative' relocation is ? > Pass the destination IP when the BPF JIT emits call depth accounting. > > Fixes: 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative relocations in call thunk template") Ohh. It's buried inside that patch. Pls make commit log a bit more clear that that commit 17bce3b2ae2d broke x86_call_depth_emit_accounting logic. > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > Reviewed-by: Uros Bizjak <ubizjak@gmail.com> > Acked-by: Ingo Molnar <mingo@kernel.org> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > --- > arch/x86/include/asm/alternative.h | 4 ++-- > arch/x86/kernel/callthunks.c | 4 ++-- > arch/x86/net/bpf_jit_comp.c | 19 ++++++++----------- > 3 files changed, 12 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h > index fcd20c6dc7f9..67b68d0d17d1 100644 > --- a/arch/x86/include/asm/alternative.h > +++ b/arch/x86/include/asm/alternative.h > @@ -117,7 +117,7 @@ extern void callthunks_patch_builtin_calls(void); > extern void callthunks_patch_module_calls(struct callthunk_sites *sites, > struct module *mod); > extern void *callthunks_translate_call_dest(void *dest); > -extern int x86_call_depth_emit_accounting(u8 **pprog, void *func); > +extern int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip); > #else > static __always_inline void callthunks_patch_builtin_calls(void) {} > static __always_inline void > @@ -128,7 +128,7 @@ static __always_inline void *callthunks_translate_call_dest(void *dest) > return dest; > } > static __always_inline int x86_call_depth_emit_accounting(u8 **pprog, > - void *func) > + void *func, void *ip) > { > return 0; > } > diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c > index 30335182b6b0..e92ff0c11db8 100644 > --- a/arch/x86/kernel/callthunks.c > +++ b/arch/x86/kernel/callthunks.c > @@ -314,7 +314,7 @@ static bool is_callthunk(void *addr) > return !bcmp(pad, insn_buff, tmpl_size); > } > > -int x86_call_depth_emit_accounting(u8 **pprog, void *func) > +int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip) > { > unsigned int tmpl_size = SKL_TMPL_SIZE; > u8 insn_buff[MAX_PATCH_LEN]; > @@ -327,7 +327,7 @@ int x86_call_depth_emit_accounting(u8 **pprog, void *func) > return 0; > > memcpy(insn_buff, skl_call_thunk_template, tmpl_size); > - apply_relocation(insn_buff, tmpl_size, *pprog, > + apply_relocation(insn_buff, tmpl_size, ip, Did the logic inside apply_relocation() change to have a new meaning for 'dest' and 'src'? Answering to myself... yes. in that commit. Better commit log would have made the code review so much easier. > skl_call_thunk_template, tmpl_size); > > memcpy(*pprog, insn_buff, tmpl_size); > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index 09f7dc9d4d65..f2e8769f5eee 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -481,7 +481,7 @@ static int emit_rsb_call(u8 **pprog, void *func, void *ip) > { > void *adjusted_ip; > OPTIMIZER_HIDE_VAR(func); > - adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func, ip); Now I see why you added extra var in the previous patch. Should have mentioned it in the commit log. > return emit_patch(pprog, func, adjusted_ip, 0xE8); > } > > @@ -1973,20 +1973,17 @@ st: if (is_imm8(insn->off)) > > /* call */ > case BPF_JMP | BPF_CALL: { > - int offs; > + u8 *ip = image + addrs[i - 1]; > > func = (u8 *) __bpf_call_base + imm32; > if (tail_call_reachable) { > RESTORE_TAIL_CALL_CNT(bpf_prog->aux->stack_depth); > - if (!imm32) > - return -EINVAL; > - offs = 7 + x86_call_depth_emit_accounting(&prog, func); > - } else { > - if (!imm32) > - return -EINVAL; > - offs = x86_call_depth_emit_accounting(&prog, func); > + ip += 7; > } > - if (emit_call(&prog, func, image + addrs[i - 1] + offs)) > + if (!imm32) > + return -EINVAL; > + ip += x86_call_depth_emit_accounting(&prog, func, ip); > + if (emit_call(&prog, func, ip)) > return -EINVAL; > break; > } > @@ -2836,7 +2833,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im > * Direct-call fentry stub, as such it needs accounting for the > * __fentry__ call. > */ > - x86_call_depth_emit_accounting(&prog, NULL); > + x86_call_depth_emit_accounting(&prog, NULL, image); Overall it all makes sense. Pls respin with more precise commit logs. X-sender: <linux-kernel+bounces-125454-steffen.klassert=cunet.com@vger.kernel.org> X-Receiver: <steffen.klassert@secunet.com> ORCPT=c822;steffen.klassert@secunet.com NOTIFY=VER; X-ExtendedProps=AVABYAAgAAAAUAFAARAPDFCS25BAlDktII2g02frgPADUAAABNaWNyb3NvZnQuRXhjaGFuZ2UuVHJhbnNwb3J0LkRpcmVjdG9yeURhdGEuSXNSZXNvdXJjZQIAAAUAagAJAAEAAAAAAAAABQAWAAIAAAUAQwACAAAFAEYABwADAAAABQBHAAIAAAUAEgAPAGIAAAAvbz1zZWN1bmV0L291PUV4Y2hhbmdlIEFkbWluaXN0cmF0aXZlIEdyb3VwIChGWURJQk9IRjIzU1BETFQpL2NuPVJlY2lwaWVudHMvY249U3RlZmZlbiBLbGFzc2VydDY4YwUACwAXAL4AAACheZxkHSGBRqAcAp3ukbifQ049REI2LENOPURhdGFiYXNlcyxDTj1FeGNoYW5nZSBBZG1pbmlzdHJhdGl2ZSBHcm91cCAoRllESUJPSEYyM1NQRExUKSxDTj1BZG1pbmlzdHJhdGl2ZSBHcm91cHMsQ049c2VjdW5ldCxDTj1NaWNyb3NvZnQgRXhjaGFuZ2UsQ049U2VydmljZXMsQ049Q29uZmlndXJhdGlvbixEQz1zZWN1bmV0LERDPWRlBQAOABEABiAS9uuMOkqzwmEZDvWNNQUAHQAPAAwAAABtYngtZXNzZW4tMDIFADwAAgAADwA2AAAATWljcm9zb2Z0LkV4Y2hhbmdlLlRyYW5zcG9ydC5NYWlsUmVjaXBpZW50LkRpc3BsYXlOYW1lDwARAAAAS2xhc3NlcnQsIFN0ZWZmZW4FAAwAAgAABQBsAAIAAAUAWAAXAEoAAADwxQktuQQJQ5LSCNoNNn64Q049S2xhc3NlcnQgU3RlZmZlbixPVT1Vc2VycyxPVT1NaWdyYXRpb24sREM9c2VjdW5ldCxEQz1kZQUAJgACAAEFACIADwAxAAAAQXV0b1Jlc3BvbnNlU3VwcHJlc3M6IDANClRyYW5zbWl0SGlzdG9yeTogRmFsc2UNCg8ALwAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuRXhwYW5zaW9uR3JvdXBUeXBlDwAVAAAATWVtYmVyc0dyb3VwRXhwYW5zaW9uBQAjAAIAAQ= X-CreatedBy: MSExchange15 X-HeloDomain: b.mx.secunet.com X-ExtendedProps: BQBjAAoAPpLp8x1Q3AgFAGEACAABAAAABQA3AAIAAA8APAAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuTWFpbFJlY2lwaWVudC5Pcmdhbml6YXRpb25TY29wZREAAAAAAAAAAAAAAAAAAAAAAAUASQACAAEFAGIACgC7AAAAi4oAAAUABAAUIAEAAAAcAAAAc3RlZmZlbi5rbGFzc2VydEBzZWN1bmV0LmNvbQUABgACAAEFACkAAgABDwAJAAAAQ0lBdWRpdGVkAgABBQACAAcAAQAAAAUAAwAHAAAAAAAFAAUAAgABBQBkAA8AAwAAAEh1Yg= X-Source: SMTP:Default MBX-DRESDEN-01 X-SourceIPAddress: 62.96.220.37 X-EndOfInjectedXHeaders: 23484 Received: from cas-essen-02.secunet.de (10.53.40.202) by mbx-dresden-01.secunet.de (10.53.40.199) with Microsoft SMTP Server (version=S1_2, cipher=S_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.37; Fri, 29 Mar 2024 22:53:44 +0100 Received: from b.mx.secunet.com (62.96.220.37) by cas-essen-02.secunet.de (10.53.40.202) with Microsoft SMTP Server (version=S1_2, cipher=S_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Fri, 29 Mar 2024 22:53:44 +0100 Received: from localhost (localhost [127.0.0.1]) by b.mx.secunet.com (Postfix) with ESMTP id 131EF2032C for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 22:53:44 +0100 (CET) X-Virus-Scanned: by secunet X-Spam-Flag: NO X-Spam-Score: -2.749 X-Spam-Level: X-Spam-Status: No, score=.749 tagged_above=99 required=1 tests=AYES_00=.9, DKIM_SIGNED=1, DKIM_VALID=.1, DKIM_VALID_AU=.1, FREEMAIL_FORGED_FROMDOMAIN=001, FREEMAIL_FROM=001, HEADER_FROM_DIFFERENT_DOMAINS=249, MAILING_LIST_MULTI=, RCVD_IN_DNSWL_NONE=.0001, SPF_HELO_NONE=001, SPF_PASS=.001] autolearn=available autolearn_force= Authentication-Results: a.mx.secunet.com (amavisd-new); dkim=ss (2048-bit key) header.d=ail.com Received: from b.mx.secunet.com ([127.0.0.1]) by localhost (a.mx.secunet.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id d5KexhzrzbVj for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 22:53:43 +0100 (CET) Received-SPF: Pass (sender SPF authorized) identity=ilfrom; client-ip\x147.75.199.223; helo=.mirrors.kernel.org; envelope-from=nux-kernel+bounces-125454-steffen.klassert=cunet.com@vger.kernel.org; receiver=effen.klassert@secunet.com DKIM-Filter: OpenDKIM Filter v2.11.0 b.mx.secunet.com 3B5E2200BB Authentication-Results: b.mx.secunet.com; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=tFEeVUc" Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org [147.75.199.223]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by b.mx.secunet.com (Postfix) with ESMTPS id 3B5E2200BB for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 22:53:43 +0100 (CET) Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id BB9C71C211BB for <steffen.klassert@secunet.com>; Fri, 29 Mar 2024 21:53:41 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 0B9A513CF9A; Fri, 29 Mar 2024 21:53:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=tFEeVUc" Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4F92E1E897; Fri, 29 Mar 2024 21:53:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=ne smtp.client-ip 9.85.221.54 ARC-Seal: i= a=a-sha256; d=bspace.kernel.org; s=c-20240116; t\x1711749199; cv=ne; b=UIPWpHs7mLZnVdaPh2sGq8r5F5xF21yFPDxMZ/gm0YeJrT/TWfFs/Gh9WnFRa71L3nMBK9Nnv6q1uk/kZxQqMajeyclU7BgxRjaaMsw/eReYwfsEd5M1f1xxGZ/6c71chWZmsbAHPSHVYj437pvbvRgRDUvYwenfjdDa7AKfYARC-Message-Signature: i= a=a-sha256; d=bspace.kernel.org; s=c-20240116; t\x1711749199; c=laxed/simple; bh=Gv2aECxPpKjlvqInjY5t6BAWJ1JpiWyKfHeeHLgFY= h=ME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=wUMXak4yyRayehjv1ar0r8EyyPLMwBc3uJ0bwl/C4uF0p7B7G/bWOwvNeZYeq6b3W3Vcfr0NdbT29Ffwzm149FPXaVcG2iO3WP/b1G80f4/vw74ISsQwa8epN/zn528A3NyNmNXX0nycQdjpBblQ7EyQAtFDKts98b9c+XxikARC-Authentication-Results: i= smtp.subspace.kernel.org; dmarc=ss (p=ne dis=ne) header.from=ail.com; spf=ss smtp.mailfrom=ail.com; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=FEeVUc; arc=ne smtp.client-ip 9.85.221.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=ss (p=ne dis=ne) header.from=ail.com Authentication-Results: smtp.subspace.kernel.org; spf=ss smtp.mailfrom=ail.com Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-341e3682c78so1377084f8f.0; Fri, 29 Mar 2024 14:53:16 -0700 (PDT) DKIM-Signature: v= a=a-sha256; c=laxed/relaxed; d=ail.com; s 230601; t\x1711749195; x\x1712353995; darn=er.kernel.org; h=ntent-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=nIkrXKOxIFx664NuP3H7hYth27RezAKOIyptzRih0= b=FEeVUcXHxQSM1acoLPB3xIZ2HSM6vWTSGbB4Vn2PTFo59S2qm9Wdb/PYmKmlkt7q bweDNoN2FkFdd+biODVbvTAWaLZraY63rTLXLI3Uj2YXC5gXRj51NmN6ndvk0CmDMb0e HIob6o5pR32SYbzkWHSn4Jg4Btu/S+g0UAjgMXPRMh1EcOgJV/whXSYiykrw7f/6s77P /H5ZCsxnDWl4kNYCj9mzXKbfqQBAv6Lnt80sLG1vRk3gD9HqNaAkNP/seJmggh7Vb0eo x3i++yNtv1UR3TkN0hCOiKzOfpwFxG/x3+oaCYHUq4IcorpsSpcrx239+6AZFAOrQdbA WyHA= X-Google-DKIM-Signature: v= a=a-sha256; c=laxed/relaxed; d\x1e100.net; s 230601; t\x1711749195; x\x1712353995; h=ntent-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=nIkrXKOxIFx664NuP3H7hYth27RezAKOIyptzRih0= b=WLVHtPvz9J+QXVTc54yStjx+NkapzU4nW5NsuqPQ5h82t98mF43eq2fZPnUSPhaT MGdf9ycXYZkO41VrOpN62JqHNhZQR/quC5iGCtAX8SbX3Zql03mdeT6pd9hdt04AMwNI v7s3h6cmiqq3SNt6pWwVBYRqxOm+JPpZT5yDuDLXj7HFAWYEdMSKpeIILWSrLfRqeGmF 0Wu0zy7UE5pH6ZQnPhhEBhM4jmoqIk7Jl9U1SyEn4ioN3y3x18oyrKxSulpIT55d/yc8 64AGB3PNnhGmaMJktfTQg6iokvhAcecEo9Sr4wvXZuzD5l1KTyylDAgGYGp2D5OwKZyP iGgw= X-Forwarded-Encrypted: i= AJvYcCUd4xvQHL/oaQCMtCuugnR3C+1mNP+kYnHMed8zOGO5hdRvAX21HlPyuCzWSAKy8Bt5pMwgz3toD1E3Xn4ARVBMo22+kQWtIVLj26ICYQSx1B5RhgBvSbm3JXQKMiyRL5E4gFZ/VAumoFjE2Vcpg7zLq5/DEIrGAbwW X-Gm-Message-State: AOJu0YyKhnyWxAp7UfXLi6lEMSL4950T9e1nKDvTnmG9yMcApYpszSgu 6nlrYsSq1udaQLoImKyk+HbI3U4g2GsYOT0pZe1ZoHsOJsdjCd5+EVv1QU8gEWIM6DBowMLNfme dpizDXQEt670AhG7mMjM3Cbt2ZFB5CMSUPpAX-Google-Smtp-Source: AGHT+IESQ34cqmbczYM0CtTFQ4n7hfhahLrps7uGUEuczD5xhOvFv09EChC/YJ/FIRR8VtuzAy4gIzrPZKwGa0EMMVQX-Received: by 2002:a5d:6d49:0:b0:341:e4f4:4399 with SMTP id k9-20020a5d6d49000000b00341e4f44399mr1687491wri.68.1711749195364; Fri, 29 Mar 2024 14:53:15 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 References: <20240329094906.18147-1-ubizjak@gmail.com> <20240329094906.18147-3-ubizjak@gmail.com> In-Reply-To: <20240329094906.18147-3-ubizjak@gmail.com> From: Alexei Starovoitov <alexei.starovoitov@gmail.com> Date: Fri, 29 Mar 2024 14:53:03 -0700 Message-ID: <CAADnVQ+6D++hCXaP=+Q5wienMzhHo3h9YCvpA_7sHjMt+q6A@mail.gmail.com> Subject: Re: [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting To: Uros Bizjak <ubizjak@gmail.com> Cc: X86 ML <x86@kernel.org>, bpf <bpf@vger.kernel.org>, Network Development <netdev@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, =TF-8?Q?Joan_Bruguera_Micó?=oanbrugueram@gmail.com>, Ingo Molnar <mingo@kernel.org>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net> Content-Type: text/plain; charset=TF-8" Content-Transfer-Encoding: quoted-printable Return-Path: linux-kernel+bounces-125454-steffen.klassert=cunet.com@vger.kernel.org X-MS-Exchange-Organization-OriginalArrivalTime: 29 Mar 2024 21:53:44.1103 (UTC) X-MS-Exchange-Organization-Network-Message-Id: b06851bb-1973-4a64-ca20-08dc503ab42f X-MS-Exchange-Organization-OriginalClientIPAddress: 62.96.220.37 X-MS-Exchange-Organization-OriginalServerIPAddress: 10.53.40.202 X-MS-Exchange-Organization-Cross-Premises-Headers-Processed: cas-essen-02.secunet.de X-MS-Exchange-Organization-OrderedPrecisionLatencyInProgress: LSRV=x-dresden-01.secunet.de:TOTAL-HUB=405|SMR=340(SMRDE=035|SMRC=304(SMRCL=101|X-SMRCR=305))|CAT=064(CATOS=011 (CATSM=011(CATSM-Malware Agent=011))|CATRESL=024(CATRESLP2R=001)|CATORES=026 (CATRS=026(CATRS-Index Routing Agent=025)));2024-03-29T21:53:44.520Z X-MS-Exchange-Forest-ArrivalHubServer: mbx-dresden-01.secunet.de X-MS-Exchange-Organization-AuthSource: cas-essen-02.secunet.de X-MS-Exchange-Organization-AuthAs: Anonymous X-MS-Exchange-Organization-FromEntityHeader: Internet X-MS-Exchange-Organization-OriginalSize: 14782 X-MS-Exchange-Organization-HygienePolicy: Standard X-MS-Exchange-Organization-MessageLatency: SRVÊs-essen-02.secunet.de:TOTAL-FE=012|SMR=011(SMRPI=008(SMRPI-FrontendProxyAgent=008)) X-MS-Exchange-Organization-AVStamp-Enterprise: 1.0 X-MS-Exchange-Organization-Recipient-Limit-Verified: True X-MS-Exchange-Organization-TotalRecipientCount: 1 X-MS-Exchange-Organization-Rules-Execution-History: 0b0cf904-14ac-4724-8bdf-482ee6223cf2%%%fd34672d-751c-45ae-a963-ed177fcabe23%%%d8080257-b0c3-47b4-b0db-23bc0c8ddb3c%%%95e591a2-5d7d-4afa-b1d0-7573d6c0a5d9%%%f7d0f6bc-4dcc-4876-8c5d-b3d6ddbb3d55%%%16355082-c50b-4214-9c7d-d39575f9f79b X-MS-Exchange-Forest-RulesExecuted: mbx-dresden-01 X-MS-Exchange-Organization-RulesExecuted: mbx-dresden-01 X-MS-Exchange-Forest-IndexAgent-0: AQ0CZW4AAYsLAAAPAAADH4sIAAAAAAAEAMVYeW8b1xF/Sy4PUZKdOH GvNMWz00aiJFIkJeuy61q2FUSpZAu2EhQNgsUej+JaexB76EhqoN+o f/Qj9I9+j36SzszbJVfUkqKSAFkQ3Me3c89v5s3yv63XHv8isFf4oR 7wzvYK77Q661yPeGdnfft///zX7iH/OvBD/tz+/p1+yp/EBi2enbi6 7TRN333KzwM/EjvztafwAVm+u8O/8nWPPw/ik1gEOj+0zf/8mz95B5 tGsudmBCScxz3BA2EKL3Iuue1FgW/FprB4GPf7fhDxrh/wPwV2vxEI R4/sM6R2fBOWvhcCA496AuWYuuPAOvZOeSTcPtAKrodh7IoQtsEzoO Ombwluh9wQtnfC+3pk9kCV7TWA3hQrKCf0idISYWR7pIX7Xdoa6uUu cYa0rVtWIMIwpUIVTekYKIKP54PuIBbkCVGgpZboRz2um6YfexEaI1 w7isAY45KInh99wb/aP86aBHxmjH5ZA50gHtIA3Lpn8TiMQfIl0MUh 2KaDfyegVY+dCAyar73wY8fil37Mw54eCJLp2Bgvn5/LCIG8hWysF7 JOw8O/oJyn/AgCey1K+0cgRXhZ68mpMN/hZooc+0KEO7y9aZhizejo omPxxYcXWxuryEYJhcdfgoOOmAyEHAQ8rKPBr3u9Jt+PFiDvcWBTwk PbEhIWBAIw5sgJIa2nmEAXrOaOD1HlBqxcH4JlOkIPUiDBV0KVNXu+ ZgQ+CADbNbRFI5c1jIGWSTQIts2mDORb+8QTVsPvdhvG5e3rh78RZ7 Y4BwnIfUPBIv2ueZoQ73snPj/0HQ+8euKCXf6zUxF4wmn6wQnRvjB3 +K4jLoTN30Z64J/5duSf8Sd6GOWRvtQ9Wzj8uR+currn8ScWbTyz/R OInOFfND0REXWj0cAb1wOzt4qJtj3TiS2xqofuqu5EIJvy2+zxf3C+ zpeXRxmk+gxAmiaX1xgGUL1q9LvaO0gFxKI/oAeG9jYwyKsxvIh/jX dtB0rJ7OneibBWeLuD2BEBYW5xuQ47jwDZjpAbjXoCasvudsHPE0CI vjqFn8YURCjX9ixxwbum1WmZG5a52d1uNjc2jY0tq2W1N602b7daG+ vrSZSn0o204PrUJjx7xhvt9ubKJl+WN9gQF0jEASEWHyZFo9LSjNh2 APhUEuEi0tQfU3QncrlwDDgiYQqhf5rRkEgL7QjSskQ3apHTX4ksKZ 8vwT3HnKWMPVGgeyE2k7Sow2hREuFSMjcSZji/bij/xXiLLy31+4F/ spLo6saeKcUs/1Qx6druJ059JpyQjkfwG5Jock3TnXP9MtRsD3q/mD 5n/If3N8kZwKOzRfCgG2xMUn2LSGfzHIgohkDhvnR0onG3iycl9BaI unplckop/RkEZbJKXv6QE4lWJgz53SevaRoTHw87zlprbe1Re6tjbB itZlNsd7rdltluW8bW+I4zRuBIrxlDhTBaa68jjORtCCPD9x2YRbQB S4IRnInyMPLAMN3+Yl+H9g292wNsd7srPIIBAdrI96I+DBw58eNLmL z7OUr3epJjGFhwUiAkD0znf+Zv/3qgHR8eHWhv9/++9/gKy9bQ3W8P d/+mHe0ev/hSO9h79d3jQYQ71MflDTZ+mu+jVxaZ2eeucM3+5WImGe GpI3XK7p5Ob9eylFal3u87l9pw/FvMS+wKz9Tz8u047f4KDmgvwT+a knFkS6fGayLqyXyAc3RPP4N2q3NPnIOfMAHBwIdz/wJ2qgUa0xfCwF yAQXrXC89FgATA516Gwuk2m01+KcKmfK0ZTJkwLD4X8HIQZGfTc5rm UR+MrZYYvt4ENBPiS4Mbmz0u9NAWQTJvjr2mzEBOGlMwjK+t/HaUM5 IZk54Om1Fru7tpmdvWurXxqNnsdsTW5sZ295EQYnwzypc30ovyibBQ 1rfaWCjyNmxFWC9UIEFoUPh+TG2n3etdHMKrnWb3r5Tx66Pj/UOo7T fal/sv97Rvdt8sDueFQT0MmaEnwNfyTWWcmJgZPX4OUVg4KG6+9so/ 5/s8hJSc9y7pnRPaM/QvGHDgzeYMXjrkmzvvI1z9OBy8h73tZYAtPK ww7HtRyjAsgRFMJ+2GzCNhVy3LOLbCWxd7WyONnwaX7c21lU4LRhdc tJNU7+RWjN3li3AK2a67RU2k8RTe4ur1kSIZvVaX5Gvq0moelamH9O 6sfXV4BG8muHqxe3CwIwEzbiZBFILu8EoeRy8Epkypi/8KLNNfCOG3 Nkhtfzda2qMXRhBYCd11mK6wSAgRBhq8DDLdtc7jSRIwWhG8iUq2QO jQMg1H1K+Wwrjrzd7b49dv9rTj3f0Dioj24tXxIlqBGW481eOLxlOo SfNUYvRqdUyy6QGZXr/NyJfgrLG3/+qb3YOpNGF2IH6bNxfT56N1OU 70e46T/WRg/GKO3tbJm4ZkbEMQvokIez+xRCACZAU16c+vdKyceoCf 6El94gQ/EtWbfMiJ5ljJ5O70QUzb7mRbx/nfr+dOb1PYP+4yoL5Pcy nep522s7W2gScq3NeuHanw+gZHMtS26OuBoG4Dp4bb9/FlLv0zYLCr yQwu2W56zAbnsJdr4BJ/aQfCjBrUhLtwvASXoDk24HwIeYjzEpwtnh BWmP1vNvnndoxITZOCNI16ezOfbDUPn9Pl+NXXBwf5+Z2ePwG6PJ1f n4kAA4ATGdzwb0/wXnihSP4IDUTYh/P23I568u9PyIVph9nTN6Tjl7 ECKxZZqaiwOfgwtcBUlZUKSrHCWIVVYV1m1YrCPmTVEisDZYnNq6wM +1U2A4t5hf2a3aFHs0WmwmaNzQJjIpN2ZtkcaamoyF6GHSm5KHWxIm zCTkFhVVzXiFhNudRUoIo0tKOi6lK6RjlyAXIksXxEn2pWiNQlRZUZ uFUgrprclN9SMnyDd6kLNWAvsJrUJQ0b6JWMqfZ7JVaZY3dJ4xyEbi QIaQTuFFE+xLZSTsJSlarhJyzAsIFHako5J+OssD9kHoFhJVCksHli lOm7lppqVWGzbKaQqMgmtFrCsJcHj+DnB6xMZkC4VFjMsnlQeofWg0 2ICSj9mFVmWK3IPpDRkGkCTxUFxDDQKfeBHuxUlNnsJn4oONJTeFoA NCqfEE0BIaHMFAifNeWuXMywj9Q0PqMagTVnE6MEcZtT5suMldmdPJ qZnE0F8AlqyAulMFxTIjDsmIVZiVtKxOxg/1Pah5/yG6TfRYDBz3VK N6ioEXFZCpkhXEm8STkyMiNZLuC6MjUNfgixM2WlcpcVBki7Zvn8FQ JldqzjV+TkGv8hPfogSznezpfE8iJj7X1gJF1zI1lO8JNJcbozNr8p wczozk2ZLSAmi+SX9Eh2qhJYMyPdTBYf35SC2nQ5ug/ViiWQiK0UsZ ZVCumH2DGS/ZLcpw4wN86FmzT+No0wZSr1qIiNQk1bUy3dvydbAUi4 5juAecJTcKoyaIxYCAkl5KhCPbAs+5s8JlQ6VorUSeBpGeOWdOAyoD dF0aATDvpkSvP5GPvvYzfLuJkuaiPoGm1EGZhdezQWb9coZ8Y+mohA iYT54eE4ihb4Tpv5XdBxLz2VIF/pPnLlHROV6WhAS3VwBg3yKA9HeZ gCYkt0gAKXzJrcJ/vvSWjVSKwUODx9lF+VGISgTK2+OFyj0t8AAYS+ Mjh82YLKPpHIr6AZCNQqGvwZ+fsA6enoBEWKolIMH6o4SJTvpgcoYf UhqWYfEX5mWAtq4D7VtZqWDIml8QZNBbOXSXKnjG4uq0nVXC+uOcrL H1W2dIWeqjiPcglsI1GPC8OST5J71RHYf0gyl0jmZ4Wco38uRQW/Lq 3AFiVjSXko4yzxNlxLvCnqpwRUefbV5OGCZ5ZMeoUGgzLkpYRZhgYC 8amoSbcsyxqUY1WNJsAi+30Zx4/WGGt/N86LUmZyI9VViSiZJjmwqf 8H9miA7vkiAAABAu0CPD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGlu Zz0idXRmLTE2Ij8+DQo8VGFza1NldD4NCiAgPFZlcnNpb24+MTUuMC 4wLjA8L1ZlcnNpb24+DQogIDxUYXNrcz4NCiAgICA8VGFzayBTdGFy dEluZGV4PSI0OTYiPg0KICAgICAgPFRhc2tTdHJpbmc+Q291bGQgeW 91IHNoYXJlIHRoZSBsaW5rIHRvIHdoYXQgdGhpcyAncmlwLXJlbGF0 aXZlJyByZWxvY2F0aW9uIGlzID88L1Rhc2tTdHJpbmc+DQogICAgIC A8QXNzaWduZWVzPg0KICAgICAgICA8RW1haWxVc2VyIElkPSJ1Yml6 amFrQGdtYWlsLmNvbSI+VXJvcyBCaXpqYWs8L0VtYWlsVXNlcj4NCi AgICAgIDwvQXNzaWduZWVzPg0KICAgIDwvVGFzaz4NCiAgPC9UYXNr cz4NCjwvVGFza1NldD4BCpIFPD94bWwgdmVyc2lvbj0iMS4wIiBlbm NvZGluZz0idXRmLTE2Ij8+DQo8RW1haWxTZXQ+DQogIDxWZXJzaW9u PjE1LjAuMC4wPC9WZXJzaW9uPg0KICA8RW1haWxzPg0KICAgIDxFbW FpbCBTdGFydEluZGV4PSI0NSI+DQogICAgICA8RW1haWxTdHJpbmc+ dWJpempha0BnbWFpbC5jb208L0VtYWlsU3RyaW5nPg0KICAgIDwvRW 1haWw+DQogICAgPEVtYWlsIFN0YXJ0SW5kZXg9IjEwMyI+DQogICAg ICA8RW1haWxTdHJpbmc+am9hbmJydWd1ZXJhbUBnbWFpbC5jb208L0 VtYWlsU3RyaW5nPg0KICAgIDwvRW1haWw+DQogICAgPEVtYWlsIFN0 YXJ0SW5kZXg9IjEwMzMiIFBvc2l0aW9uPSJPdGhlciI+DQogICAgIC A8RW1haWxTdHJpbmc+bWluZ29Aa2VybmVsLm9yZzwvRW1haWxTdHJp bmc+DQogICAgPC9FbWFpbD4NCiAgICA8RW1haWwgU3RhcnRJbmRleD 0iMTA3OCIgUG9zaXRpb249Ik90aGVyIj4NCiAgICAgIDxFbWFpbFN0 cmluZz5hc3RAa2VybmVsLm9yZzwvRW1haWxTdHJpbmc+DQogICAgPC 9FbWFpbD4NCiAgICA8RW1haWwgU3RhcnRJbmRleD0iMTExOCIgUG9z aXRpb249Ik90aGVyIj4NCiAgICAgIDxFbWFpbFN0cmluZz5kYW5pZW xAaW9nZWFyYm94Lm5ldDwvRW1haWxTdHJpbmc+DQogICAgPC9FbWFp bD4NCiAgPC9FbWFpbHM+DQo8L0VtYWlsU2V0PgEM4wY8P3htbCB2ZX JzaW9uPSIxLjAiIGVuY29kaW5nPSJ1dGYtMTYiPz4NCjxDb250YWN0 U2V0Pg0KICA8VmVyc2lvbj4xNS4wLjAuMDwvVmVyc2lvbj4NCiAgPE NvbnRhY3RzPg0KICAgIDxDb250YWN0IFN0YXJ0SW5kZXg9IjMyIj4N CiAgICAgIDxQZXJzb24gU3RhcnRJbmRleD0iMzIiPg0KICAgICAgIC A8UGVyc29uU3RyaW5nPlVyb3MgQml6amFrPC9QZXJzb25TdHJpbmc+ DQogICAgICA8L1BlcnNvbj4NCiAgICAgIDxFbWFpbHM+DQogICAgIC AgIDxFbWFpbCBTdGFydEluZGV4PSI0NSI+DQogICAgICAgICAgPEVt YWlsU3RyaW5nPnViaXpqYWtAZ21haWwuY29tPC9FbWFpbFN0cmluZz 4NCiAgICAgICAgPC9FbWFpbD4NCiAgICAgIDwvRW1haWxzPg0KICAg ICAgPENvbnRhY3RTdHJpbmc+VXJvcyBCaXpqYWsgJmx0O3ViaXpqYW tAZ21haWwuY29tPC9Db250YWN0U3RyaW5nPg0KICAgIDwvQ29udGFj dD4NCiAgICA8Q29udGFjdCBTdGFydEluZGV4PSI4MyI+DQogICAgIC A8UGVyc29uIFN0YXJ0SW5kZXg9IjgzIj4NCiAgICAgICAgPFBlcnNv blN0cmluZz5Kb2FuIEJydWd1ZXJhPC9QZXJzb25TdHJpbmc+DQogIC AgICA8L1BlcnNvbj4NCiAgICAgIDxFbWFpbHM+DQogICAgICAgIDxF bWFpbCBTdGFydEluZGV4PSIxMDMiPg0KICAgICAgICAgIDxFbWFpbF N0cmluZz5qb2FuYnJ1Z3VlcmFtQGdtYWlsLmNvbTwvRW1haWxTdHJp bmc+DQogICAgICAgIDwvRW1haWw+DQogICAgICA8L0VtYWlscz4NCi AgICAgIDxDb250YWN0U3RyaW5nPkpvYW4gQnJ1Z3VlcmEgTWljw7Mg Jmx0O2pvYW5icnVndWVyYW1AZ21haWwuY29tPC9Db250YWN0U3RyaW 5nPg0KICAgIDwvQ29udGFjdD4NCiAgPC9Db250YWN0cz4NCjwvQ29u dGFjdFNldD4BDs8BUmV0cmlldmVyT3BlcmF0b3IsMTAsMTtSZXRyaW V2ZXJPcGVyYXRvciwxMSwxO1Bvc3REb2NQYXJzZXJPcGVyYXRvciwx MCwwO1Bvc3REb2NQYXJzZXJPcGVyYXRvciwxMSwwO1Bvc3RXb3JkQn JlYWtlckRpYWdub3N0aWNPcGVyYXRvciwxMCwyO1Bvc3RXb3JkQnJl YWtlckRpYWdub3N0aWNPcGVyYXRvciwxMSwwO1RyYW5zcG9ydFdyaX RlclByb2R1Y2VyLDIwLDEy X-MS-Exchange-Forest-IndexAgent: 1 5079 X-MS-Exchange-Forest-EmailMessageHash: E9B9F661 X-MS-Exchange-Forest-Language: en X-MS-Exchange-Organization-Processed-By-Journaling: Journal Agent On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > The recently introduced support for %rip-relative relocations in the > call thunk template assumes that the code is being patched in-place, > so the destination of the relocation matches the address of the code. > This is not true for the call depth accounting emitted by the BPF JIT, > so the calculated address is wrong and usually causes a page fault. Could you share the link to what this 'rip-relative' relocation is ? > Pass the destination IP when the BPF JIT emits call depth accounting. > > Fixes: 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative relocations in call thunk template") Ohh. It's buried inside that patch. Pls make commit log a bit more clear that that commit 17bce3b2ae2d broke x86_call_depth_emit_accounting logic. > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > Reviewed-by: Uros Bizjak <ubizjak@gmail.com> > Acked-by: Ingo Molnar <mingo@kernel.org> > Cc: Alexei Starovoitov <ast@kernel.org> > Cc: Daniel Borkmann <daniel@iogearbox.net> > --- > arch/x86/include/asm/alternative.h | 4 ++-- > arch/x86/kernel/callthunks.c | 4 ++-- > arch/x86/net/bpf_jit_comp.c | 19 ++++++++----------- > 3 files changed, 12 insertions(+), 15 deletions(-) > > diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h > index fcd20c6dc7f9..67b68d0d17d1 100644 > --- a/arch/x86/include/asm/alternative.h > +++ b/arch/x86/include/asm/alternative.h > @@ -117,7 +117,7 @@ extern void callthunks_patch_builtin_calls(void); > extern void callthunks_patch_module_calls(struct callthunk_sites *sites, > struct module *mod); > extern void *callthunks_translate_call_dest(void *dest); > -extern int x86_call_depth_emit_accounting(u8 **pprog, void *func); > +extern int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip); > #else > static __always_inline void callthunks_patch_builtin_calls(void) {} > static __always_inline void > @@ -128,7 +128,7 @@ static __always_inline void *callthunks_translate_call_dest(void *dest) > return dest; > } > static __always_inline int x86_call_depth_emit_accounting(u8 **pprog, > - void *func) > + void *func, void *ip) > { > return 0; > } > diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c > index 30335182b6b0..e92ff0c11db8 100644 > --- a/arch/x86/kernel/callthunks.c > +++ b/arch/x86/kernel/callthunks.c > @@ -314,7 +314,7 @@ static bool is_callthunk(void *addr) > return !bcmp(pad, insn_buff, tmpl_size); > } > > -int x86_call_depth_emit_accounting(u8 **pprog, void *func) > +int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip) > { > unsigned int tmpl_size = SKL_TMPL_SIZE; > u8 insn_buff[MAX_PATCH_LEN]; > @@ -327,7 +327,7 @@ int x86_call_depth_emit_accounting(u8 **pprog, void *func) > return 0; > > memcpy(insn_buff, skl_call_thunk_template, tmpl_size); > - apply_relocation(insn_buff, tmpl_size, *pprog, > + apply_relocation(insn_buff, tmpl_size, ip, Did the logic inside apply_relocation() change to have a new meaning for 'dest' and 'src'? Answering to myself... yes. in that commit. Better commit log would have made the code review so much easier. > skl_call_thunk_template, tmpl_size); > > memcpy(*pprog, insn_buff, tmpl_size); > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index 09f7dc9d4d65..f2e8769f5eee 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -481,7 +481,7 @@ static int emit_rsb_call(u8 **pprog, void *func, void *ip) > { > void *adjusted_ip; > OPTIMIZER_HIDE_VAR(func); > - adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func, ip); Now I see why you added extra var in the previous patch. Should have mentioned it in the commit log. > return emit_patch(pprog, func, adjusted_ip, 0xE8); > } > > @@ -1973,20 +1973,17 @@ st: if (is_imm8(insn->off)) > > /* call */ > case BPF_JMP | BPF_CALL: { > - int offs; > + u8 *ip = image + addrs[i - 1]; > > func = (u8 *) __bpf_call_base + imm32; > if (tail_call_reachable) { > RESTORE_TAIL_CALL_CNT(bpf_prog->aux->stack_depth); > - if (!imm32) > - return -EINVAL; > - offs = 7 + x86_call_depth_emit_accounting(&prog, func); > - } else { > - if (!imm32) > - return -EINVAL; > - offs = x86_call_depth_emit_accounting(&prog, func); > + ip += 7; > } > - if (emit_call(&prog, func, image + addrs[i - 1] + offs)) > + if (!imm32) > + return -EINVAL; > + ip += x86_call_depth_emit_accounting(&prog, func, ip); > + if (emit_call(&prog, func, ip)) > return -EINVAL; > break; > } > @@ -2836,7 +2833,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im > * Direct-call fentry stub, as such it needs accounting for the > * __fentry__ call. > */ > - x86_call_depth_emit_accounting(&prog, NULL); > + x86_call_depth_emit_accounting(&prog, NULL, image); Overall it all makes sense. Pls respin with more precise commit logs. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting 2024-03-29 21:53 ` Alexei Starovoitov 2024-03-29 21:53 ` Alexei Starovoitov @ 2024-03-30 9:01 ` Uros Bizjak 2024-03-30 9:01 ` Uros Bizjak ` (2 more replies) 1 sibling, 3 replies; 12+ messages in thread From: Uros Bizjak @ 2024-03-30 9:01 UTC (permalink / raw) To: Alexei Starovoitov Cc: X86 ML, bpf, Network Development, LKML, Joan Bruguera Micó, Ingo Molnar, Alexei Starovoitov, Daniel Borkmann On Fri, Mar 29, 2024 at 10:53 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > > > The recently introduced support for %rip-relative relocations in the > > call thunk template assumes that the code is being patched in-place, > > so the destination of the relocation matches the address of the code. > > This is not true for the call depth accounting emitted by the BPF JIT, > > so the calculated address is wrong and usually causes a page fault. > > Could you share the link to what this 'rip-relative' relocation is ? Please see the "RIP relative addressing" section in [1]. [1] https://compas.cs.stonybrook.edu/~nhonarmand/courses/sp17/cse506/ref/assembly.html In our case: The callthunks patching creates a call thunk template in the .rodata section (please see arch/x86/kernel/callthunks.c) that is later copied to the .text section at the correct place. The template uses X86_call_depth in the pcpu_hot structure. Previously, the template used absolute location for X86_call_depth and the linker resolved the address in the template to this absolute location. There is no issue when this template is copied to the various places in the .text section. When we want to use PC relative relocations (to reduce the code size), then the linker calculates the address of the variable in the template according to the PC in the .rodata section. If we want to copy the template to its final location, then the address of X86_call_depth, relative to the PC, has to be adjusted, as explained in arch/x86/kernel/alternative.c, in the comment above apply_reloc_n macro. Uros. > > Pass the destination IP when the BPF JIT emits call depth accounting. > > > > Fixes: 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative relocations in call thunk template") > > Ohh. It's buried inside that patch. > Pls make commit log a bit more clear that that commit 17bce3b2ae2d > broke x86_call_depth_emit_accounting logic. > > > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > > Reviewed-by: Uros Bizjak <ubizjak@gmail.com> > > Acked-by: Ingo Molnar <mingo@kernel.org> > > Cc: Alexei Starovoitov <ast@kernel.org> > > Cc: Daniel Borkmann <daniel@iogearbox.net> > > --- > > arch/x86/include/asm/alternative.h | 4 ++-- > > arch/x86/kernel/callthunks.c | 4 ++-- > > arch/x86/net/bpf_jit_comp.c | 19 ++++++++----------- > > 3 files changed, 12 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h > > index fcd20c6dc7f9..67b68d0d17d1 100644 > > --- a/arch/x86/include/asm/alternative.h > > +++ b/arch/x86/include/asm/alternative.h > > @@ -117,7 +117,7 @@ extern void callthunks_patch_builtin_calls(void); > > extern void callthunks_patch_module_calls(struct callthunk_sites *sites, > > struct module *mod); > > extern void *callthunks_translate_call_dest(void *dest); > > -extern int x86_call_depth_emit_accounting(u8 **pprog, void *func); > > +extern int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip); > > #else > > static __always_inline void callthunks_patch_builtin_calls(void) {} > > static __always_inline void > > @@ -128,7 +128,7 @@ static __always_inline void *callthunks_translate_call_dest(void *dest) > > return dest; > > } > > static __always_inline int x86_call_depth_emit_accounting(u8 **pprog, > > - void *func) > > + void *func, void *ip) > > { > > return 0; > > } > > diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c > > index 30335182b6b0..e92ff0c11db8 100644 > > --- a/arch/x86/kernel/callthunks.c > > +++ b/arch/x86/kernel/callthunks.c > > @@ -314,7 +314,7 @@ static bool is_callthunk(void *addr) > > return !bcmp(pad, insn_buff, tmpl_size); > > } > > > > -int x86_call_depth_emit_accounting(u8 **pprog, void *func) > > +int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip) > > { > > unsigned int tmpl_size = SKL_TMPL_SIZE; > > u8 insn_buff[MAX_PATCH_LEN]; > > @@ -327,7 +327,7 @@ int x86_call_depth_emit_accounting(u8 **pprog, void *func) > > return 0; > > > > memcpy(insn_buff, skl_call_thunk_template, tmpl_size); > > - apply_relocation(insn_buff, tmpl_size, *pprog, > > + apply_relocation(insn_buff, tmpl_size, ip, > > Did the logic inside apply_relocation() change to have > a new meaning for 'dest' and 'src'? > Answering to myself... yes. in that commit. > Better commit log would have made the code review so much easier. > > > skl_call_thunk_template, tmpl_size); > > > > memcpy(*pprog, insn_buff, tmpl_size); > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > > index 09f7dc9d4d65..f2e8769f5eee 100644 > > --- a/arch/x86/net/bpf_jit_comp.c > > +++ b/arch/x86/net/bpf_jit_comp.c > > @@ -481,7 +481,7 @@ static int emit_rsb_call(u8 **pprog, void *func, void *ip) > > { > > void *adjusted_ip; > > OPTIMIZER_HIDE_VAR(func); > > - adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); > > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func, ip); > > Now I see why you added extra var in the previous patch. > Should have mentioned it in the commit log. > > > return emit_patch(pprog, func, adjusted_ip, 0xE8); > > } > > > > @@ -1973,20 +1973,17 @@ st: if (is_imm8(insn->off)) > > > > /* call */ > > case BPF_JMP | BPF_CALL: { > > - int offs; > > + u8 *ip = image + addrs[i - 1]; > > > > func = (u8 *) __bpf_call_base + imm32; > > if (tail_call_reachable) { > > RESTORE_TAIL_CALL_CNT(bpf_prog->aux->stack_depth); > > - if (!imm32) > > - return -EINVAL; > > - offs = 7 + x86_call_depth_emit_accounting(&prog, func); > > - } else { > > - if (!imm32) > > - return -EINVAL; > > - offs = x86_call_depth_emit_accounting(&prog, func); > > + ip += 7; > > } > > - if (emit_call(&prog, func, image + addrs[i - 1] + offs)) > > + if (!imm32) > > + return -EINVAL; > > + ip += x86_call_depth_emit_accounting(&prog, func, ip); > > + if (emit_call(&prog, func, ip)) > > return -EINVAL; > > break; > > } > > @@ -2836,7 +2833,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im > > * Direct-call fentry stub, as such it needs accounting for the > > * __fentry__ call. > > */ > > - x86_call_depth_emit_accounting(&prog, NULL); > > + x86_call_depth_emit_accounting(&prog, NULL, image); > > Overall it all makes sense. > Pls respin with more precise commit logs. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting 2024-03-30 9:01 ` Uros Bizjak @ 2024-03-30 9:01 ` Uros Bizjak 2024-03-30 9:01 ` Uros Bizjak 2024-04-01 18:02 ` Alexei Starovoitov 2 siblings, 0 replies; 12+ messages in thread From: Uros Bizjak @ 2024-03-30 9:01 UTC (permalink / raw) To: Alexei Starovoitov Cc: X86 ML, bpf, Network Development, LKML, Joan Bruguera Micó, Ingo Molnar, Alexei Starovoitov, Daniel Borkmann [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 36238 bytes --] On Fri, Mar 29, 2024 at 10:53 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > > > The recently introduced support for %rip-relative relocations in the > > call thunk template assumes that the code is being patched in-place, > > so the destination of the relocation matches the address of the code. > > This is not true for the call depth accounting emitted by the BPF JIT, > > so the calculated address is wrong and usually causes a page fault. > > Could you share the link to what this 'rip-relative' relocation is ? Please see the "RIP relative addressing" section in [1]. [1] https://compas.cs.stonybrook.edu/~nhonarmand/courses/sp17/cse506/ref/assembly.html In our case: The callthunks patching creates a call thunk template in the .rodata section (please see arch/x86/kernel/callthunks.c) that is later copied to the .text section at the correct place. The template uses X86_call_depth in the pcpu_hot structure. Previously, the template used absolute location for X86_call_depth and the linker resolved the address in the template to this absolute location. There is no issue when this template is copied to the various places in the .text section. When we want to use PC relative relocations (to reduce the code size), then the linker calculates the address of the variable in the template according to the PC in the .rodata section. If we want to copy the template to its final location, then the address of X86_call_depth, relative to the PC, has to be adjusted, as explained in arch/x86/kernel/alternative.c, in the comment above apply_reloc_n macro. Uros. > > Pass the destination IP when the BPF JIT emits call depth accounting. > > > > Fixes: 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative relocations in call thunk template") > > Ohh. It's buried inside that patch. > Pls make commit log a bit more clear that that commit 17bce3b2ae2d > broke x86_call_depth_emit_accounting logic. > > > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > > Reviewed-by: Uros Bizjak <ubizjak@gmail.com> > > Acked-by: Ingo Molnar <mingo@kernel.org> > > Cc: Alexei Starovoitov <ast@kernel.org> > > Cc: Daniel Borkmann <daniel@iogearbox.net> > > --- > > arch/x86/include/asm/alternative.h | 4 ++-- > > arch/x86/kernel/callthunks.c | 4 ++-- > > arch/x86/net/bpf_jit_comp.c | 19 ++++++++----------- > > 3 files changed, 12 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h > > index fcd20c6dc7f9..67b68d0d17d1 100644 > > --- a/arch/x86/include/asm/alternative.h > > +++ b/arch/x86/include/asm/alternative.h > > @@ -117,7 +117,7 @@ extern void callthunks_patch_builtin_calls(void); > > extern void callthunks_patch_module_calls(struct callthunk_sites *sites, > > struct module *mod); > > extern void *callthunks_translate_call_dest(void *dest); > > -extern int x86_call_depth_emit_accounting(u8 **pprog, void *func); > > +extern int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip); > > #else > > static __always_inline void callthunks_patch_builtin_calls(void) {} > > static __always_inline void > > @@ -128,7 +128,7 @@ static __always_inline void *callthunks_translate_call_dest(void *dest) > > return dest; > > } > > static __always_inline int x86_call_depth_emit_accounting(u8 **pprog, > > - void *func) > > + void *func, void *ip) > > { > > return 0; > > } > > diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c > > index 30335182b6b0..e92ff0c11db8 100644 > > --- a/arch/x86/kernel/callthunks.c > > +++ b/arch/x86/kernel/callthunks.c > > @@ -314,7 +314,7 @@ static bool is_callthunk(void *addr) > > return !bcmp(pad, insn_buff, tmpl_size); > > } > > > > -int x86_call_depth_emit_accounting(u8 **pprog, void *func) > > +int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip) > > { > > unsigned int tmpl_size = SKL_TMPL_SIZE; > > u8 insn_buff[MAX_PATCH_LEN]; > > @@ -327,7 +327,7 @@ int x86_call_depth_emit_accounting(u8 **pprog, void *func) > > return 0; > > > > memcpy(insn_buff, skl_call_thunk_template, tmpl_size); > > - apply_relocation(insn_buff, tmpl_size, *pprog, > > + apply_relocation(insn_buff, tmpl_size, ip, > > Did the logic inside apply_relocation() change to have > a new meaning for 'dest' and 'src'? > Answering to myself... yes. in that commit. > Better commit log would have made the code review so much easier. > > > skl_call_thunk_template, tmpl_size); > > > > memcpy(*pprog, insn_buff, tmpl_size); > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > > index 09f7dc9d4d65..f2e8769f5eee 100644 > > --- a/arch/x86/net/bpf_jit_comp.c > > +++ b/arch/x86/net/bpf_jit_comp.c > > @@ -481,7 +481,7 @@ static int emit_rsb_call(u8 **pprog, void *func, void *ip) > > { > > void *adjusted_ip; > > OPTIMIZER_HIDE_VAR(func); > > - adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); > > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func, ip); > > Now I see why you added extra var in the previous patch. > Should have mentioned it in the commit log. > > > return emit_patch(pprog, func, adjusted_ip, 0xE8); > > } > > > > @@ -1973,20 +1973,17 @@ st: if (is_imm8(insn->off)) > > > > /* call */ > > case BPF_JMP | BPF_CALL: { > > - int offs; > > + u8 *ip = image + addrs[i - 1]; > > > > func = (u8 *) __bpf_call_base + imm32; > > if (tail_call_reachable) { > > RESTORE_TAIL_CALL_CNT(bpf_prog->aux->stack_depth); > > - if (!imm32) > > - return -EINVAL; > > - offs = 7 + x86_call_depth_emit_accounting(&prog, func); > > - } else { > > - if (!imm32) > > - return -EINVAL; > > - offs = x86_call_depth_emit_accounting(&prog, func); > > + ip += 7; > > } > > - if (emit_call(&prog, func, image + addrs[i - 1] + offs)) > > + if (!imm32) > > + return -EINVAL; > > + ip += x86_call_depth_emit_accounting(&prog, func, ip); > > + if (emit_call(&prog, func, ip)) > > return -EINVAL; > > break; > > } > > @@ -2836,7 +2833,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im > > * Direct-call fentry stub, as such it needs accounting for the > > * __fentry__ call. > > */ > > - x86_call_depth_emit_accounting(&prog, NULL); > > + x86_call_depth_emit_accounting(&prog, NULL, image); > > Overall it all makes sense. > Pls respin with more precise commit logs. X-sender: <linux-kernel+bounces-125632-steffen.klassert=cunet.com@vger.kernel.org> X-Receiver: <steffen.klassert@secunet.com> ORCPT=c822;steffen.klassert@secunet.com NOTIFY=VER; X-ExtendedProps=AVABYAAgAAAAUAFAARAPDFCS25BAlDktII2g02frgPADUAAABNaWNyb3NvZnQuRXhjaGFuZ2UuVHJhbnNwb3J0LkRpcmVjdG9yeURhdGEuSXNSZXNvdXJjZQIAAAUAagAJAAEAAAAAAAAABQAWAAIAAAUAQwACAAAFAEYABwADAAAABQBHAAIAAAUAEgAPAGIAAAAvbz1zZWN1bmV0L291PUV4Y2hhbmdlIEFkbWluaXN0cmF0aXZlIEdyb3VwIChGWURJQk9IRjIzU1BETFQpL2NuPVJlY2lwaWVudHMvY249U3RlZmZlbiBLbGFzc2VydDY4YwUACwAXAL4AAACheZxkHSGBRqAcAp3ukbifQ049REI2LENOPURhdGFiYXNlcyxDTj1FeGNoYW5nZSBBZG1pbmlzdHJhdGl2ZSBHcm91cCAoRllESUJPSEYyM1NQRExUKSxDTj1BZG1pbmlzdHJhdGl2ZSBHcm91cHMsQ049c2VjdW5ldCxDTj1NaWNyb3NvZnQgRXhjaGFuZ2UsQ049U2VydmljZXMsQ049Q29uZmlndXJhdGlvbixEQz1zZWN1bmV0LERDPWRlBQAOABEABiAS9uuMOkqzwmEZDvWNNQUAHQAPAAwAAABtYngtZXNzZW4tMDIFADwAAgAADwA2AAAATWljcm9zb2Z0LkV4Y2hhbmdlLlRyYW5zcG9ydC5NYWlsUmVjaXBpZW50LkRpc3BsYXlOYW1lDwARAAAAS2xhc3NlcnQsIFN0ZWZmZW4FAAwAAgAABQBsAAIAAAUAWAAXAEoAAADwxQktuQQJQ5LSCNoNNn64Q049S2xhc3NlcnQgU3RlZmZlbixPVT1Vc2VycyxPVT1NaWdyYXRpb24sREM9c2VjdW5ldCxEQz1kZQUAJgACAAEFACIADwAxAAAAQXV0b1Jlc3BvbnNlU3VwcHJlc3M6IDANClRyYW5zbWl0SGlzdG9yeTogRmFsc2UNCg8ALwAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuRXhwYW5zaW9uR3JvdXBUeXBlDwAVAAAATWVtYmVyc0dyb3VwRXhwYW5zaW9uBQAjAAIAAQ= X-CreatedBy: MSExchange15 X-HeloDomain: b.mx.secunet.com X-ExtendedProps: BQBjAAoADaTp8x1Q3AgFAGEACAABAAAABQA3AAIAAA8APAAAAE1pY3Jvc29mdC5FeGNoYW5nZS5UcmFuc3BvcnQuTWFpbFJlY2lwaWVudC5Pcmdhbml6YXRpb25TY29wZREAAAAAAAAAAAAAAAAAAAAAAAUASQACAAEFAGIACgAaAAAAl4oAAAUABAAUIAEAAAAcAAAAc3RlZmZlbi5rbGFzc2VydEBzZWN1bmV0LmNvbQUABgACAAEFACkAAgABDwAJAAAAQ0lBdWRpdGVkAgABBQACAAcAAQAAAAUAAwAHAAAAAAAFAAUAAgABBQBkAA8AAwAAAEh1Yg= X-Source: SMTP:Default MBX-DRESDEN-01 X-SourceIPAddress: 62.96.220.37 X-EndOfInjectedXHeaders: 27137 Received: from cas-essen-01.secunet.de (10.53.40.201) by mbx-dresden-01.secunet.de (10.53.40.199) with Microsoft SMTP Server (version=S1_2, cipher=S_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.37; Sat, 30 Mar 2024 10:01:43 +0100 Received: from b.mx.secunet.com (62.96.220.37) by cas-essen-01.secunet.de (10.53.40.201) with Microsoft SMTP Server (version=S1_2, cipher=S_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Sat, 30 Mar 2024 10:01:43 +0100 Received: from localhost (localhost [127.0.0.1]) by b.mx.secunet.com (Postfix) with ESMTP id 10A2520315 for <steffen.klassert@secunet.com>; Sat, 30 Mar 2024 10:01:43 +0100 (CET) X-Virus-Scanned: by secunet X-Spam-Flag: NO X-Spam-Score: -5.049 X-Spam-Level: X-Spam-Status: No, score=.049 tagged_above=99 required=1 tests=AYES_00=.9, DKIM_SIGNED=1, DKIM_VALID=.1, DKIM_VALID_AU=.1, FREEMAIL_FORGED_FROMDOMAIN=001, FREEMAIL_FROM=001, HEADER_FROM_DIFFERENT_DOMAINS=249, MAILING_LIST_MULTI=, RCVD_IN_DNSWL_MED=.3, SPF_HELO_NONE=001, SPF_PASS=.001] autolearn=m autolearn_force= Authentication-Results: a.mx.secunet.com (amavisd-new); dkim=ss (2048-bit key) header.d=ail.com Received: from b.mx.secunet.com ([127.0.0.1]) by localhost (a.mx.secunet.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id B8ZWMzr-RJt4 for <steffen.klassert@secunet.com>; Sat, 30 Mar 2024 10:01:42 +0100 (CET) Received-SPF: Pass (sender SPF authorized) identity=ilfrom; client-ip\x139.178.88.99; helo=.mirrors.kernel.org; envelope-from=nux-kernel+bounces-125632-steffen.klassert=cunet.com@vger.kernel.org; receiver=effen.klassert@secunet.com DKIM-Filter: OpenDKIM Filter v2.11.0 b.mx.secunet.com A420F2025D Authentication-Results: b.mx.secunet.com; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=le+vzJj" Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org [139.178.88.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by b.mx.secunet.com (Postfix) with ESMTPS id A420F2025D for <steffen.klassert@secunet.com>; Sat, 30 Mar 2024 10:01:41 +0100 (CET) Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 9FF4D282F2A for <steffen.klassert@secunet.com>; Sat, 30 Mar 2024 09:01:39 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id D1AE9DF5A; Sat, 30 Mar 2024 09:01:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=le+vzJj" Received: from mail-lj1-f171.google.com (mail-lj1-f171.google.com [209.85.208.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3F2F7847E; Sat, 30 Mar 2024 09:01:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=ne smtp.client-ip 9.85.208.171 ARC-Seal: i= a=a-sha256; d=bspace.kernel.org; s=c-20240116; t\x1711789276; cv=ne; b=liKGtxdDA5DncUU3WQ86IqJfxxDdmZL62lReMe091FMMVzSAkVNUab0eixlHeqVTjIQ9EEtdUySkdcLp6rFtixRUTTmQdeeFsygIBsVZzOYDo25RMQNI1crLiHrVMDdGnqz68Wu6CyxrARRiiLwfKV6AS/7WwNSvW09W60zckARC-Message-Signature: i= a=a-sha256; d=bspace.kernel.org; s=c-20240116; t\x1711789276; c=laxed/simple; bhÏUfaZfb7X/JJv0DB5cn0CBJrMrECWzMZrlMK1UCZYA= h=ME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=BMwVDUvqhNVOZ/cIBlL/eEF8qwRIumKR1fMWrSA7H4zcSOpuQv/ioQzXQqrodsIUyARBuvkzwtgX1F3ahG1oBV9JsqyrWjN58qqzsylGWNrONt419EuA+HV2Hnb6GN/l1yLYA/QgPCW6C6kp/GDTzIPXK00zlXkPjIpLmtnu4ARC-Authentication-Results: i= smtp.subspace.kernel.org; dmarc=ss (p=ne dis=ne) header.from=ail.com; spf=ss smtp.mailfrom=ail.com; dkim=ss (2048-bit key) header.d=ail.com header.i=mail.com header.b=e+vzJj; arc=ne smtp.client-ip 9.85.208.171 Authentication-Results: smtp.subspace.kernel.org; dmarc=ss (p=ne dis=ne) header.from=ail.com Authentication-Results: smtp.subspace.kernel.org; spf=ss smtp.mailfrom=ail.com Received: by mail-lj1-f171.google.com with SMTP id 38308e7fff4ca-2d6ee6c9945so20258311fa.3; Sat, 30 Mar 2024 02:01:13 -0700 (PDT) DKIM-Signature: v= a=a-sha256; c=laxed/relaxed; d=ail.com; s 230601; t\x1711789272; x\x1712394072; darn=er.kernel.org; h=ntent-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bhËl/CLMq3nWKqNjaubWInMz1kaQBbZVVwZ4B6Yhcsik= b=e+vzJjShWs0Ti3+DNi47la3wp5ECIHDAW+EvbhX+Ton8pxr5GAT+OKLN9tkpsdWp 7pmY92Vv0JN9muB7m6ZKKOed2eTaDIbVC7yQe3iYoQYmuzNt6FG/2CRXCn5axo5T0Er+ mIawl+AjZ9LcJ+2LwfqLqtGS9Swzi333W06D5Rkope/9r9oRpQjGT7vy46uzFz6Xcl6b S77spLQ2G+CQpl+4BKwdDBzhD3S36TpnXk6mrY+NzXkbO3VsCQIQvMCBSh38nAhwLdwg DDcnS4Gu8cVLbgH/tpVDy41vXbLuY8G8wYbQmHDkXrHBBWVRq9R4WCn8orBArD2q7glw BXGA= X-Google-DKIM-Signature: v= a=a-sha256; c=laxed/relaxed; d\x1e100.net; s 230601; t\x1711789272; x\x1712394072; h=ntent-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bhËl/CLMq3nWKqNjaubWInMz1kaQBbZVVwZ4B6Yhcsik= b=ssUbPdoWrUM0u1xqTsea9iktJot0W8ikz/21w6oDLw29oHo1E/zyykrKQer/3OWO qPdKEegxJqdMg3v5GqOnOKWhyGLwFbh6p2b85j1Ts+6BbLFl9oFkAqze+79DH3UTgvpX 8eHg+dcra2W81v3T0kNwO/Rz19x/a4qUnJoY4xXfsGku0ULlQwIKn/HG4uSfeFFaQsvQ uTzuZm8A/9h9KcYqL8SvAcdlpvE/AK5/d243lGtC7Es8bpk5HpBF6YLgEqB9CQU7scmf XLKGETg+gvfwQ/3vIK2nDBH9GUewEyTA+wZonkJx4DDqfRJaO4cU3mQBB6idP5IB1mep Hw+g= X-Forwarded-Encrypted: i= AJvYcCXMPjsPXXatSVDH641eiZD9Tylx5ujw2zvU8pLsO9HYR1XJhfBDwIOO8cDOnkZQvsFw51iNGrNYsj+ZOC9s/nmtelqQeBg/s4CmUPCyx03PDGJu4fX5flrhGr2i0fu9s/8eCySMSfdAa696W6WVC4RItEDieQnLUtLc X-Gm-Message-State: AOJu0YxogawIyIXt1FmwtLXKYPIfA7/NqcoJp2+CkbXskVq401KWqBS9 dv/uHIbm1VAuF/Vdv+59bCR/JaF94WfnbXteVNB+T1wBqWVEkaHONlAM1xQ2s4fdMV8thx39cxA tPcarlYh9vw0yYnOBdeRY+OsGAlIX-Google-Smtp-Source: AGHT+IGeaymGC7cAQwieQg4TakZq0ARBnpQFix3PExtV6ez87eomevGZHhs6eDxl1CtxiB/HrZm8Uq2a8idogtbdJIcX-Received: by 2002:a2e:8612:0:b0:2d6:a5ce:b261 with SMTP id a18-20020a2e8612000000b002d6a5ceb261mr2175202lji.25.1711789271815; Sat, 30 Mar 2024 02:01:11 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 References: <20240329094906.18147-1-ubizjak@gmail.com> <20240329094906.18147-3-ubizjak@gmail.com> <CAADnVQ+6D++hCXaP=+Q5wienMzhHo3h9YCvpA_7sHjMt+q6A@mail.gmail.com> In-Reply-To: <CAADnVQ+6D++hCXaP=+Q5wienMzhHo3h9YCvpA_7sHjMt+q6A@mail.gmail.com> From: Uros Bizjak <ubizjak@gmail.com> Date: Sat, 30 Mar 2024 10:01:04 +0100 Message-ID: <CAFULd4b6juiw3wC3Z61V9=nA+NGyUt4231vC14UnGAATk6tA@mail.gmail.com> Subject: Re: [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting To: Alexei Starovoitov <alexei.starovoitov@gmail.com> Cc: X86 ML <x86@kernel.org>, bpf <bpf@vger.kernel.org>, Network Development <netdev@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, =TF-8?Q?Joan_Bruguera_Micó?=oanbrugueram@gmail.com>, Ingo Molnar <mingo@kernel.org>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net> Content-Type: text/plain; charset=TF-8" Content-Transfer-Encoding: quoted-printable Return-Path: linux-kernel+bounces-125632-steffen.klassert=cunet.com@vger.kernel.org X-MS-Exchange-Organization-OriginalArrivalTime: 30 Mar 2024 09:01:43.0950 (UTC) X-MS-Exchange-Organization-Network-Message-Id: 728bc2a0-e193-4119-8421-08dc5098051f X-MS-Exchange-Organization-OriginalClientIPAddress: 62.96.220.37 X-MS-Exchange-Organization-OriginalServerIPAddress: 10.53.40.201 X-MS-Exchange-Organization-Cross-Premises-Headers-Processed: cas-essen-01.secunet.de X-MS-Exchange-Organization-OrderedPrecisionLatencyInProgress: LSRV=x-dresden-01.secunet.de:TOTAL-HUB=436|SMR=345(SMRDE=035|SMRC=310(SMRCL=101|X-SMRCR=310))|CAT=090(CATOS=012 (CATSM=012(CATSM-Malware Agent=012))|CATRESL=040(CATRESLP2R=017)|CATORES=035 (CATRS=035(CATRS-Index Routing Agent=034)));2024-03-30T09:01:43.556Z X-MS-Exchange-Forest-ArrivalHubServer: mbx-dresden-01.secunet.de X-MS-Exchange-Organization-AuthSource: cas-essen-01.secunet.de X-MS-Exchange-Organization-AuthAs: Anonymous X-MS-Exchange-Organization-FromEntityHeader: Internet X-MS-Exchange-Organization-OriginalSize: 16312 X-MS-Exchange-Organization-HygienePolicy: Standard X-MS-Exchange-Organization-MessageLatency: SRVÊs-essen-01.secunet.de:TOTAL-FE=025|SMR=009(SMRPI=007(SMRPI-FrontendProxyAgent=007))|SMS=015 X-MS-Exchange-Organization-AVStamp-Enterprise: 1.0 X-MS-Exchange-Organization-Recipient-Limit-Verified: True X-MS-Exchange-Organization-TotalRecipientCount: 1 X-MS-Exchange-Organization-Rules-Execution-History: 0b0cf904-14ac-4724-8bdf-482ee6223cf2%%%fd34672d-751c-45ae-a963-ed177fcabe23%%%d8080257-b0c3-47b4-b0db-23bc0c8ddb3c%%%95e591a2-5d7d-4afa-b1d0-7573d6c0a5d9%%%f7d0f6bc-4dcc-4876-8c5d-b3d6ddbb3d55%%%16355082-c50b-4214-9c7d-d39575f9f79b X-MS-Exchange-Forest-RulesExecuted: mbx-dresden-01 X-MS-Exchange-Organization-RulesExecuted: mbx-dresden-01 X-MS-Exchange-Forest-IndexAgent-0: AQ0CZW4AAT4OAAAPAAADH4sIAAAAAAAEAMVZ6XLb1hUGKS4SJXppnK Zp486Nk9jaSImUrM2uY9lRJkolW2MraaaZDAcEL0VYIMDBoiWpO32j /ugj9E3aF+l3zgVAkARFOclMMRwSujj7+c6550L/ffTSFl+65rI41F 1R314W9dX6utB9UVvdebj2n3/88+hQ7FryQprita+7zplj+s5ZufRY 58Wq1198etLVTatqON0n4tx1fLlTLj3BR4xTUd9Z34aG3UPxjet44p n54xv9VDwOmnyTKk6wwCeQ53R3xNeObotnbnASSFcXh6bx73+Jx2+w 2AzXugkhCe7jjhSuNKTtW5fCtH3XaQWGbAkv6PUc1xdtxxWfuWav4k pL980zorYcA7eO7YFB+B2pJBm6ZeGvwD4Vvuz2QC2F7nlBV3pYho+g FIbTksL0RFOa9ono6b7RgTLTroDekMtKkucwbUt6vmmzJuG0eamvW3 SZ1+NlvdVypedFVKSkGrkHZfjYDvS7gWR/mIasbcme3xG6YTiB7ZNB smv6PgxqXjLRs6Mvxdf7x4NmgdMIyLtWrBcKkBbw63ZLBF4A2ZegCz zYp8PLE+jVA8uvhih47gRWS1w6gfA6uitZrGVS4BxxrkIFkQ+SYX+Q 9B0PPy+XyqUjS+qeFJ5UIu692j8ScZ5C4+DWPVAYitEW39d+qBIvfk XH93vezsoKMNHTvarhAcOOfdl0Hee0KlvByt/tjmPrbhd+gShw4dCK 16ttrhiefLi6seLK9gpyLLtN67La8bsWSd5HugIX/nuE03LpOAw3Y8 NTSadgG65EFClCadBR0BJVAFL39XIpcmG+13dad43OysXWxsqpdG1p rfS1VI0FoVCHWJE8t1wynJ6JpPkqjVVfXvhxYGJ4uigGXzAaq1wcsU GUzXLpu62NBqlpKOyEVvaMXtDoAGMeQGb4gQvmI1eemU7gWZfLTBMJ KpcgCdhpeo4VQG6cVYLmkHjCUwQO6SK34DmTLVV0MfrsAfnKQbg9oo EdcqWqB3x7AaScd6St6Puh98RgrM50lzxRYYkVcgTjxDCo/kLCzqU4 122fmOGpOHouUnvHPJ67krpNvzN45o9yAeXmK6Nix+OaS613Mk9vWn I4EggRStttEdhCT2DMILAiBFTFfjtpOQJwqcKcjKvpe6KNnmTFMeXc 2sNWDaYRDsURiO1YFh3doz+bxPkm8NBQltEwhbyAOtPmtggPhiCuW8 CyzbKqxnLkDAq4ixaOlDtU+b2eddngWDcgoqsbrsPpob2Fb6idHaFw R/osGsh55E/Y/bgpeukNs5rch8wL6e2I2mbTkGvNui7rLTF/j0zvl+ WO+AqQRqau3FJS2sG9hWgD7XSQKv8BtpDANTlIntmSqta5tbBNR5aH HeJURcb0kS40Z9HEXddBBRjoIW60K+ErpEraTlLQCCHiYiCZDQpHI7 FnQLRpRJ39iXhtniB3FafdrjQvf86+LF6hb8hzyCD+CeOA4tg1TkPy ffvEEYeOhaYtHndhnvNUAafquCch9XNjJ2WOEY91z08n/kK3TWmJZ4 57ip3AFo9bvPDUdE4QxqZzUbWlH9JXKhV102/Opm1YQUtio+gOwLcj /ibEulhaGmVJ6edCXWNZYMJKs9duvEF2aD+LOcBS2waLuir9K5Swho q20FiMjm6fUA3W6gQq6TIe55cWsPIQyLekWqgsJEDfMttt+HwC8Ogr 1/C4eQ0iJdm0W/JCtI1WfdXYaBmb7e1qdWOzubHVWm3VNls1TKWrG+ vrcdSvpV9RIwzvYMjTp6JSq20ub4ol9YMFNH6QCSCnldjaG1x/jWZg WqgMrhlvnmgWHoWxvpKvi9HTkiGb2kj7RA3PpO6/yD/hQHb9K5SmNI hF/KaatJiwyXd126PWE9W+588rIrqN2CshOybnCX1iPtgSi4u9nuuc LIfa2oFtRIKWfqmg6N7sxa59Ii0vHM4RASTVEI2Gbp3rl17DtLG1yu tnUPz0drKkBGDqWwwY/sHCVerfIeqDeXclJi2b96/I5QlGvlt0wxS/ A84Gr0SWwyT/CqISeQ59/Sk1KqtDIUnvVGmNtnnl42R3WltdW3tY26 o3N5qr1arcrrfbq0at1mpuXdWdxgod6ktj6Qhga7V1Apj66QOs6TgW 5tdGzBSih2azdPR83DS6vfme3qJhyrOB/XYbQx3GjgZPo4NhDB36+e UeevprlHl6+gPMQydqevT7bog/idd/PmgcHx4dNF7v/3Xv0RDTVt/5 7w93v2sc7R4//6pxsPfih0eJmNd5F1A/WPilURi+BpE7SNOVXaN3OZ 9IkXdqKc1qf4gmxZTcRRWcmIt52JxPS/iyGKj+pXfjNXvL4ST4hRke 3Wg8jKbUESkL4dhBB4GOfsbdWhe2PIfDGLIwXtKp8AG1uAd8Gnzguc aDz4ls1/bOpRuebbqXnrTa1WpVXEqvqo4F8VzLE/Ez6ft0kurPw+f8 FoK0YlRuJU5hLg+g9L6jGxgdgeO2Kd3+hDv2unZGUlMbweSqKkxvYi mjX/Oqp8kWtrrd3mwZ26311sbDarVdl1ubG9vth1LKq1rYOJlDHWwc GRXT+laNikn99BsY1RQXkes1OZg/tw9EfU+dLBtmb6jkXx4d7x+iE7 xqfLX/xV7j291X88l5JK6ZvgD0EHwtTSr50NSB4ebXEEbFxQJJ5gvn XOzzO6DzziW/SUOHR9PDFIVj1hlOP9GbmfAtTOJs+LqTAD5OzahDap d+8jCtSmQU82GLYjNZ4qCFCQeXxerF3lbq9sED0vbm2nJ9FSMS3dRC COyk1pXZFvPY08xud4vbTuUJTpcLCymlNHytLKrD9OJKOh29o6Nzfu PrwyOck+ju+e7BwU4EpnGzD6EUNnhD+R2+CLoq1V16B7rEr0i8703I rf2Q1gqGL4oq2LkGFjDNUTExWppk+BLkdtfqj66WQdHzcV5WjK7U0X GbllwYLphx16u918cvX+01jnf3Dzg6jecvjufJEsp85YkeXFSeoHqN U4Xh4Qq6yq6P2YGFdxs0QwxW9vZffLt7cE1tlC1EcnNyyd0frd9xwt 8KOmFMAsv/1eF3d3bSkE5tC4GcgLq3EwoIsWBbuMXfH+hyKbWCP8mf hQnniJEIT/IlNbJjpbPj1w9o3LAnWDwuDr2FMXPitbwYdzXRAU7H0L zt9+f61toG7c/4XRvZoHGsxBaP+pc93ZXclbDtdHsOHTKjVxfxakNl dNHsRpu2e461MWYuYnKk/0FUuHG3sT+5l9AdNPnlsEcDGTYnW8qWl/ zPVfh/rbFCGw0lqtHgHaE6jnAlHbfXy/qLbw4OxmX8+hLCEoj3+pdn 0qVY0NCHH3qti0BI25Pxq15Xej3s3uem31EveJEaw/SSe7l68a1pWW 1qSstPZbQ5fLRcVsvltHw2MzWtadPaTEErzmo3ChntI7qZy2sFEOe1 8jiuoqYVtWncF7TpYka7rU1HLLdzWgHrkImbckb7QLvBj25OaTkslr RZMIYyeQXqWAvowV7AipI8pXRpU1jESjYDO3GviHMRVy4SmCMaXslp s4pRScC3uoEcRawe8aeUFKJ0KVEISEbLMldZLapvJRnf8C5yoQT2LL OwzHzk6RybnctoWiYzTd+Qn9FeMSWEgAvESjVHsqgYc31HZpRtKiaR PRQilRcKPucu9hT3oM/0Q1dUuqbI4Hzse5HyFasjR25EIQpVU7RJ4I w2C0eYsqTSOs1WJVlUMKGIH/W1KDsjrpISrqziAOYVQWRhGPbIWiR9 Lo62IsA3A6asuJTXuQgtyvckEqJAkfEzUa6VDRy9YpEJYhUsJx9zca hLSngos+/1dEyWZfeVnGnWwrr6GZml6plKRL4AGJTxrRUIEqSirIgV qmPQKhsi6P4WcZvjOkJGkPqhCoqwcVvlusCp58WSCpSKGFAUl0Muop zTbuUZBn9MPEIw89rNAplanIpqf6SuS8DlLIUiN9INSnlCUSF+hD9v aQU2g+LDkSmrkCYXEUMovaMVZ8jyO3H8VXwymTyXUnkA8JnZ5KLC51 xU+HiaRQPM3GWaLPWTzEyWW2Ipc1PdzGgf5KL4DGsEa8piEXLmtBtz mXJB0wrarTSa2ZTFDNINNexFJtu/50RQ2CkLs6rpcSLK8fpdXi+wan zPZLSbVFb4c4PTDRVzTFxQQmYYvapZKTkqMkNZztJ98do09MlSbc4W MsWbWjZG2ojlNwcIMuWxjg/ISTX+PX50J0k53s4vmWUvYe2HYGRdt4 ayHDXM7MjK2PxGBLPDK5MymyVMTrFfyiPVuPKwZka5Gd78flIK5q6X ow9Vr47EFqeollXPfI86RrieV+vcAW6Mc2GSxj9EEX6fcx16NEWNIh e1plK0fke1AkgY8R1hv+IpnCrFjZEKIaQsq40jx7tkVG5sAFk+o5p/ gXjDPl8AeiMUxZ0w7pMRzcIY+z8s8AY6Yt7cELqGG1ECZiOPxuJthH J27KMrEaiQUO5PVsNowXfUzG+jt/wm2vuQr2iduNK2ieL1aKBlNt6D 4jyqESLesrFhqXpRWVPrbH8IrRKLVQL7u0/mo7yGEBS41U/170np70 CA0BfjyU1byml3FfKLZAYBdZoM/oz9/YToeZenPVqNcNp9GswyhZvR BspY/ZRVa+8xfma0Omrgfa7rXFQyLJZnYzIVZldY8nqBpp1KLqya0e Ka47xA6fIAPVdxGuUybGNRn2f7JR8md9ARrH/KMpdZ5mfZlK1/LkLF vVFpWW1RMeYz91WcFd769wpvmdxdBqra+0pccVO0Z6mkF3kwKCAvec oyGgjiU8yF3bIQT2Vqbme0/LFA48f6GGs/GudFXrsRz4SsuqQQlYum Yrr/H4HsKjsIKwAAAQKDBTw/eG1sIHZlcnNpb249IjEuMCIgZW5jb2 Rpbmc9InV0Zi0xNiI/Pg0KPFRhc2tTZXQ+DQogIDxWZXJzaW9uPjE1 LjAuMC4wPC9WZXJzaW9uPg0KICA8VGFza3M+DQogICAgPFRhc2sgU3 RhcnRJbmRleD0iNjEwIj4NCiAgICAgIDxUYXNrU3RyaW5nPiZndDsg Q291bGQgeW91IHNoYXJlIHRoZSBsaW5rIHRvIHdoYXQgdGhpcyAncm lwLXJlbGF0aXZlJyByZWxvY2F0aW9uIGlzID88L1Rhc2tTdHJpbmc+ DQogICAgICA8QXNzaWduZWVzPg0KICAgICAgICA8RW1haWxVc2VyIE lkPSJhbGV4ZWkuc3Rhcm92b2l0b3ZAZ21haWwuY29tIj5BbGV4ZWkg U3Rhcm92b2l0b3Y8L0VtYWlsVXNlcj4NCiAgICAgIDwvQXNzaWduZW VzPg0KICAgIDwvVGFzaz4NCiAgICA8VGFzayBTdGFydEluZGV4PSI2 ODQiPg0KICAgICAgPFRhc2tTdHJpbmc+UGxlYXNlIHNlZSB0aGUgIl JJUCByZWxhdGl2ZSBhZGRyZXNzaW5nIiBzZWN0aW9uIGluIFsxXS48 L1Rhc2tTdHJpbmc+DQogICAgICA8QXNzaWduZWVzPg0KICAgICAgIC A8RW1haWxVc2VyIElkPSJhbGV4ZWkuc3Rhcm92b2l0b3ZAZ21haWwu Y29tIj5BbGV4ZWkgU3Rhcm92b2l0b3Y8L0VtYWlsVXNlcj4NCiAgIC AgIDwvQXNzaWduZWVzPg0KICAgIDwvVGFzaz4NCiAgPC9UYXNrcz4N CjwvVGFza1NldD4BCr0DPD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZG luZz0idXRmLTE2Ij8+DQo8RW1haWxTZXQ+DQogIDxWZXJzaW9uPjE1 LjAuMC4wPC9WZXJzaW9uPg0KICA8RW1haWxzPg0KICAgIDxFbWFpbC BTdGFydEluZGV4PSI1NCI+DQogICAgICA8RW1haWxTdHJpbmc+YWxl eGVpLnN0YXJvdm9pdG92QGdtYWlsLmNvbTwvRW1haWxTdHJpbmc+DQ ogICAgPC9FbWFpbD4NCiAgICA8RW1haWwgU3RhcnRJbmRleD0iMTQy IiBQb3NpdGlvbj0iU2lnbmF0dXJlIj4NCiAgICAgIDxFbWFpbFN0cm luZz51Yml6amFrQGdtYWlsLmNvbTwvRW1haWxTdHJpbmc+DQogICAg PC9FbWFpbD4NCiAgICA8RW1haWwgU3RhcnRJbmRleD0iMjA0Ij4NCi AgICAgIDxFbWFpbFN0cmluZz5qb2FuYnJ1Z3VlcmFtQGdtYWlsLmNv bTwvRW1haWxTdHJpbmc+DQogICAgPC9FbWFpbD4NCiAgPC9FbWFpbH M+DQo8L0VtYWlsU2V0PgELlAI8P3htbCB2ZXJzaW9uPSIxLjAiIGVu Y29kaW5nPSJ1dGYtMTYiPz4NCjxVcmxTZXQ+DQogIDxWZXJzaW9uPj E1LjAuMC4wPC9WZXJzaW9uPg0KICA8VXJscz4NCiAgICA8VXJsIFN0 YXJ0SW5kZXg9Ijc0OCIgVHlwZT0iVXJsIj4NCiAgICAgIDxVcmxTdH Jpbmc+aHR0cHM6Ly9jb21wYXMuY3Muc3Rvbnlicm9vay5lZHUvfm5o b25hcm1hbmQvY291cnNlcy9zcDE3L2NzZTUwNi9yZWYvYXNzZW1ibH kuaHRtbDwvVXJsU3RyaW5nPg0KICAgIDwvVXJsPg0KICA8L1VybHM+ DQo8L1VybFNldD4BDLIKPD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZG luZz0idXRmLTE2Ij8+DQo8Q29udGFjdFNldD4NCiAgPFZlcnNpb24+ MTUuMC4wLjA8L1ZlcnNpb24+DQogIDxDb250YWN0cz4NCiAgICA8Q2 9udGFjdCBTdGFydEluZGV4PSIzMyI+DQogICAgICA8UGVyc29uIFN0 YXJ0SW5kZXg9IjMzIj4NCiAgICAgICAgPFBlcnNvblN0cmluZz5BbG V4ZWkgU3Rhcm92b2l0b3Y8L1BlcnNvblN0cmluZz4NCiAgICAgIDwv UGVyc29uPg0KICAgICAgPEVtYWlscz4NCiAgICAgICAgPEVtYWlsIF N0YXJ0SW5kZXg9IjU0Ij4NCiAgICAgICAgICA8RW1haWxTdHJpbmc+ YWxleGVpLnN0YXJvdm9pdG92QGdtYWlsLmNvbTwvRW1haWxTdHJpbm c+DQogICAgICAgIDwvRW1haWw+DQogICAgICA8L0VtYWlscz4NCiAg ICAgIDxDb250YWN0U3RyaW5nPkFsZXhlaSBTdGFyb3ZvaXRvdg0KJm x0O2FsZXhlaS5zdGFyb3ZvaXRvdkBnbWFpbC5jb208L0NvbnRhY3RT dHJpbmc+DQogICAgPC9Db250YWN0Pg0KICAgIDxDb250YWN0IFN0YX J0SW5kZXg9IjEyOSIgUG9zaXRpb249IlNpZ25hdHVyZSI+DQogICAg ICA8UGVyc29uIFN0YXJ0SW5kZXg9IjEyOSIgUG9zaXRpb249IlNpZ2 5hdHVyZSI+DQogICAgICAgIDxQZXJzb25TdHJpbmc+VXJvcyBCaXpq YWs8L1BlcnNvblN0cmluZz4NCiAgICAgIDwvUGVyc29uPg0KICAgIC AgPEVtYWlscz4NCiAgICAgICAgPEVtYWlsIFN0YXJ0SW5kZXg9IjE0 MiIgUG9zaXRpb249IlNpZ25hdHVyZSI+DQogICAgICAgICAgPEVtYW lsU3RyaW5nPnViaXpqYWtAZ21haWwuY29tPC9FbWFpbFN0cmluZz4N CiAgICAgICAgPC9FbWFpbD4NCiAgICAgIDwvRW1haWxzPg0KICAgIC AgPENvbnRhY3RTdHJpbmc+VXJvcyBCaXpqYWsgJmx0O3ViaXpqYWtA Z21haWwuY29tPC9Db250YWN0U3RyaW5nPg0KICAgIDwvQ29udGFjdD 4NCiAgICA8Q29udGFjdCBTdGFydEluZGV4PSIxODQiPg0KICAgICAg PFBlcnNvbiBTdGFydEluZGV4PSIxODQiPg0KICAgICAgICA8UGVyc2 9uU3RyaW5nPkpvYW4gQnJ1Z3VlcmE8L1BlcnNvblN0cmluZz4NCiAg ICAgIDwvUGVyc29uPg0KICAgICAgPEVtYWlscz4NCiAgICAgICAgPE VtYWlsIFN0YXJ0SW5kZXg9IjIwNCI+DQogICAgICAgICAgPEVtYWls U3RyaW5nPmpvYW5icnVndWVyYW1AZ21haWwuY29tPC9FbWFpbFN0cm luZz4NCiAgICAgICAgPC9FbWFpbD4NCiAgICAgIDwvRW1haWxzPg0K ICAgICAgPENvbnRhY3RTdHJpbmc+Sm9hbiBCcnVndWVyYSBNaWPDsy AmbHQ7am9hbmJydWd1ZXJhbUBnbWFpbC5jb208L0NvbnRhY3RTdHJp bmc+DQogICAgPC9Db250YWN0Pg0KICA8L0NvbnRhY3RzPg0KPC9Db2 50YWN0U2V0PgEOzwFSZXRyaWV2ZXJPcGVyYXRvciwxMCwwO1JldHJp ZXZlck9wZXJhdG9yLDExLDE7UG9zdERvY1BhcnNlck9wZXJhdG9yLD EwLDA7UG9zdERvY1BhcnNlck9wZXJhdG9yLDExLDA7UG9zdFdvcmRC cmVha2VyRGlhZ25vc3RpY09wZXJhdG9yLDEwLDI7UG9zdFdvcmRCcm Vha2VyRGlhZ25vc3RpY09wZXJhdG9yLDExLDA7VHJhbnNwb3J0V3Jp dGVyUHJvZHVjZXIsMjAsMTgX-MS-Exchange-Forest-IndexAgent: 1 6578 X-MS-Exchange-Forest-EmailMessageHash: 400C00D2 X-MS-Exchange-Forest-Language: en X-MS-Exchange-Organization-Processed-By-Journaling: Journal Agent On Fri, Mar 29, 2024 at 10:53 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > > > The recently introduced support for %rip-relative relocations in the > > call thunk template assumes that the code is being patched in-place, > > so the destination of the relocation matches the address of the code. > > This is not true for the call depth accounting emitted by the BPF JIT, > > so the calculated address is wrong and usually causes a page fault. > > Could you share the link to what this 'rip-relative' relocation is ? Please see the "RIP relative addressing" section in [1]. [1] https://compas.cs.stonybrook.edu/~nhonarmand/courses/sp17/cse506/ref/assembly.html In our case: The callthunks patching creates a call thunk template in the .rodata section (please see arch/x86/kernel/callthunks.c) that is later copied to the .text section at the correct place. The template uses X86_call_depth in the pcpu_hot structure. Previously, the template used absolute location for X86_call_depth and the linker resolved the address in the template to this absolute location. There is no issue when this template is copied to the various places in the .text section. When we want to use PC relative relocations (to reduce the code size), then the linker calculates the address of the variable in the template according to the PC in the .rodata section. If we want to copy the template to its final location, then the address of X86_call_depth, relative to the PC, has to be adjusted, as explained in arch/x86/kernel/alternative.c, in the comment above apply_reloc_n macro. Uros. > > Pass the destination IP when the BPF JIT emits call depth accounting. > > > > Fixes: 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative relocations in call thunk template") > > Ohh. It's buried inside that patch. > Pls make commit log a bit more clear that that commit 17bce3b2ae2d > broke x86_call_depth_emit_accounting logic. > > > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > > Reviewed-by: Uros Bizjak <ubizjak@gmail.com> > > Acked-by: Ingo Molnar <mingo@kernel.org> > > Cc: Alexei Starovoitov <ast@kernel.org> > > Cc: Daniel Borkmann <daniel@iogearbox.net> > > --- > > arch/x86/include/asm/alternative.h | 4 ++-- > > arch/x86/kernel/callthunks.c | 4 ++-- > > arch/x86/net/bpf_jit_comp.c | 19 ++++++++----------- > > 3 files changed, 12 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h > > index fcd20c6dc7f9..67b68d0d17d1 100644 > > --- a/arch/x86/include/asm/alternative.h > > +++ b/arch/x86/include/asm/alternative.h > > @@ -117,7 +117,7 @@ extern void callthunks_patch_builtin_calls(void); > > extern void callthunks_patch_module_calls(struct callthunk_sites *sites, > > struct module *mod); > > extern void *callthunks_translate_call_dest(void *dest); > > -extern int x86_call_depth_emit_accounting(u8 **pprog, void *func); > > +extern int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip); > > #else > > static __always_inline void callthunks_patch_builtin_calls(void) {} > > static __always_inline void > > @@ -128,7 +128,7 @@ static __always_inline void *callthunks_translate_call_dest(void *dest) > > return dest; > > } > > static __always_inline int x86_call_depth_emit_accounting(u8 **pprog, > > - void *func) > > + void *func, void *ip) > > { > > return 0; > > } > > diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c > > index 30335182b6b0..e92ff0c11db8 100644 > > --- a/arch/x86/kernel/callthunks.c > > +++ b/arch/x86/kernel/callthunks.c > > @@ -314,7 +314,7 @@ static bool is_callthunk(void *addr) > > return !bcmp(pad, insn_buff, tmpl_size); > > } > > > > -int x86_call_depth_emit_accounting(u8 **pprog, void *func) > > +int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip) > > { > > unsigned int tmpl_size = SKL_TMPL_SIZE; > > u8 insn_buff[MAX_PATCH_LEN]; > > @@ -327,7 +327,7 @@ int x86_call_depth_emit_accounting(u8 **pprog, void *func) > > return 0; > > > > memcpy(insn_buff, skl_call_thunk_template, tmpl_size); > > - apply_relocation(insn_buff, tmpl_size, *pprog, > > + apply_relocation(insn_buff, tmpl_size, ip, > > Did the logic inside apply_relocation() change to have > a new meaning for 'dest' and 'src'? > Answering to myself... yes. in that commit. > Better commit log would have made the code review so much easier. > > > skl_call_thunk_template, tmpl_size); > > > > memcpy(*pprog, insn_buff, tmpl_size); > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > > index 09f7dc9d4d65..f2e8769f5eee 100644 > > --- a/arch/x86/net/bpf_jit_comp.c > > +++ b/arch/x86/net/bpf_jit_comp.c > > @@ -481,7 +481,7 @@ static int emit_rsb_call(u8 **pprog, void *func, void *ip) > > { > > void *adjusted_ip; > > OPTIMIZER_HIDE_VAR(func); > > - adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); > > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func, ip); > > Now I see why you added extra var in the previous patch. > Should have mentioned it in the commit log. > > > return emit_patch(pprog, func, adjusted_ip, 0xE8); > > } > > > > @@ -1973,20 +1973,17 @@ st: if (is_imm8(insn->off)) > > > > /* call */ > > case BPF_JMP | BPF_CALL: { > > - int offs; > > + u8 *ip = image + addrs[i - 1]; > > > > func = (u8 *) __bpf_call_base + imm32; > > if (tail_call_reachable) { > > RESTORE_TAIL_CALL_CNT(bpf_prog->aux->stack_depth); > > - if (!imm32) > > - return -EINVAL; > > - offs = 7 + x86_call_depth_emit_accounting(&prog, func); > > - } else { > > - if (!imm32) > > - return -EINVAL; > > - offs = x86_call_depth_emit_accounting(&prog, func); > > + ip += 7; > > } > > - if (emit_call(&prog, func, image + addrs[i - 1] + offs)) > > + if (!imm32) > > + return -EINVAL; > > + ip += x86_call_depth_emit_accounting(&prog, func, ip); > > + if (emit_call(&prog, func, ip)) > > return -EINVAL; > > break; > > } > > @@ -2836,7 +2833,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im > > * Direct-call fentry stub, as such it needs accounting for the > > * __fentry__ call. > > */ > > - x86_call_depth_emit_accounting(&prog, NULL); > > + x86_call_depth_emit_accounting(&prog, NULL, image); > > Overall it all makes sense. > Pls respin with more precise commit logs. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting 2024-03-30 9:01 ` Uros Bizjak 2024-03-30 9:01 ` Uros Bizjak @ 2024-03-30 9:01 ` Uros Bizjak 2024-04-01 18:02 ` Alexei Starovoitov 2 siblings, 0 replies; 12+ messages in thread From: Uros Bizjak @ 2024-03-30 9:01 UTC (permalink / raw) To: Alexei Starovoitov Cc: X86 ML, bpf, Network Development, LKML, Joan Bruguera Micó, Ingo Molnar, Alexei Starovoitov, Daniel Borkmann [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 24958 bytes --] On Fri, Mar 29, 2024 at 10:53 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > > > The recently introduced support for %rip-relative relocations in the > > call thunk template assumes that the code is being patched in-place, > > so the destination of the relocation matches the address of the code. > > This is not true for the call depth accounting emitted by the BPF JIT, > > so the calculated address is wrong and usually causes a page fault. > > Could you share the link to what this 'rip-relative' relocation is ? Please see the "RIP relative addressing" section in [1]. [1] https://compas.cs.stonybrook.edu/~nhonarmand/courses/sp17/cse506/ref/assembly.html In our case: The callthunks patching creates a call thunk template in the .rodata section (please see arch/x86/kernel/callthunks.c) that is later copied to the .text section at the correct place. The template uses X86_call_depth in the pcpu_hot structure. Previously, the template used absolute location for X86_call_depth and the linker resolved the address in the template to this absolute location. There is no issue when this template is copied to the various places in the .text section. When we want to use PC relative relocations (to reduce the code size), then the linker calculates the address of the variable in the template according to the PC in the .rodata section. If we want to copy the template to its final location, then the address of X86_call_depth, relative to the PC, has to be adjusted, as explained in arch/x86/kernel/alternative.c, in the comment above apply_reloc_n macro. Uros. > > Pass the destination IP when the BPF JIT emits call depth accounting. > > > > Fixes: 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative relocations in call thunk template") > > Ohh. It's buried inside that patch. > Pls make commit log a bit more clear that that commit 17bce3b2ae2d > broke x86_call_depth_emit_accounting logic. > > > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > > Reviewed-by: Uros Bizjak <ubizjak@gmail.com> > > Acked-by: Ingo Molnar <mingo@kernel.org> > > Cc: Alexei Starovoitov <ast@kernel.org> > > Cc: Daniel Borkmann <daniel@iogearbox.net> > > --- > > arch/x86/include/asm/alternative.h | 4 ++-- > > arch/x86/kernel/callthunks.c | 4 ++-- > > arch/x86/net/bpf_jit_comp.c | 19 ++++++++----------- > > 3 files changed, 12 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h > > index fcd20c6dc7f9..67b68d0d17d1 100644 > > --- a/arch/x86/include/asm/alternative.h > > +++ b/arch/x86/include/asm/alternative.h > > @@ -117,7 +117,7 @@ extern void callthunks_patch_builtin_calls(void); > > extern void callthunks_patch_module_calls(struct callthunk_sites *sites, > > struct module *mod); > > extern void *callthunks_translate_call_dest(void *dest); > > -extern int x86_call_depth_emit_accounting(u8 **pprog, void *func); > > +extern int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip); > > #else > > static __always_inline void callthunks_patch_builtin_calls(void) {} > > static __always_inline void > > @@ -128,7 +128,7 @@ static __always_inline void *callthunks_translate_call_dest(void *dest) > > return dest; > > } > > static __always_inline int x86_call_depth_emit_accounting(u8 **pprog, > > - void *func) > > + void *func, void *ip) > > { > > return 0; > > } > > diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c > > index 30335182b6b0..e92ff0c11db8 100644 > > --- a/arch/x86/kernel/callthunks.c > > +++ b/arch/x86/kernel/callthunks.c > > @@ -314,7 +314,7 @@ static bool is_callthunk(void *addr) > > return !bcmp(pad, insn_buff, tmpl_size); > > } > > > > -int x86_call_depth_emit_accounting(u8 **pprog, void *func) > > +int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip) > > { > > unsigned int tmpl_size = SKL_TMPL_SIZE; > > u8 insn_buff[MAX_PATCH_LEN]; > > @@ -327,7 +327,7 @@ int x86_call_depth_emit_accounting(u8 **pprog, void *func) > > return 0; > > > > memcpy(insn_buff, skl_call_thunk_template, tmpl_size); > > - apply_relocation(insn_buff, tmpl_size, *pprog, > > + apply_relocation(insn_buff, tmpl_size, ip, > > Did the logic inside apply_relocation() change to have > a new meaning for 'dest' and 'src'? > Answering to myself... yes. in that commit. > Better commit log would have made the code review so much easier. > > > skl_call_thunk_template, tmpl_size); > > > > memcpy(*pprog, insn_buff, tmpl_size); > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > > index 09f7dc9d4d65..f2e8769f5eee 100644 > > --- a/arch/x86/net/bpf_jit_comp.c > > +++ b/arch/x86/net/bpf_jit_comp.c > > @@ -481,7 +481,7 @@ static int emit_rsb_call(u8 **pprog, void *func, void *ip) > > { > > void *adjusted_ip; > > OPTIMIZER_HIDE_VAR(func); > > - adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); > > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func, ip); > > Now I see why you added extra var in the previous patch. > Should have mentioned it in the commit log. > > > return emit_patch(pprog, func, adjusted_ip, 0xE8); > > } > > > > @@ -1973,20 +1973,17 @@ st: if (is_imm8(insn->off)) > > > > /* call */ > > case BPF_JMP | BPF_CALL: { > > - int offs; > > + u8 *ip = image + addrs[i - 1]; > > > > func = (u8 *) __bpf_call_base + imm32; > > if (tail_call_reachable) { > > RESTORE_TAIL_CALL_CNT(bpf_prog->aux->stack_depth); > > - if (!imm32) > > - return -EINVAL; > > - offs = 7 + x86_call_depth_emit_accounting(&prog, func); > > - } else { > > - if (!imm32) > > - return -EINVAL; > > - offs = x86_call_depth_emit_accounting(&prog, func); > > + ip += 7; > > } > > - if (emit_call(&prog, func, image + addrs[i - 1] + offs)) > > + if (!imm32) > > + return -EINVAL; > > + ip += x86_call_depth_emit_accounting(&prog, func, ip); > > + if (emit_call(&prog, func, ip)) > > return -EINVAL; > > break; > > } > > @@ -2836,7 +2833,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im > > * Direct-call fentry stub, as such it needs accounting for the > > * __fentry__ call. > > */ > > - x86_call_depth_emit_accounting(&prog, NULL); > > + x86_call_depth_emit_accounting(&prog, NULL, image); > > Overall it all makes sense. > Pls respin with more precise commit logs. X-sender: <linux-kernel+bounces-125632-steffen.klassert=secunet.com@vger.kernel.org> X-Receiver: <steffen.klassert@secunet.com> ORCPT=rfc822;steffen.klassert@secunet.com X-CreatedBy: MSExchange15 X-HeloDomain: mbx-dresden-01.secunet.de X-ExtendedProps: BQBjAAoATIOmlidQ3AgFADcAAgAADwA8AAAATWljcm9zb2Z0LkV4Y2hhbmdlLlRyYW5zcG9ydC5NYWlsUmVjaXBpZW50Lk9yZ2FuaXphdGlvblNjb3BlEQAAAAAAAAAAAAAAAAAAAAAADwA/AAAATWljcm9zb2Z0LkV4Y2hhbmdlLlRyYW5zcG9ydC5EaXJlY3RvcnlEYXRhLk1haWxEZWxpdmVyeVByaW9yaXR5DwADAAAATG93 X-Source: SMTP:Default MBX-ESSEN-02 X-SourceIPAddress: 10.53.40.199 X-EndOfInjectedXHeaders: 16859 Received: from mbx-dresden-01.secunet.de (10.53.40.199) by mbx-essen-02.secunet.de (10.53.40.198) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.37; Sat, 30 Mar 2024 10:01:43 +0100 Received: from b.mx.secunet.com (62.96.220.37) by cas-essen-01.secunet.de (10.53.40.201) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Sat, 30 Mar 2024 10:01:43 +0100 Received: from localhost (localhost [127.0.0.1]) by b.mx.secunet.com (Postfix) with ESMTP id 10A2520315 for <steffen.klassert@secunet.com>; Sat, 30 Mar 2024 10:01:43 +0100 (CET) X-Virus-Scanned: by secunet X-Spam-Flag: NO X-Spam-Score: -5.049 X-Spam-Level: X-Spam-Status: No, score=-5.049 tagged_above=-999 required=2.1 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, FREEMAIL_FORGED_FROMDOMAIN=0.001, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.249, MAILING_LIST_MULTI=-1, RCVD_IN_DNSWL_MED=-2.3, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no Authentication-Results: a.mx.secunet.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from b.mx.secunet.com ([127.0.0.1]) by localhost (a.mx.secunet.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id B8ZWMzr-RJt4 for <steffen.klassert@secunet.com>; Sat, 30 Mar 2024 10:01:42 +0100 (CET) Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip\x139.178.88.99; helo=sv.mirrors.kernel.org; envelope-from=linux-kernel+bounces-125632-steffen.klassert=secunet.com@vger.kernel.org; receiver=steffen.klassert@secunet.com DKIM-Filter: OpenDKIM Filter v2.11.0 b.mx.secunet.com A420F2025D Authentication-Results: b.mx.secunet.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Nle+vzJj" Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org [139.178.88.99]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by b.mx.secunet.com (Postfix) with ESMTPS id A420F2025D for <steffen.klassert@secunet.com>; Sat, 30 Mar 2024 10:01:41 +0100 (CET) Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 9FF4D282F2A for <steffen.klassert@secunet.com>; Sat, 30 Mar 2024 09:01:39 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id D1AE9DF5A; Sat, 30 Mar 2024 09:01:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Nle+vzJj" Received: from mail-lj1-f171.google.com (mail-lj1-f171.google.com [209.85.208.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3F2F7847E; Sat, 30 Mar 2024 09:01:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip 9.85.208.171 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t\x1711789276; cv=none; b=LJliKGtxdDA5DncUU3WQ86IqJfxxDdmZL62lReMe091FMMVzSAkVNUab0eixlHeqVTjIQ9EEtdUySkdcLp6rFtixRUTTmQdeeFsygIBsVZzOYDo25RMQNI1crLiHrVMDdGnqz68Wu6CyxrARRiiLwfKV6AS/7WwNSvW09W60zckARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t\x1711789276; c=relaxed/simple; bhÏUfaZfb7X/JJv0DB5cn0CBJrMrECWzMZrlMK1UCZYA=; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject: To:Cc:Content-Type; b=o3BMwVDUvqhNVOZ/cIBlL/eEF8qwRIumKR1fMWrSA7H4zcSOpuQv/ioQzXQqrodsIUyARBuvkzwtgX1F3ahG1oBV9JsqyrWjN58qqzsylGWNrONt419EuA+HV2Hnb6GN/l1yLYA/QgPCW6C6kp/GDTzIPXK00zlXkPjIpLmtnu4ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Nle+vzJj; arc=none smtp.client-ip 9.85.208.171 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lj1-f171.google.com with SMTP id 38308e7fff4ca-2d6ee6c9945so20258311fa.3; Sat, 30 Mar 2024 02:01:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s 230601; t\x1711789272; x\x1712394072; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bhËl/CLMq3nWKqNjaubWInMz1kaQBbZVVwZ4B6Yhcsik=; b=Nle+vzJjShWs0Ti3+DNi47la3wp5ECIHDAW+EvbhX+Ton8pxr5GAT+OKLN9tkpsdWp 7pmY92Vv0JN9muB7m6ZKKOed2eTaDIbVC7yQe3iYoQYmuzNt6FG/2CRXCn5axo5T0Er+ mIawl+AjZ9LcJ+2LwfqLqtGS9Swzi333W06D5Rkope/9r9oRpQjGT7vy46uzFz6Xcl6b S77spLQ2G+CQpl+4BKwdDBzhD3S36TpnXk6mrY+NzXkbO3VsCQIQvMCBSh38nAhwLdwg DDcnS4Gu8cVLbgH/tpVDy41vXbLuY8G8wYbQmHDkXrHBBWVRq9R4WCn8orBArD2q7glw BXGA=X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d\x1e100.net; s 230601; t\x1711789272; x\x1712394072; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bhËl/CLMq3nWKqNjaubWInMz1kaQBbZVVwZ4B6Yhcsik=; b=SissUbPdoWrUM0u1xqTsea9iktJot0W8ikz/21w6oDLw29oHo1E/zyykrKQer/3OWO qPdKEegxJqdMg3v5GqOnOKWhyGLwFbh6p2b85j1Ts+6BbLFl9oFkAqze+79DH3UTgvpX 8eHg+dcra2W81v3T0kNwO/Rz19x/a4qUnJoY4xXfsGku0ULlQwIKn/HG4uSfeFFaQsvQ uTzuZm8A/9h9KcYqL8SvAcdlpvE/AK5/d243lGtC7Es8bpk5HpBF6YLgEqB9CQU7scmf XLKGETg+gvfwQ/3vIK2nDBH9GUewEyTA+wZonkJx4DDqfRJaO4cU3mQBB6idP5IB1mep Hw+g=X-Forwarded-Encrypted: i=1; AJvYcCXMPjsPXXatSVDH641eiZD9Tylx5ujw2zvU8pLsO9HYR1XJhfBDwIOO8cDOnkZQvsFw51iNGrNYsj+ZOC9s/nmtelqQeBg/s4CmUPCyx03PDGJu4fX5flrhGr2i0fu9s/8eCySMSfdAa696W6WVC4RItEDieQnLUtLc X-Gm-Message-State: AOJu0YxogawIyIXt1FmwtLXKYPIfA7/NqcoJp2+CkbXskVq401KWqBS9 dv/uHIbm1VAuF/Vdv+59bCR/JaF94WfnbXteVNB+T1wBqWVEkaHONlAM1xQ2s4fdMV8thx39cxA tPcarlYh9vw0yYnOBdeRY+OsGAlIX-Google-Smtp-Source: AGHT+IGeaymGC7cAQwieQg4TakZq0ARBnpQFix3PExtV6ez87eomevGZHhs6eDxl1CtxiB/HrZm8Uq2a8idogtbdJIcX-Received: by 2002:a2e:8612:0:b0:2d6:a5ce:b261 with SMTP id a18-20020a2e8612000000b002d6a5ceb261mr2175202lji.25.1711789271815; Sat, 30 Mar 2024 02:01:11 -0700 (PDT) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 References: <20240329094906.18147-1-ubizjak@gmail.com> <20240329094906.18147-3-ubizjak@gmail.com> <CAADnVQ+6D++hCXaP=aK+Q5wienMzhHo3h9YCvpA_7sHjMt+q6A@mail.gmail.com> In-Reply-To: <CAADnVQ+6D++hCXaP=aK+Q5wienMzhHo3h9YCvpA_7sHjMt+q6A@mail.gmail.com> From: Uros Bizjak <ubizjak@gmail.com> Date: Sat, 30 Mar 2024 10:01:04 +0100 Message-ID: <CAFULd4b6juiw3wC3Z61V9=-UnA+NGyUt4231vC14UnGAATk6tA@mail.gmail.com> Subject: Re: [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting To: Alexei Starovoitov <alexei.starovoitov@gmail.com> Cc: X86 ML <x86@kernel.org>, bpf <bpf@vger.kernel.org>, Network Development <netdev@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>, =?UTF-8?Q?Joan_Bruguera_Micó?= <joanbrugueram@gmail.com>, Ingo Molnar <mingo@kernel.org>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Return-Path: linux-kernel+bounces-125632-steffen.klassert=secunet.com@vger.kernel.org X-MS-Exchange-Organization-OriginalArrivalTime: 30 Mar 2024 09:01:43.0950 (UTC) X-MS-Exchange-Organization-Network-Message-Id: 728bc2a0-e193-4119-8421-08dc5098051f X-MS-Exchange-Organization-OriginalClientIPAddress: 62.96.220.37 X-MS-Exchange-Organization-OriginalServerIPAddress: 10.53.40.201 X-MS-Exchange-Organization-Cross-Premises-Headers-Processed: cas-essen-01.secunet.de X-MS-Exchange-Organization-OrderedPrecisionLatencyInProgress: LSRVÊs-essen-01.secunet.de:TOTAL-FE=0.010|SMR=0.009(SMRPI=0.007(SMRPI-FrontendProxyAgent=0.007));2024-03-30T09:01:43.105Z X-MS-Exchange-Forest-ArrivalHubServer: mbx-essen-02.secunet.de X-MS-Exchange-Organization-AuthSource: cas-essen-01.secunet.de X-MS-Exchange-Organization-AuthAs: Anonymous X-MS-Exchange-Organization-OriginalSize: 16312 X-MS-Exchange-Organization-Transport-Properties: DeliveryPriority=Low X-MS-Exchange-Organization-Prioritization: 2:ShadowRedundancy X-MS-Exchange-Organization-IncludeInSla: False:ShadowRedundancy On Fri, Mar 29, 2024 at 10:53 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > > > The recently introduced support for %rip-relative relocations in the > > call thunk template assumes that the code is being patched in-place, > > so the destination of the relocation matches the address of the code. > > This is not true for the call depth accounting emitted by the BPF JIT, > > so the calculated address is wrong and usually causes a page fault. > > Could you share the link to what this 'rip-relative' relocation is ? Please see the "RIP relative addressing" section in [1]. [1] https://compas.cs.stonybrook.edu/~nhonarmand/courses/sp17/cse506/ref/assembly.html In our case: The callthunks patching creates a call thunk template in the .rodata section (please see arch/x86/kernel/callthunks.c) that is later copied to the .text section at the correct place. The template uses X86_call_depth in the pcpu_hot structure. Previously, the template used absolute location for X86_call_depth and the linker resolved the address in the template to this absolute location. There is no issue when this template is copied to the various places in the .text section. When we want to use PC relative relocations (to reduce the code size), then the linker calculates the address of the variable in the template according to the PC in the .rodata section. If we want to copy the template to its final location, then the address of X86_call_depth, relative to the PC, has to be adjusted, as explained in arch/x86/kernel/alternative.c, in the comment above apply_reloc_n macro. Uros. > > Pass the destination IP when the BPF JIT emits call depth accounting. > > > > Fixes: 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative relocations in call thunk template") > > Ohh. It's buried inside that patch. > Pls make commit log a bit more clear that that commit 17bce3b2ae2d > broke x86_call_depth_emit_accounting logic. > > > Signed-off-by: Joan Bruguera Micó <joanbrugueram@gmail.com> > > Reviewed-by: Uros Bizjak <ubizjak@gmail.com> > > Acked-by: Ingo Molnar <mingo@kernel.org> > > Cc: Alexei Starovoitov <ast@kernel.org> > > Cc: Daniel Borkmann <daniel@iogearbox.net> > > --- > > arch/x86/include/asm/alternative.h | 4 ++-- > > arch/x86/kernel/callthunks.c | 4 ++-- > > arch/x86/net/bpf_jit_comp.c | 19 ++++++++----------- > > 3 files changed, 12 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h > > index fcd20c6dc7f9..67b68d0d17d1 100644 > > --- a/arch/x86/include/asm/alternative.h > > +++ b/arch/x86/include/asm/alternative.h > > @@ -117,7 +117,7 @@ extern void callthunks_patch_builtin_calls(void); > > extern void callthunks_patch_module_calls(struct callthunk_sites *sites, > > struct module *mod); > > extern void *callthunks_translate_call_dest(void *dest); > > -extern int x86_call_depth_emit_accounting(u8 **pprog, void *func); > > +extern int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip); > > #else > > static __always_inline void callthunks_patch_builtin_calls(void) {} > > static __always_inline void > > @@ -128,7 +128,7 @@ static __always_inline void *callthunks_translate_call_dest(void *dest) > > return dest; > > } > > static __always_inline int x86_call_depth_emit_accounting(u8 **pprog, > > - void *func) > > + void *func, void *ip) > > { > > return 0; > > } > > diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c > > index 30335182b6b0..e92ff0c11db8 100644 > > --- a/arch/x86/kernel/callthunks.c > > +++ b/arch/x86/kernel/callthunks.c > > @@ -314,7 +314,7 @@ static bool is_callthunk(void *addr) > > return !bcmp(pad, insn_buff, tmpl_size); > > } > > > > -int x86_call_depth_emit_accounting(u8 **pprog, void *func) > > +int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip) > > { > > unsigned int tmpl_size = SKL_TMPL_SIZE; > > u8 insn_buff[MAX_PATCH_LEN]; > > @@ -327,7 +327,7 @@ int x86_call_depth_emit_accounting(u8 **pprog, void *func) > > return 0; > > > > memcpy(insn_buff, skl_call_thunk_template, tmpl_size); > > - apply_relocation(insn_buff, tmpl_size, *pprog, > > + apply_relocation(insn_buff, tmpl_size, ip, > > Did the logic inside apply_relocation() change to have > a new meaning for 'dest' and 'src'? > Answering to myself... yes. in that commit. > Better commit log would have made the code review so much easier. > > > skl_call_thunk_template, tmpl_size); > > > > memcpy(*pprog, insn_buff, tmpl_size); > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > > index 09f7dc9d4d65..f2e8769f5eee 100644 > > --- a/arch/x86/net/bpf_jit_comp.c > > +++ b/arch/x86/net/bpf_jit_comp.c > > @@ -481,7 +481,7 @@ static int emit_rsb_call(u8 **pprog, void *func, void *ip) > > { > > void *adjusted_ip; > > OPTIMIZER_HIDE_VAR(func); > > - adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func); > > + adjusted_ip = ip + x86_call_depth_emit_accounting(pprog, func, ip); > > Now I see why you added extra var in the previous patch. > Should have mentioned it in the commit log. > > > return emit_patch(pprog, func, adjusted_ip, 0xE8); > > } > > > > @@ -1973,20 +1973,17 @@ st: if (is_imm8(insn->off)) > > > > /* call */ > > case BPF_JMP | BPF_CALL: { > > - int offs; > > + u8 *ip = image + addrs[i - 1]; > > > > func = (u8 *) __bpf_call_base + imm32; > > if (tail_call_reachable) { > > RESTORE_TAIL_CALL_CNT(bpf_prog->aux->stack_depth); > > - if (!imm32) > > - return -EINVAL; > > - offs = 7 + x86_call_depth_emit_accounting(&prog, func); > > - } else { > > - if (!imm32) > > - return -EINVAL; > > - offs = x86_call_depth_emit_accounting(&prog, func); > > + ip += 7; > > } > > - if (emit_call(&prog, func, image + addrs[i - 1] + offs)) > > + if (!imm32) > > + return -EINVAL; > > + ip += x86_call_depth_emit_accounting(&prog, func, ip); > > + if (emit_call(&prog, func, ip)) > > return -EINVAL; > > break; > > } > > @@ -2836,7 +2833,7 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im, void *rw_im > > * Direct-call fentry stub, as such it needs accounting for the > > * __fentry__ call. > > */ > > - x86_call_depth_emit_accounting(&prog, NULL); > > + x86_call_depth_emit_accounting(&prog, NULL, image); > > Overall it all makes sense. > Pls respin with more precise commit logs. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting 2024-03-30 9:01 ` Uros Bizjak 2024-03-30 9:01 ` Uros Bizjak 2024-03-30 9:01 ` Uros Bizjak @ 2024-04-01 18:02 ` Alexei Starovoitov 2024-04-01 18:38 ` Uros Bizjak 2 siblings, 1 reply; 12+ messages in thread From: Alexei Starovoitov @ 2024-04-01 18:02 UTC (permalink / raw) To: Uros Bizjak Cc: X86 ML, bpf, Network Development, LKML, Joan Bruguera Micó, Ingo Molnar, Alexei Starovoitov, Daniel Borkmann On Sat, Mar 30, 2024 at 2:01 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > On Fri, Mar 29, 2024 at 10:53 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > > > > > The recently introduced support for %rip-relative relocations in the > > > call thunk template assumes that the code is being patched in-place, > > > so the destination of the relocation matches the address of the code. > > > This is not true for the call depth accounting emitted by the BPF JIT, > > > so the calculated address is wrong and usually causes a page fault. > > > > Could you share the link to what this 'rip-relative' relocation is ? > > Please see the "RIP relative addressing" section in [1]. > > [1] https://compas.cs.stonybrook.edu/~nhonarmand/courses/sp17/cse506/ref/assembly.html > > In our case: > > The callthunks patching creates a call thunk template in the .rodata > section (please see arch/x86/kernel/callthunks.c) that is later > copied to the .text section at the correct place. The template uses > X86_call_depth in the pcpu_hot structure. Previously, the template > used absolute location for X86_call_depth and the linker resolved the > address in the template to this absolute location. There is no issue > when this template is copied to the various places in the .text > section. > > When we want to use PC relative relocations (to reduce the code size), > then the linker calculates the address of the variable in the template > according to the PC in the .rodata section. If we want to copy the > template to its final location, then the address of X86_call_depth, > relative to the PC, has to be adjusted, as explained in > arch/x86/kernel/alternative.c, in the comment above apply_reloc_n > macro. I didn't mean to ask for info about the definition of rip-relative, but how it's used in this case and what you've been trying to achieve with commit 17bce3b2ae2d that broke call depth accounting. And the whole sequence of events that caused this breakage. Something like: commit 59bec00ace28 ("x86/percpu: Introduce %rip-relative addressing to PER_CPU_VAR()") made PER_CPU_VAR() to use rip-relative addressing, hence INCREMENT_CALL_DEPTH macro and skl_call_thunk_template got rip-relative asm code inside of it. Hence x86_call_depth_emit_accounting() was changed in commit 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative relocations in call thunk template") to use apply_relocation(), but it was mistakenly made to use *pprog as dest ip, so jit-ed bpf progs on kernels with call depth tracking got broken. Such details should be in the commit log. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating call depth accounting 2024-04-01 18:02 ` Alexei Starovoitov @ 2024-04-01 18:38 ` Uros Bizjak 0 siblings, 0 replies; 12+ messages in thread From: Uros Bizjak @ 2024-04-01 18:38 UTC (permalink / raw) To: Alexei Starovoitov Cc: X86 ML, bpf, Network Development, LKML, Joan Bruguera Micó, Ingo Molnar, Alexei Starovoitov, Daniel Borkmann On Mon, Apr 1, 2024 at 8:03 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Sat, Mar 30, 2024 at 2:01 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > On Fri, Mar 29, 2024 at 10:53 PM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> wrote: > > > > > > On Fri, Mar 29, 2024 at 2:49 AM Uros Bizjak <ubizjak@gmail.com> wrote: > > > > > > > > From: Joan Bruguera Micó <joanbrugueram@gmail.com> > > > > > > > > The recently introduced support for %rip-relative relocations in the > > > > call thunk template assumes that the code is being patched in-place, > > > > so the destination of the relocation matches the address of the code. > > > > This is not true for the call depth accounting emitted by the BPF JIT, > > > > so the calculated address is wrong and usually causes a page fault. > > > > > > Could you share the link to what this 'rip-relative' relocation is ? > > > > Please see the "RIP relative addressing" section in [1]. > > > > [1] https://compas.cs.stonybrook.edu/~nhonarmand/courses/sp17/cse506/ref/assembly.html > > > > In our case: > > > > The callthunks patching creates a call thunk template in the .rodata > > section (please see arch/x86/kernel/callthunks.c) that is later > > copied to the .text section at the correct place. The template uses > > X86_call_depth in the pcpu_hot structure. Previously, the template > > used absolute location for X86_call_depth and the linker resolved the > > address in the template to this absolute location. There is no issue > > when this template is copied to the various places in the .text > > section. > > > > When we want to use PC relative relocations (to reduce the code size), > > then the linker calculates the address of the variable in the template > > according to the PC in the .rodata section. If we want to copy the > > template to its final location, then the address of X86_call_depth, > > relative to the PC, has to be adjusted, as explained in > > arch/x86/kernel/alternative.c, in the comment above apply_reloc_n > > macro. > > I didn't mean to ask for info about the definition of rip-relative, > but how it's used in this case and what you've been trying > to achieve with commit 17bce3b2ae2d that broke call depth accounting. > And the whole sequence of events that caused this breakage. > Something like: > commit 59bec00ace28 ("x86/percpu: Introduce %rip-relative addressing > to PER_CPU_VAR()") > made PER_CPU_VAR() to use rip-relative addressing, > hence INCREMENT_CALL_DEPTH macro and skl_call_thunk_template > got rip-relative asm code inside of it. > Hence x86_call_depth_emit_accounting() was changed > in commit 17bce3b2ae2d ("x86/callthunks: Handle %rip-relative > relocations in call thunk template") to use apply_relocation(), > but it was mistakenly made to use *pprog as dest ip, > so jit-ed bpf progs on kernels with call depth tracking got broken. > Such details should be in the commit log. Oh, I was not sure that all those x86 specific details should be in the commit log, since x86 maintainers already acked the patch. Sure, I'll add your description of the fix to the patch commit message, it really describes the problem in a way, understandable also to non-x86 people. Thanks, Uros. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-04-01 18:38 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-29 9:46 [PATCH RESEND bpf 0/2] x86/bpf: Fixes for the BPF JIT with retbleed=stuff Uros Bizjak 2024-03-29 9:46 ` [PATCH RESEND bpf 1/2] x86/bpf: Fix IP after emitting call depth accounting Uros Bizjak 2024-03-29 21:26 ` Alexei Starovoitov 2024-03-29 21:26 ` Alexei Starovoitov 2024-03-29 9:46 ` [PATCH RESEND bpf 2/2] x86/bpf: Fix IP for relocating " Uros Bizjak 2024-03-29 21:53 ` Alexei Starovoitov 2024-03-29 21:53 ` Alexei Starovoitov 2024-03-30 9:01 ` Uros Bizjak 2024-03-30 9:01 ` Uros Bizjak 2024-03-30 9:01 ` Uros Bizjak 2024-04-01 18:02 ` Alexei Starovoitov 2024-04-01 18:38 ` Uros Bizjak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).