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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BAF89C77B6F for ; Wed, 12 Apr 2023 15:50:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229977AbjDLPuN (ORCPT ); Wed, 12 Apr 2023 11:50:13 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51600 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229699AbjDLPuK (ORCPT ); Wed, 12 Apr 2023 11:50:10 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B53E559F0; Wed, 12 Apr 2023 08:50:09 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 4FC9E63583; Wed, 12 Apr 2023 15:50:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 50CDBC433D2; Wed, 12 Apr 2023 15:50:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1681314608; bh=py3z9HpgKFsgkK9Fic8o4YVTIgR1hDxGNyaU/pbt2JI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=vNRd7hdMc13Bgl7TeBLNA48Df/uWyCYgfPg5viZELVHwugJcquo2tgx9145J/rs2W /SSxGdV6Iykvm44ZpdC2sktgczRGEd+Jgm5v/zkNKqRcEpIv6WsAjf0n9+FWmNuL3D Nvd7bzr8kQQ1vNF3Lo+TXjwP1VnCn4pWQjd/mHMtSQe/qqhDH9GDxsEVTei/QJqZhl Xa2vazN0P6ewrIjITHHldUsLcIVoYQ7+fHDZD1pwls9H0iS/NxJMz1hDshh+HR2l0u MwfUiF/kWBlM9ldSuVmr0GagEmhkmaUQoSSMdaPjkJrdU4zrp9TJ0legQthIuVoRfQ W1jxRySoyjAag== Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id 94F7F40080; Wed, 12 Apr 2023 12:50:05 -0300 (-03) Date: Wed, 12 Apr 2023 12:50:05 -0300 From: Arnaldo Carvalho de Melo To: Ian Rogers Cc: Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Thomas Gleixner , Darren Hart , Davidlohr Bueso , James Clark , John Garry , Riccardo Mancini , Yury Norov , Andy Shevchenko , Andrew Morton , Adrian Hunter , Leo Yan , Andi Kleen , Thomas Richter , Kan Liang , Madhavan Srinivasan , Shunsuke Nakamura , Song Liu , Masami Hiramatsu , Steven Rostedt , Miaoqian Lin , Stephen Brennan , Kajol Jain , Alexey Bayduraev , German Gomez , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, Eric Dumazet , Dmitry Vyukov , Hao Luo , Stephane Eranian Subject: Re: [PATCH v7 2/5] perf cpumap: Add reference count checking Message-ID: References: <20230407230405.2931830-1-irogers@google.com> <20230407230405.2931830-3-irogers@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Url: http://acmel.wordpress.com Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, Apr 11, 2023 at 03:19:06PM -0300, Arnaldo Carvalho de Melo escreveu: > Em Fri, Apr 07, 2023 at 04:04:02PM -0700, Ian Rogers escreveu: > > This change is intended to catch: > > - use after put: using a cpumap after you have put it will cause a > > segv. > > - unbalanced puts: two puts for a get will result in a double free > > that can be captured and reported by tools like address sanitizer, > > including with the associated stack traces of allocation and frees. > > - missing puts: if a put is missing then the get turns into a memory > > leak that can be reported by leak sanitizer, including the stack > > trace at the point the get occurs. > I think this should be further split into self contained patches as it > does: > These are missing convertions that should be in a separate patch, no? > > > @@ -239,7 +236,7 @@ struct cpu_aggr_map *cpu_aggr_map__new(const struct perf_cpu_map *cpus, > > { > > int idx; > > struct perf_cpu cpu; > > - struct cpu_aggr_map *c = cpu_aggr_map__empty_new(cpus->nr); > > + struct cpu_aggr_map *c = cpu_aggr_map__empty_new(perf_cpu_map__nr(cpus)); > > > > if (!c) > > return NULL; Extracted this from your larger patch: >From f8a23fce48400168be0cc078a0b0bd0e7d4a889f Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Wed, 12 Apr 2023 12:45:45 -0300 Subject: [PATCH 1/1] perf cpumap: Use perf_cpu_map__nr(cpus) to access cpus->nr So that we can have a single point where to refcount check 'struct perf_cpu_map' instances for use after free, etc. Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexey Bayduraev Cc: Dmitriy Vyukov Cc: Jiri Olsa Cc: Namhyung Kim Cc: Riccardo Mancini Cc: Stephane Eranian Cc: Stephen Brennan Link: https://lore.kernel.org/lkml/ Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/cpumap.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c index 5e564974fba4ffab..c8484b75413ef709 100644 --- a/tools/perf/util/cpumap.c +++ b/tools/perf/util/cpumap.c @@ -239,7 +239,7 @@ struct cpu_aggr_map *cpu_aggr_map__new(const struct perf_cpu_map *cpus, { int idx; struct perf_cpu cpu; - struct cpu_aggr_map *c = cpu_aggr_map__empty_new(cpus->nr); + struct cpu_aggr_map *c = cpu_aggr_map__empty_new(perf_cpu_map__nr(cpus)); if (!c) return NULL; @@ -263,7 +263,7 @@ struct cpu_aggr_map *cpu_aggr_map__new(const struct perf_cpu_map *cpus, } } /* Trim. */ - if (c->nr != cpus->nr) { + if (c->nr != perf_cpu_map__nr(cpus)) { struct cpu_aggr_map *trimmed_c = realloc(c, sizeof(struct cpu_aggr_map) + sizeof(struct aggr_cpu_id) * c->nr); @@ -582,9 +582,9 @@ size_t cpu_map__snprint(struct perf_cpu_map *map, char *buf, size_t size) #define COMMA first ? "" : "," - for (i = 0; i < map->nr + 1; i++) { + for (i = 0; i < perf_cpu_map__nr(map) + 1; i++) { struct perf_cpu cpu = { .cpu = INT_MAX }; - bool last = i == map->nr; + bool last = i == perf_cpu_map__nr(map); if (!last) cpu = map->map[i]; @@ -633,7 +633,7 @@ size_t cpu_map__snprint_mask(struct perf_cpu_map *map, char *buf, size_t size) int i, cpu; char *ptr = buf; unsigned char *bitmap; - struct perf_cpu last_cpu = perf_cpu_map__cpu(map, map->nr - 1); + struct perf_cpu last_cpu = perf_cpu_map__cpu(map, perf_cpu_map__nr(map) - 1); if (buf == NULL) return 0; @@ -644,7 +644,7 @@ size_t cpu_map__snprint_mask(struct perf_cpu_map *map, char *buf, size_t size) return 0; } - for (i = 0; i < map->nr; i++) { + for (i = 0; i < perf_cpu_map__nr(map); i++) { cpu = perf_cpu_map__cpu(map, i).cpu; bitmap[cpu / 8] |= 1 << (cpu % 8); } -- 2.39.2