Received: by 2002:a05:6358:1087:b0:cb:c9d3:cd90 with SMTP id j7csp3351260rwi; Fri, 21 Oct 2022 15:19:22 -0700 (PDT) X-Google-Smtp-Source: AMsMyM66Gmp+BEL5AlO/e0xF+0fJ9E4O6fxKBucOVSl3Gpz7lrd8RzULczfMOY3mdTNuezI2A5q7 X-Received: by 2002:aa7:de81:0:b0:461:2122:a807 with SMTP id j1-20020aa7de81000000b004612122a807mr7918019edv.382.1666390762523; Fri, 21 Oct 2022 15:19:22 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666390762; cv=none; d=google.com; s=arc-20160816; b=DWkO6LJrDDuiwYjqU+7mlN+P2Imjvpc/BE6t2hW1k2eUJjAIPBuNQ4CVFi8HCer0ah 7a6QLH6c+FZ6Mv0VIIHYDlc1QcVimMz60sG43QImf1aUysIRkSbwhny0Jb/fCPtmxSl9 LuH+FM8aDi9JgoKexaUNTRjTszxduCZw4DA1F+sYXzunESVhv9AKL6qEvaWb02KKFT6e FEBatkTjvsC5CUEq1+BCbSJudz0D2Mb26GwKIRwl6f+HEL5Q1nATPx6xUAWUzg4u8Xst A6Y2YtGaeOdWYcgLmFNuRd0vA+GoZB6cOnDMYR3SQocEfCUoIlAGJOfZub3G2AC1RUu3 E66w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=6lz31d9qvODy/+YzXxTbbN3fpreRhI0QwWPeqaSVENc=; b=P99xh6k44JE4WNTGQAXpLQJuGVgHjYtaYE5HTUuFyKuKypZ4pPfsYyXGU1OozlbK3E 3LOjdxSLtrfD1dEQvXkqr+hhcf2gQDFKDXoSfWr8/wTBNbUHrux4we8tCeCwLTMWTcMP GJeR2PFoFPBnfCZkzGcUcUGG8wiJo4fN4NdfrtKy71pc4iW6D/WOXGrJd9Obt8mWTXXd Us628uuILkiQQRSfK78MeDwNhC2zNUNitSqA75ZEb1mh7A6YwSSimLcpcpi5/2cj8oOV TWmL2uTCcBQBBISKfvaB087PAVAFhI3FGeNFDVW4e6r5jkesix9r2Bw/RhSuxEm/kmZJ UMtA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=hkRepDIy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id e14-20020a056402148e00b00459e39448b3si18548848edv.254.2022.10.21.15.18.57; Fri, 21 Oct 2022 15:19:22 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=hkRepDIy; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229875AbiJUVth (ORCPT + 99 others); Fri, 21 Oct 2022 17:49:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41470 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229835AbiJUVtd (ORCPT ); Fri, 21 Oct 2022 17:49:33 -0400 Received: from mail-ed1-x535.google.com (mail-ed1-x535.google.com [IPv6:2a00:1450:4864:20::535]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3D1ADD11C; Fri, 21 Oct 2022 14:49:28 -0700 (PDT) Received: by mail-ed1-x535.google.com with SMTP id m16so10598836edc.4; Fri, 21 Oct 2022 14:49:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=6lz31d9qvODy/+YzXxTbbN3fpreRhI0QwWPeqaSVENc=; b=hkRepDIyCPQlTuG3ehlzYNbZ05cgs5JRUHl7KdWDqLGeLR0DY1MmrF23IVNjoty/sY 8JDyZvFvIzlV6vdfl751Zk9zKAQ5aRKiWXjLrbLn2QQykH1p9WseF7l4qN2FUeZhPNIx K0XOpY+hnIyeAuNl0VdOB0Mi150TLCGhVU6h5BhC0VW216sTfI3pGszG9wfnUXs8sTRB TIsexCCg5tLzZn2BCNT4yZPsYFWHDQC6tmNv1xzRU4zalnlkkEm/yZvq/qtRrbZGaByY g2OOSEo/OQHPouNmV18UV4fCXePx++QqDh5tFnVBbWyFsEO1765ADL73IbZ847zrpZZ3 hrMg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=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=6lz31d9qvODy/+YzXxTbbN3fpreRhI0QwWPeqaSVENc=; b=UYl5BDcGd3m5sezxcsIBM7WS3OoTJ+Dc+OHQglpbkomLSFLEc7M+F2LOJBkznyvs8/ d3EINz7GrhgWcHFPhIeTReFMxbg2mWsfdXtwqtw9E4fa/BwHLhwyP7eyFDS29TSuZQN0 2U2Gx9rqbpKvJk3Vt2BXL3fFv57NaItir0goyKOGlqE1IVpZb9Ur6P9JxHz9ZW+I4d7Q XlRSRvMD2tRta+aajBHC/Uolr8xLWihC8UFjrs24j05HBkQ2heKK9yt4/UBL7dwQEx6x vbGhXiPHcjeC+a6szdJybopu+ddjf5trrwR/J8J4n79DWPk0243Fpl+HIM3LDHcOR38t 8QEw== X-Gm-Message-State: ACrzQf3HGjlHOsdth1nPDv3nLBkfxS6UazQZIZrcCrpCj7kaGWdHSlRX gDPi6rP1ZMlXvjrqrftSGwi4NotVfLK7a1P4fjE= X-Received: by 2002:a17:907:2cca:b0:78d:ec48:ac29 with SMTP id hg10-20020a1709072cca00b0078dec48ac29mr17253005ejc.114.1666388966591; Fri, 21 Oct 2022 14:49:26 -0700 (PDT) MIME-Version: 1.0 References: <20221018122708.823792-1-jolsa@kernel.org> In-Reply-To: From: Andrii Nakryiko Date: Fri, 21 Oct 2022 14:49:14 -0700 Message-ID: Subject: Re: [PATCH] x86: Include asm/ptrace.h in syscall_wrapper header To: Jiri Olsa Cc: sdf@google.com, Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Peter Zijlstra , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , Andy Lutomirski , Akihiro HARAI , bpf@vger.kernel.org, linux-kernel@vger.kernel.org, Martin KaFai Lau , Song Liu , Yonghong Song , John Fastabend , KP Singh , Hao Luo Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 19, 2022 at 2:30 AM Jiri Olsa wrote: > > On Tue, Oct 18, 2022 at 11:23:21AM -0700, sdf@google.com wrote: > > On 10/18, Jiri Olsa wrote: > > > From: Jiri Olsa > > > > > With just the forward declaration of the 'struct pt_regs' in > > > syscall_wrapper.h, the syscall stub functions: > > > > > __[x64|ia32]_sys_*(struct pt_regs *regs) > > > > > will have different definition of 'regs' argument in BTF data > > > based on which object file they are defined in. > > > > > If the syscall's object includes 'struct pt_regs' definition, > > > the BTF argument data will point to 'struct pt_regs' record, > > > like: > > > > > [226] STRUCT 'pt_regs' size=168 vlen=21 > > > 'r15' type_id=1 bits_offset=0 > > > 'r14' type_id=1 bits_offset=64 > > > 'r13' type_id=1 bits_offset=128 > > > ... > > > > > If not, it will point to fwd declaration record: > > > > > [15439] FWD 'pt_regs' fwd_kind=struct > > > > > and make bpf tracing program hooking on those functions unable > > > to access fields from 'struct pt_regs'. > > > > Is the core issue here is that we can't / don't resolve FWD declarations > > at load time (or dedup time)? I'm assuming 'struct pt_regs' is still > > exposed somewhere in BTF via some other obj file, it's just those > > syscall definitions that have FWD decl? > > yes, BTF is generated from dwarf debug info, so it's object based, > and in some objects the regs argument will point to full struct > definition and in some just to forward declaration > > > > > Applying this patch seems fine for now, but also seems fragile. Should > > we instead/also teach verifier/dedup/whatever to resolve those FWD > > declarations? > > I'm not sure how hard it'd be to connect forward declarations > to definitions.. it'd need to be probably in dedup or verifier > as you suggest > > and I think it'd need to be done just based on struct name search, > so I expect all sort of unexpected problems ;-) other than to be > sure you connect to proper struct exactly, which is why BTF dedup algorithm resolves FWD to STRUCT/UNION only when we have to outer structs/unions that we are deduplicating, and one of their field points to FWD on one side and STRUCT on another side. Looking up by name is ambiguous and could be incorrect, so BTF dedup needs sort of a "structural proof". As far as the patch goes, it's a good thing to have more complete types wherever possible, so: Acked-by: Andrii Nakryiko > > Andrii will have probably better idea if and where this is possible > > jirka > > > > > > Including asm/ptrace.h directly in syscall_wrapper.h to make > > > sure all syscalls see 'struct pt_regs' definition and resulted > > > BTF for '__*_sys_*(struct pt_regs *regs)' functions point to > > > actual struct, not just forward declaration. > > > > > Reported-by: Akihiro HARAI > > > Signed-off-by: Jiri Olsa > > > --- > > > arch/x86/include/asm/syscall_wrapper.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > diff --git a/arch/x86/include/asm/syscall_wrapper.h > > > b/arch/x86/include/asm/syscall_wrapper.h > > > index 59358d1bf880..fd2669b1cb2d 100644 > > > --- a/arch/x86/include/asm/syscall_wrapper.h > > > +++ b/arch/x86/include/asm/syscall_wrapper.h > > > @@ -6,7 +6,7 @@ > > > #ifndef _ASM_X86_SYSCALL_WRAPPER_H > > > #define _ASM_X86_SYSCALL_WRAPPER_H > > > > > -struct pt_regs; > > > +#include > > > > > extern long __x64_sys_ni_syscall(const struct pt_regs *regs); > > > extern long __ia32_sys_ni_syscall(const struct pt_regs *regs); > > > -- > > > 2.37.3 > >