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

[Li Zhang]

On 2015å06æ18æ 13:56, Sukadev Bhattiprolu wrote:
> Li Zhang [zhlcindy@xxxxxxxxxxxxxxxxxx] 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

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