From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753558AbbFRIZs (ORCPT ); Thu, 18 Jun 2015 04:25:48 -0400 Received: from e23smtp03.au.ibm.com ([202.81.31.145]:53577 "EHLO e23smtp03.au.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752797AbbFRIZl (ORCPT ); Thu, 18 Jun 2015 04:25:41 -0400 X-Helo: d23dlp03.au.ibm.com X-MailFrom: zhlcindy@linux.vnet.ibm.com X-RcptTo: linux-kernel@vger.kernel.org Message-ID: <55828043.6010904@linux.vnet.ibm.com> Date: Thu, 18 Jun 2015 16:24:35 +0800 From: Li Zhang User-Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 MIME-Version: 1.0 To: Sukadev Bhattiprolu CC: acme@kernel.org, mingo@redhat.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/1] perf tools: Check access permission when reading /proc/kcore file. References: <1434350500-32342-1-git-send-email-zhlcindy@linux.vnet.ibm.com> <20150617060946.GA18162@us.ibm.com> <55823B78.3050403@linux.vnet.ibm.com> <20150618055608.GA2193@us.ibm.com> In-Reply-To: <20150618055608.GA2193@us.ibm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15061808-0009-0000-0000-000001928D33 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2015年06月18日 13:56, Sukadev Bhattiprolu wrote: > Li Zhang [zhlcindy@linux.vnet.ibm.com] wrote: > > | >For consistency with rest of the file, use pr_warning() or pr_err(). > | > | ui_warning can report the message to users directly when this > | program is running. > | But if we considered the consistency, pr_warning or pr_err should be better. > | And users can get this message by trying another time. > > That seems to be the way perf currently operates - silent by default for > non-fatal errors. -v or -vvv increases verbosity and reports non-fatal > warnings/errors also. OK, I will follow the way perf operates. > > | > | > > | >Also, we could drop the access() call and report the error when open() > | >fails below? > | > | I think we can drop this access. But /proc/kcore also require the > | process with CAP_SYS_RAWIO > | capability. Even if chown this file, access report right result, but > | open still fails. > > Maybe the error message could hint that CAP_SYS_RAWIO would be needed. I will check this in my V3 patch. > | > | > > | >| 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. > | I have another patch which checks this files. I will merge it to this patch. > | > | > > | >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? > | > | In the current program, even if open fails, the program still > | continue to run. > | Is it helpful for users to get the address without symbols? > > Well, if profiling applications, user may not care about kernel symbols, > so being unable to open /proc/kcore would be ok? If OTOH, user specifies > --kallsyms, then they care about the kenrel symbols so we should treat > the open() error () as fatal. I see, thanks. I will send out v3 soon. -- Li Zhang IBM China Linux Technology Centre