linux-kernel - Re: [PATCH v4 2/5] perf tools riscv: Add support for get_cpuid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YrXnFAaT/B1In/+t@kernel.org>
Date:   Fri, 24 Jun 2022 13:32:20 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Nikita Shubin <nikita.shubin@...uefel.me>
Cc:     Atish Patra <atishp@...shpatra.org>,
        Anup Patel <anup@...infault.org>,
        João Mário Domingos 
        <joao.mario@...nico.ulisboa.pt>, linux@...ro.com,
        Nikita Shubin <n.shubin@...ro.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v4 2/5] perf tools riscv: Add support for get_cpuid_str
 function

Em Fri, Jun 24, 2022 at 07:00:52PM +0300, Nikita Shubin escreveu:
> From: Nikita Shubin <n.shubin@...ro.com>
> 
> The get_cpuid_str function returns the string that
> contains values of MVENDORID, MARCHID and MIMPID in
> hex format separated by coma.
> 
> The values themselves are taken from first cpu entry
> in "/proc/cpuid" that contains "mvendorid", "marchid"
> and "mimpid".
> 
> Signed-off-by: Nikita Shubin <n.shubin@...ro.com>
> ---
>  tools/perf/arch/riscv/util/Build    |   1 +
>  tools/perf/arch/riscv/util/header.c | 109 ++++++++++++++++++++++++++++
>  2 files changed, 110 insertions(+)
>  create mode 100644 tools/perf/arch/riscv/util/header.c
> 
> diff --git a/tools/perf/arch/riscv/util/Build b/tools/perf/arch/riscv/util/Build
> index 7d3050134ae0..603dbb5ae4dc 100644
> --- a/tools/perf/arch/riscv/util/Build
> +++ b/tools/perf/arch/riscv/util/Build
> @@ -1,4 +1,5 @@
>  perf-y += perf_regs.o
> +perf-y += header.o
>  
>  perf-$(CONFIG_DWARF) += dwarf-regs.o
>  perf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
> diff --git a/tools/perf/arch/riscv/util/header.c b/tools/perf/arch/riscv/util/header.c
> new file mode 100644
> index 000000000000..53e8ddf7990b
> --- /dev/null
> +++ b/tools/perf/arch/riscv/util/header.c
> @@ -0,0 +1,109 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Implementation of get_cpuid().
> + *
> + * Author: Nikita Shubin <n.shubin@...ro.com>
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <api/fs/fs.h>
> +#include <errno.h>
> +#include "../../util/debug.h"
> +#include "../../util/header.h"
> +
> +#define CPUINFO_MVEN	"mvendorid"
> +#define CPUINFO_MARCH	"marchid"
> +#define CPUINFO_MIMP	"mimpid"
> +#define CPUINFO		"/proc/cpuinfo"
> +
> +static char *_get_field(const char *line)
> +{
> +	char *line2, *nl;
> +
> +	line2 = strrchr(line, ' ');
> +	if (!line2)
> +		return NULL;
> +
> +	line2++;
> +	nl = strrchr(line, '\n');
> +	if (!nl)
> +		return NULL;
> +
> +	return strndup(line2, nl - line2);
> +}
> +
> +static char *_get_cpuid(void)
> +{
> +	char *line = NULL;
> +	char *mvendorid = NULL;
> +	char *marchid = NULL;
> +	char *mimpid = NULL;
> +	char *cpuid = NULL;
> +	int read;
> +	unsigned long line_sz;
> +	FILE *cpuinfo;
> +
> +	cpuinfo = fopen(CPUINFO, "r");
> +	if (cpuinfo == NULL)
> +		return cpuid;
> +
> +	while ((read = getline(&line, &line_sz, cpuinfo)) != -1) {
> +		if (!strncmp(line, CPUINFO_MVEN, strlen(CPUINFO_MVEN))) {
> +			mvendorid = _get_field(line);
> +			if (!mvendorid)
> +				goto free;
> +		} else if (!strncmp(line, CPUINFO_MARCH, strlen(CPUINFO_MARCH))) {
> +			marchid = _get_field(line);
> +			if (!marchid)
> +				goto free;
> +		} else if (!strncmp(line, CPUINFO_MIMP, strlen(CPUINFO_MIMP))) {
> +			mimpid = _get_field(line);
> +			if (!mimpid)
> +				goto free;
> +
> +			break;
> +		}
> +	}
> +
> +	if (!mvendorid || !marchid || !mimpid) {
> +		cpuid = NULL;
> +		goto free;
> +	}
> +
> +	if (asprintf(&cpuid, "%s-%s-%s", mvendorid, marchid, mimpid) < 0)
> +		cpuid = NULL;
> +
> +free:
> +	fclose(cpuinfo);
> +
> +	if (mvendorid)
> +		free(mvendorid);
> +
> +	if (marchid)
> +		free(marchid);
> +
> +	if (mimpid)
> +		free(mimpid);

just use:

	
	free(mvendorid);
	free(marchid);
	free(mimpid);

fewer lines, free() accepts NULL.

> +
> +	return cpuid;
> +}
> +
> +int get_cpuid(char *buffer, size_t sz)
> +{
> +	char *cpuid = _get_cpuid();
> +
> +	if (sz < strlen(cpuid)) {
> +		free(cpuid);
> +		return -EINVAL;
> +	}
> +
> +	scnprintf(buffer, sz, "%s", cpuid);

You're leaking cpuid here.

> +	return 0;
> +}
> +
> +char *
> +get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
> +{
> +	return _get_cpuid();
> +}
> -- 
> 2.35.1

-- 

- Arnaldo