From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.9 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 92A32C388F7 for ; Thu, 22 Oct 2020 21:20:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3428221D43 for ; Thu, 22 Oct 2020 21:20:39 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="N/0AvT8+" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S372245AbgJVVUi (ORCPT ); Thu, 22 Oct 2020 17:20:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40664 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S372233AbgJVVUg (ORCPT ); Thu, 22 Oct 2020 17:20:36 -0400 Received: from mail-pg1-x541.google.com (mail-pg1-x541.google.com [IPv6:2607:f8b0:4864:20::541]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 96571C0613CF for ; Thu, 22 Oct 2020 14:20:36 -0700 (PDT) Received: by mail-pg1-x541.google.com with SMTP id 19so1769135pge.12 for ; Thu, 22 Oct 2020 14:20:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=YsUSHc5bYUNmuFMA3WRkMmx4GDjXmb6yOh7O9xZbDRc=; b=N/0AvT8+Hmb3Rp8IZcUtW/xq7QBs+gg8I6Im+7uExqg6EoLb9sXgBZz51+CZ+qnUuh SBx7rgrojGMMIHQoHOr/JSqOuFR35YDeN14XDliCDF1gmzKsxxor3D8TtwnO43MDPC3L CitQN82Mgy21Xnf1OAs2eG/ca46bJrn/G+FZFpfdhEhmZeLLBGNYI7z77kJTWcUthBRO Kk3lUArNhh5snSxjzXB2p10AvB+shf/2Uk2zAiCQDT+lB0BhYAX7x4T8RBuZ9X8Pse+3 VXgbuUmLljtDtL/KxQc23w1wka+dptL4JtJPQP4cXFKa2oImmbpxw01grfDWRrQXPkl8 0huw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=YsUSHc5bYUNmuFMA3WRkMmx4GDjXmb6yOh7O9xZbDRc=; b=P5Psrx8SefqFzVpEg04qNL/tGj6wiIOVsAam1U6zWgY7826Sy8xbKxG3UNgi+imyur kFYQ10ZNp2K+GizMIr/scT8jLoXbS5ofnDmmvOIMKJwPHuubX3b1UdTZ1BheIlb5KBB/ fTJVipBC9dDe1C4O2Y2/e4gNEhRJBDuZWyNEUaT6zIIU3RIK4POer9je/P/avCgoy58W Q4aGM6ClYYwOIBzr34lUS3nyQ+CdqZSNZcKqaKYBM+JFOaIZn7RfWcZ1VIA7Iz8X8l5V 1JI6i9SEU4UPAA9H47NxaLZxgIr056hzux31KMy8qFjgr8OwVHnW13npA7TLy60W8RMK 20IA== X-Gm-Message-State: AOAM531D57jE5lzKsXP2q5liFbelaR1unIY1Go4iLkRJLqkRHchzij5f Hc52BOB2+9bds3inxkSKLsYULA== X-Google-Smtp-Source: ABdhPJz+Cp9vygopwE/M6gjCd4Vr2adBC00b2KaBAjRV6HQipRCfUo6ON+X5DoToZl8Q4eAKTSdMMg== X-Received: by 2002:a17:90a:f504:: with SMTP id cs4mr3832476pjb.134.1603401636089; Thu, 22 Oct 2020 14:20:36 -0700 (PDT) Received: from xps15 (S0106002369de4dac.cg.shawcable.net. [68.147.8.254]) by smtp.gmail.com with ESMTPSA id j23sm3154622pgm.76.2020.10.22.14.20.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Oct 2020 14:20:35 -0700 (PDT) Date: Thu, 22 Oct 2020 15:20:33 -0600 From: Mathieu Poirier To: Suzuki Poulose Cc: Peter Zijlstra , Sai Prakash Ranjan , Mike Leach , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , coresight@lists.linaro.org, Stephen Boyd , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in tmc_enable_etf_sink_perf() Message-ID: <20201022212033.GA646497@xps15> References: <20201022113214.GD2611@hirez.programming.kicks-ass.net> <20201022150609.GI2611@hirez.programming.kicks-ass.net> <788706f2-0670-b7b6-a153-3ec6f16e0f2e@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <788706f2-0670-b7b6-a153-3ec6f16e0f2e@arm.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Peter, On Thu, Oct 22, 2020 at 04:32:36PM +0100, Suzuki Poulose wrote: > On 10/22/20 4:06 PM, Peter Zijlstra wrote: > > On Thu, Oct 22, 2020 at 02:30:21PM +0100, Suzuki Poulose wrote: > > > On 10/22/20 12:32 PM, Peter Zijlstra wrote: > > > > On Thu, Oct 22, 2020 at 04:27:52PM +0530, Sai Prakash Ranjan wrote: > > > > > > > > > Looking at the ETR and other places in the kernel, ETF and the > > > > > ETB are the only places trying to dereference the task(owner) > > > > > in tmc_enable_etf_sink_perf() which is also called from the > > > > > sched_in path as in the call trace. > > > > > > > > > @@ -391,6 +392,10 @@ static void *tmc_alloc_etf_buffer(struct coresight_device *csdev, > > > > > { > > > > > int node; > > > > > struct cs_buffers *buf; > > > > > + struct task_struct *task = READ_ONCE(event->owner); > > > > > + > > > > > + if (!task || is_kernel_event(event)) > > > > > + return NULL; > > > > > > > > > > > > This is *wrong*... why do you care about who owns the events? > > > > > > > > > > This is due to the special case of the CoreSight configuration, where > > > a "sink" (where the trace data is captured) is shared by multiple Trace > > > units. So, we could share the "sink" for multiple trace units if they > > > are tracing the events that belong to the same "perf" session. (The > > > userspace tool could decode the trace data based on the TraceID > > > in the trace packets). Is there a better way to do this ? > > > > I thought we added sink identification through perf_event_attr::config2 > > ? > > > > Correct. attr:config2 identifies the "sink" for the collection. But, > that doesn't solve the problem we have here. If two separate perf > sessions use the "same sink", we don't want to mix the > trace data into the same sink for events from different sessions. > > Thus, we need a way to check if a new event starting the tracing on > an ETM belongs to the same session as the one already pumping the trace > into the sink. Suzuki's depiction of the usecase is accurate. Using the pid of the process that created the events comes out of a discussion you and I had in the common area by the Intel booth at ELC in Edinburgh in the fall of 2018. At the time I exposed the problem of having multiple events sharing the same HW resources and you advised to proceed this way. That being said it is plausible that I did not expressed myself clearly enough for you to understand the full extend of the problem. If that is the case we are more than willing to revisit that solution. Do you see a better option than what has currently been implemented? Many thanks, Mathieu > > We use event->owner pid for this check and thats where we encountered > a NULL event->owner. Looking at the code further, we identified that > kernel events could also trigger this issue. > > Suzuki