lkml.org 
[lkml]   [2020]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] kernel: events: Use scnprintf() in show_pmu_*() instead of snprintf()
From
Date
On Wed, 2020-09-09 at 10:19 -0600, Shuah Khan wrote:
> On 9/9/20 12:45 AM, Alexander Shishkin wrote:
> > Shuah Khan <skhan@linuxfoundation.org> writes:
> >
> > > Since snprintf() returns would-be-output size instead of the actual
> > > output size, replace it with scnprintf(), so the nr_addr_filters_show(),
> > > type_show(), and perf_event_mux_interval_ms_show() routines return the
> > > actual size.
> >
> > Well, firstly they should just be sprintf()s, and secondly, I wouldn't
> > worry about it, because [0].
>
> scnprintf() or sprinf() could be used.
>
> > [0] https://marc.info/?l=linux-kernel&m=159874491103969&w=2
>
> Awesome. Thanks for the pointer. I wasn't aware of this work and
> it takes care of the problem kernel wide. A better way to solve
> the problem.

There is a fairly large, though fairly trivial direct conversion
using a cocci script for 90+% (~5000) of the existing uses of
device and kobject show functions that use any of the sprintf
call family.

https://lore.kernel.org/lkml/c22b7006813b1776467a72e716a5970e9277b4b7.camel@perches.com/

The other < 10% though require some manual changes.

There are some code blocks where it's possible for a
PAGE_SIZE buffer overrun to occur, though perhaps it's not
ever occurred in practice.

A defect I've seen when looking at the code is to always
output to a presumed PAGE_SIZE buffer even though the
output buffer address has been advanced.

i.e.:

for (i = 0; i < count; i++)
buf += scnprintf(buf, PAGE_SIZE, " %u", val[i]);

In actual code: (from drivers/staging/gasket/gasket_core.c)

In this code buf is passed to a helper function without adding
an offset in buf to the argument list and PAGE_SIZE is used for
multiple calls in a for loop in the case statement.

static ssize_t
gasket_write_mappable_regions(char *buf,
const struct gasket_driver_desc *driver_desc,
int bar_index)
{
int i;
ssize_t written;
ssize_t total_written = 0;
ulong min_addr, max_addr;
struct gasket_bar_desc bar_desc =
driver_desc->bar_descriptions[bar_index];

if (bar_desc.permissions == GASKET_NOMAP)
return 0;
for (i = 0;
i < bar_desc.num_mappable_regions && total_written < PAGE_SIZE;
i++) {
min_addr = bar_desc.mappable_regions[i].start -
driver_desc->legacy_mmap_address_offset;
max_addr = bar_desc.mappable_regions[i].start -
driver_desc->legacy_mmap_address_offset +
bar_desc.mappable_regions[i].length_bytes;
written = scnprintf(buf, PAGE_SIZE - total_written,
"0x%08lx-0x%08lx\n", min_addr, max_addr);
total_written += written;
buf += written;
}
return total_written;
}

...

static ssize_t gasket_sysfs_data_show(struct device *device,
struct device_attribute *attr, char *buf)
{
...
switch (sysfs_type) {
...
case ATTR_USER_MEM_RANGES:
for (i = 0; i < PCI_STD_NUM_BARS; ++i) {
current_written =
gasket_write_mappable_regions(buf, driver_desc,
i);
buf += current_written;
ret += current_written;
}
break;
...
}


\
 
 \ /
  Last update: 2020-09-09 19:27    [W:0.235 / U:0.252 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site