From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 6E8E217C8 for ; Fri, 20 Jan 2023 01:15:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E9CFAC433EF; Fri, 20 Jan 2023 01:15:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1674177303; bh=lLzTOxxNjaiQKOPlo3L6pPfggeDPqcWEZB1PMvTcy6A=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=NQNuijB4n4kkGrzv324l0NBBy1ipJBEK+maRGfVoBchwhY9nt4uuL9KykH5Tix8mV Atk+0PxofGkqOdSDgM7IauQa47LEegSTOe0s08ouVyZglvAUpz+t6cetrcabg5TSoi TadqePjFJLI+jiOuLOnZ5FqRw2wWn1wKvKCWfyx6w4Wyvw1r45fZDv5v0ZMB07O5S9 9YEsrrzHLh/akN4Qkc/I2GE0cAanh4ncw2pJwvhh22rbzMiFkryABySJEvyD2YiKcd FiHt9duGkQfBlJ2/s34CXH4HLiLjBB2seQcz+OHGR2/91yBdGvCXgSg+AVzjmJYfIt MJzY4cEZkYg/g== Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id 8D44B405BE; Thu, 19 Jan 2023 22:15:00 -0300 (-03) Date: Thu, 19 Jan 2023 22:15:00 -0300 From: Arnaldo Carvalho de Melo To: Ian Rogers Cc: Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Nathan Chancellor , Nick Desaulniers , Tom Rix , Adrian Hunter , Leo Yan , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, llvm@lists.linux.dev, Stephane Eranian Subject: Re: [PATCH] perf buildid: Avoid copy of uninitialized memory Message-ID: References: <20230113185732.134861-1-irogers@google.com> Precedence: bulk X-Mailing-List: llvm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Url: http://acmel.wordpress.com Em Thu, Jan 19, 2023 at 08:57:21AM -0800, Ian Rogers escreveu: > On Thu, Jan 19, 2023 at 8:39 AM Arnaldo Carvalho de Melo > wrote: > > > > Em Thu, Jan 19, 2023 at 01:29:30PM -0300, Arnaldo Carvalho de Melo escreveu: > > > Em Thu, Jan 19, 2023 at 08:08:13AM -0800, Ian Rogers escreveu: > > > > On Fri, Jan 13, 2023 at 10:57 AM Ian Rogers wrote: > > > > > > > > > > build_id__init only copies the buildid data up to size leaving the > > > > > rest of the data array uninitialized. Copying the full array during > > > > > synthesis means the written event contains uninitialized memory. This > > > > > was detected by the Clang/LLVM memory sanitizer. > > > > > > > > > > Signed-off-by: Ian Rogers > > > > > --- > > > > > tools/perf/util/synthetic-events.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c > > > > > index 3ab6a92b1a6d..348d05e4ec03 100644 > > > > > --- a/tools/perf/util/synthetic-events.c > > > > > +++ b/tools/perf/util/synthetic-events.c > > > > > @@ -2219,7 +2219,7 @@ int perf_event__synthesize_build_id(struct perf_tool *tool, struct dso *pos, u16 > > > > > > > > > > len = pos->long_name_len + 1; > > > > > len = PERF_ALIGN(len, NAME_ALIGN); > > > > > - memcpy(&ev.build_id.build_id, pos->bid.data, sizeof(pos->bid.data)); > > > > > + memcpy(&ev.build_id.build_id, pos->bid.data, pos->bid.size); > > > > > > > > Ping. Should be an uncontroversial change to fix a copy of > > > > uninitialized memory into the perf.data file during synthesis. > > > > > > Indeed, applied. > > > > Humm, don't we better do it as: > > > > + memcpy(&ev.build_id.build_id, pos->bid.data, min(pos->bid.size, sizeof(pos->bid.data)); > > > > Lemme check what is setting that pos->bid.size.... > > > > Things like sysfs__read_build_id() that does such checks, but perhaps we > > should be defensive and do it in this function as well? > > Defensive is good, another option would be an assert but they can be > compiled out. Do you want me to repost? Please. - Arnaldo