linux-kernel - Re: [PATCH v2 1/1] perf tools: Check access permission when reading /proc/kcore file.

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-next>] [day] [month] [year] [list]

Message-ID: <20150617060946.GA18162@us.ibm.com>
Date:	Tue, 16 Jun 2015 23:09:46 -0700
From:	Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>
To:	Li Zhang <zhlcindy@...ux.vnet.ibm.com>
Cc:	acme@...nel.org, mingo@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/1] perf tools: Check access permission when reading
 /proc/kcore file.

Li Zhang [zhlcindy@...ux.vnet.ibm.com] wrote:
| When using command perf report --kallsyms=/proc/kallsyms with a non-root
| user, symbols are resolved. Then select one symbol and annotate it, it
| reports the error as the following:
| Can't annotate __clear_user: No vmlinux file with build id xxx was
| found.
| 
| The problem is caused by reading /proc/kcore without access permission.
| It needs to change access permission to allow a specific user to read
| /proc/kcore or use root to execute the perf command.
| 
| This patch is to check access permission when reading kcore file.
| 
| Signed-off-by: Li Zhang <zhlcindy@...ux.vnet.ibm.com>
| ---
|  v2 -> v1:
|     * Report one useful message to users about the access permision,
|       then go back to the tools. Suggested by Arnaldo Carvalho de Melo.
| 
|  tools/perf/util/symbol.c | 4 ++++
|  1 file changed, 4 insertions(+)
| 
| diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
| index 201f6c4c..1bcd8dc 100644
| --- a/tools/perf/util/symbol.c
| +++ b/tools/perf/util/symbol.c
| @@ -1125,6 +1125,10 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
|  	md.type = map->type;
|  	INIT_LIST_HEAD(&md.maps);
| 
| +	if (access(kcore_filename, R_OK))
| +		ui__warning("Insufficient permission to access %s.\n",
| +			    kcore_filename);
| +

Couple of comments. 

For consistency with rest of the file, use pr_warning() or pr_err(). 

Also, we could drop the access() call and report the error when open()
fails below?

|  	fd = open(kcore_filename, O_RDONLY);
|  	if (fd < 0)
|  		return -EINVAL;

Further, if user specifies the file with --kallsyms and we are not
able to read it, we should treat it as a fatal error and exit - this
would be easer when parsing command line args.

If user did not specify the option and we are proactively trying to
use /proc/kcore, we should not treat errors as fatal? i.e report
a warning message and continue without symbols?

| -- 
| 2.1.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/