From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-187.mta1.migadu.com (out-187.mta1.migadu.com [95.215.58.187]) (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 2FF9222B8AB for ; Thu, 25 Sep 2025 17:45:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.187 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758822357; cv=none; b=UeFYCaYk7bi1Pe4Ee72GLXfzo+Zps53z9b2lFBK2kj9/l6ktMbuGMJNx/+t8nD7qzG5bSblvFKHFD2aXHIGvrKqjNNRzlSp4XxRjkEnMbMEBDppLW+xjWwZCmxpij410uhxWb6CWK7rnuNAIlmB2Owu/6/7rHAPMtdckyfpY9js= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1758822357; c=relaxed/simple; bh=cyKhDezdYWwGbQB43lnNusq2qtiII1uVfZrv+wRg5vA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=bhBbxNYPUxIxA6pxJXPrfDhGKDLSvSKsqnIynhYPSbyBr6o6o3prUfKF47VL+FxYprk0WOs2vG/bqv/+R39QRe/Q4JMxmIFzY7rpeYc6p3JiNVIWPSqJpnxVRLd6wOWtjC3O9XFnweqXoD9H1Fm24J1aBFLdBtOhKUxayX8/pIA= 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=n39f1Ke3; arc=none smtp.client-ip=95.215.58.187 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="n39f1Ke3" Message-ID: <310091f8-ee17-4dfc-bbb4-1bb262cbfd98@linux.dev> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1758822343; 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=Uuxu7hZwOaKDVqdhli7Mdbk2jg5kVbusifSuE5+jNMY=; b=n39f1Ke34lHYFK4IXMv9SUO4R1MZtlV1QKmZylX0YOcFHATqUpwfmu1sZQy826VRJz4CMz pqxliQbAond7RRsG0yx8bO8DJajAOyE6Uc2dV1XJymGX1vY9mDcA1S3INTQ3ojbmdqgYfE aUD10sBpS5DZBvEC224B037rqToz2t8= Date: Fri, 26 Sep 2025 01:45:30 +0800 Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH bpf-next] bpf: Add preempt_disable to protect get_perf_callchain To: Alexei Starovoitov Cc: Song Liu , Jiri Olsa , Alexei Starovoitov , Daniel Borkmann , Andrii Nakryiko , Martin KaFai Lau , Eduard , Yonghong Song , John Fastabend , KP Singh , Stanislav Fomichev , Hao Luo , bpf , LKML References: <20250922075333.1452803-1-chen.dylane@linux.dev> X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Tao Chen In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT 在 2025/9/23 10:53, Alexei Starovoitov 写道: > On Mon, Sep 22, 2025 at 12:54 AM Tao Chen wrote: >> >> As Alexei suggested, the return value from get_perf_callchain() may be >> reused if another task preempts and requests the stack after BPF program >> switched to migrate disable. >> >> Reported-by: Alexei Starovoitov >> Signed-off-by: Tao Chen >> --- >> kernel/bpf/stackmap.c | 14 +++++--------- >> 1 file changed, 5 insertions(+), 9 deletions(-) >> >> diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c >> index 2e182a3ac4c..07892320906 100644 >> --- a/kernel/bpf/stackmap.c >> +++ b/kernel/bpf/stackmap.c >> @@ -314,8 +314,10 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map, >> if (max_depth > sysctl_perf_event_max_stack) >> max_depth = sysctl_perf_event_max_stack; >> >> + preempt_disable(); >> trace = get_perf_callchain(regs, 0, kernel, user, max_depth, >> false, false); >> + preempt_enable(); > > This is obviously wrong. > As soon as preemption is enabled, trace can be overwritten. > guard(preempt)(); > can fix it, but the length of the preempt disabled section > will be quite big. > The way get_perf_callchain() api is written I don't see > another option though. Unless we refactor it similar > to bpf_try_get_buffers(). > > pw-bot: cr Hi Alexei, I tried to understand what you meant and looked at the implementation of get_perf_callchain. Only one perf_callchain_entry on every cpu right now. callchain_cpus_entries(rcu global avariable) ↓ struct callchain_cpus_entries { struct perf_callchain_entry *cpu_entries[]; | } ｜-> perf_callchain_entry0 cpu0 perf_callchain_entry1 cpu1 … perf_callchain_entryn cpun If we want to realise it like bpf_try_get_buffers, we should alloc a perf_callchain_entry array on every cpu right? callchain_cpus_entries(rcu global avariable) ↓ struct callchain_cpus_entries { struct perf_callchain_entry *cpu_entries[]; | } ｜-> perf_callchain_entry0[N] cpu0 perf_callchain_entry1[N] cpu1 … perf_callchain_entryn[N] cpun -- Best Regards Tao Chen