Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1200926 > unrolled thread

Re: [PATCH RFC v1 1/4] perf: Introduce extended syscall error reporting

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH RFC v1 1/4] perf: Introduce extended syscall error  reporting Peter Zijlstra <peterz@infradead.org> - 2015-08-05 17:50 +0200
    Re: [PATCH RFC v1 1/4] perf: Introduce extended syscall error reporting Alexander Shishkin <alexander.shishkin@linux.intel.com> - 2015-08-17 15:00 +0200

#1200926 — Re: [PATCH RFC v1 1/4] perf: Introduce extended syscall error reporting

On Fri, Jul 24, 2015 at 02:45:56PM +0300, Alexander Shishkin wrote:
> +static void perf_error_report_site(struct perf_event_attr *attr,
> +				   const struct perf_err_site *site)
> +{
> +	void *buffer;
> +
> +	if (!site || !extended_reporting_enabled(attr))
> +		return;
> +
> +	/* in case of nested perf_err()s, which you shouldn't really do */
> +	while (site->code <= -PERF_ERRNO)
> +		site = perf_errno_to_site(site->code);
> +
> +	buffer = kasprintf(GFP_KERNEL,
> +			   "{\n"
> +			   "\t\"code\": %d,\n"
> +			   "\t\"module\": \"%s\",\n"
> +			   "\t\"message\": \"%s\"\n"
> +			   "}\n",
> +			   site->code, site->owner, site->message
> +			   );
> +	if (!buffer)
> +		return;
> +
> +	if (copy_to_user((void __user *)attr->perf_err, buffer,
> +			 attr->perf_err_size)) {

Should that not be min(attr->perf_err_size, strlen(buffer)) ?

Also, should we not '\0' the last char in attr->perf_err in case buffer
is longer.

> +		/* if we failed to copy once, don't bother later */
> +		attr->perf_err_size = 0;
> +	}

So we want update the user's perf_err_size with the actual size we
copied in?

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

[toc] | [next] | [standalone]

#1208571 — Re: [PATCH RFC v1 1/4] perf: Introduce extended syscall error reporting

Peter Zijlstra <peterz@infradead.org> writes:

> On Fri, Jul 24, 2015 at 02:45:56PM +0300, Alexander Shishkin wrote:
>> +static void perf_error_report_site(struct perf_event_attr *attr,
>> +				   const struct perf_err_site *site)
>> +{
>> +	void *buffer;
>> +
>> +	if (!site || !extended_reporting_enabled(attr))
>> +		return;
>> +
>> +	/* in case of nested perf_err()s, which you shouldn't really do */
>> +	while (site->code <= -PERF_ERRNO)
>> +		site = perf_errno_to_site(site->code);
>> +
>> +	buffer = kasprintf(GFP_KERNEL,
>> +			   "{\n"
>> +			   "\t\"code\": %d,\n"
>> +			   "\t\"module\": \"%s\",\n"
>> +			   "\t\"message\": \"%s\"\n"
>> +			   "}\n",
>> +			   site->code, site->owner, site->message
>> +			   );
>> +	if (!buffer)
>> +		return;
>> +
>> +	if (copy_to_user((void __user *)attr->perf_err, buffer,
>> +			 attr->perf_err_size)) {
>
> Should that not be min(attr->perf_err_size, strlen(buffer)) ?

Indeed.

> Also, should we not '\0' the last char in attr->perf_err in case buffer
> is longer.

I'm guessing we should, because otherwise the user is likely to get all
confused. They can tell, however, if the buffer they allocated was too
small by looking for (and not finding) the closing bracket.

>
>> +		/* if we failed to copy once, don't bother later */
>> +		attr->perf_err_size = 0;
>> +	}
>
> So we want update the user's perf_err_size with the actual size we
> copied in?

We *could* do that for the perf_event_open() case where we have the
user's copy of the attribute and can mess around with it, but as you
noticed we're also using this error reporting in the other syscalls that
involve perf events and with those we won't have the user's attribute
any more.

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web