From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-181.mta1.migadu.com (out-181.mta1.migadu.com [95.215.58.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3D6A41CD35 for ; Thu, 25 Jul 2024 05:55:13 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.181 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721886915; cv=none; b=AotpYwUP7Ep49TMN/IWy99dBtP46dg4gcNi0xGC2Zphud8I8aPtLBHgJgXI9P638l1jwSCl6SoMBVikrMK4J0INszw0crSNjXdqWFDbo7vIqqUxDV4m4wU8TnbweLyC4YgXLngg98IHvpRy7YD6N1YFqSyjKfphNZk/33a4TPz0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1721886915; c=relaxed/simple; bh=xuzbDE52Bn6g9Euv+PJQJSS0ckZ6+0Rbj8cvnQofx9M=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=LBYikuoGq0nNHWItABSve8UOfsII/rUdIPmyKhtqsIuv6BCYrqcV1x2QzufTqRLP+sNeQmI/udVLYjUI3iJFwUVK20++f9yswVMlkFz6TGW6NLk2knKO0fZXmK9+Gr6NkcyxqxHiaalOEo/rqx9cWKC2S6jvOfFFnCjzmDBkFPE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=jxrq+YPM; arc=none smtp.client-ip=95.215.58.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="jxrq+YPM" Message-ID: <08e180da-e841-427d-bed6-3ba8d73e8519@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1721886910; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=OzZHqkug/79QkjvohWjTIiykDjrWCF7qe3jwRF2k0Zc=; b=jxrq+YPMUDN/EtNcSM8Vr6FrUYeofEuJoGyd1CjZ7tXTeUt6kNrNjdfaJD4eEmtkdCxdk/ EjwjCQri7eAAqPAGFXmt+5ao20q5FEcrCaaBVHPs/J2/j0JABoos8SC91XnUTGzvVqqqXb euIV3+jiqz+VCU+FRKpQnnCDwLaWfLg= Date: Wed, 24 Jul 2024 22:54:53 -0700 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH bpf-next v2] bpf: Add bpf_check_attach_target_with_klog method to output failure logs to kernel Content-Language: en-GB To: Zheao Li , Alexei Starovoitov , Daniel Borkmann , John Fastabend , Andrii Nakryiko , Martin KaFai Lau , Eduard Zingerman , Song Liu , KP Singh , Stanislav Fomichev , Hao Luo , Jiri Olsa Cc: Leon Hwang , bpf@vger.kernel.org, linux-kernel@vger.kernel.org References: <20240725051511.57112-1-me@manjusaka.me> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Yonghong Song In-Reply-To: <20240725051511.57112-1-me@manjusaka.me> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT On 7/24/24 10:15 PM, Zheao Li wrote: > This is a v2 patch, previous Link: https://lore.kernel.org/bpf/20240724152521.20546-1-me@manjusaka.me/T/#u > > Compare with v1: > > 1. Format the code style and signed-off field > 2. Use a shorter name bpf_check_attach_target_with_klog instead of > original name bpf_check_attach_target_with_kernel_log > > When attaching a freplace hook, failures can occur, > but currently, no output is provided to help developers diagnose the root cause. > > This commit adds a new method, bpf_check_attach_target_with_klog, > which outputs the verifier log to the kernel. > Developers can then use dmesg to obtain more detailed information about the failure. > > For an example of eBPF code, > Link: https://github.com/Asphaltt/learn-by-example/blob/main/ebpf/freplace/main.go > > Co-developed-by: Leon Hwang > Signed-off-by: Leon Hwang > Signed-off-by: Zheao Li > --- > include/linux/bpf_verifier.h | 5 +++++ > kernel/bpf/syscall.c | 5 +++-- > kernel/bpf/trampoline.c | 6 +++--- > kernel/bpf/verifier.c | 19 +++++++++++++++++++ > 4 files changed, 30 insertions(+), 5 deletions(-) > > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h > index 5cea15c81b8a..8eddba62c194 100644 > --- a/include/linux/bpf_verifier.h > +++ b/include/linux/bpf_verifier.h > @@ -848,6 +848,11 @@ static inline void bpf_trampoline_unpack_key(u64 key, u32 *obj_id, u32 *btf_id) > *btf_id = key & 0x7FFFFFFF; > } > > +int bpf_check_attach_target_with_klog(const struct bpf_prog *prog, > + const struct bpf_prog *tgt_prog, > + u32 btf_id, > + struct bpf_attach_target_info *tgt_info); format issue in the above. Same code alignment is needed for arguments in different lines. > + > int bpf_check_attach_target(struct bpf_verifier_log *log, > const struct bpf_prog *prog, > const struct bpf_prog *tgt_prog, > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > index 869265852d51..bf826fcc8cf4 100644 > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -3464,8 +3464,9 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog, > */ > struct bpf_attach_target_info tgt_info = {}; > > - err = bpf_check_attach_target(NULL, prog, tgt_prog, btf_id, > - &tgt_info); > + err = bpf_check_attach_target_with_klog(prog, NULL, > + prog->aux->attach_btf_id, > + &tgt_info); code alignment issue here as well. Also, the argument should be 'prog, tgt_prog, btf_id, &tgt_info', right? > if (err) > goto out_unlock; > > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c > index f8302a5ca400..8862adaa7302 100644 > --- a/kernel/bpf/trampoline.c > +++ b/kernel/bpf/trampoline.c > @@ -699,9 +699,9 @@ int bpf_trampoline_link_cgroup_shim(struct bpf_prog *prog, > u64 key; > int err; > > - err = bpf_check_attach_target(NULL, prog, NULL, > - prog->aux->attach_btf_id, > - &tgt_info); > + err = bpf_check_attach_target_with_klog(prog, NULL, > + prog->aux->attach_btf_id, > + &tgt_info); code alignment issue here > if (err) > return err; > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 1f5302fb0957..4873b72f5a9a 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -21643,6 +21643,25 @@ static int check_non_sleepable_error_inject(u32 btf_id) > return btf_id_set_contains(&btf_non_sleepable_error_inject, btf_id); > } > > +int bpf_check_attach_target_with_klog(const struct bpf_prog *prog, > + const struct bpf_prog *tgt_prog, > + u32 btf_id, > + struct bpf_attach_target_info *tgt_info); code alignment issue here. > +{ > + struct bpf_verifier_log *log; > + int err; > + > + log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN); __GFP_NOWARN is unnecessary here. > + if (!log) { > + err = -ENOMEM; > + return err; > + } > + log->level = BPF_LOG_KERNEL; > + err = bpf_check_attach_target(log, prog, tgt_prog, btf_id, tgt_info); > + kfree(log); > + return err; > +} > + > int bpf_check_attach_target(struct bpf_verifier_log *log, > const struct bpf_prog *prog, > const struct bpf_prog *tgt_prog, More importantly, Andrii has implemented retsnoop, which intends to locate precise location in the kernel where err happens. The link is https://github.com/anakryiko/retsnoop Maybe you want to take a look and see whether it can resolve your issue. We should really avoid putting more stuff in dmesg whenever possible.