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.
Signed-off-by: Shuah Khan <[email protected]>
---
kernel/events/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 7ed5248f0445..5f4236c84940 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10571,7 +10571,7 @@ static ssize_t nr_addr_filters_show(struct device *dev,
{
struct pmu *pmu = dev_get_drvdata(dev);
- return snprintf(page, PAGE_SIZE - 1, "%d\n", pmu->nr_addr_filters);
+ return scnprintf(page, PAGE_SIZE - 1, "%d\n", pmu->nr_addr_filters);
}
DEVICE_ATTR_RO(nr_addr_filters);
@@ -10582,7 +10582,7 @@ type_show(struct device *dev, struct device_attribute *attr, char *page)
{
struct pmu *pmu = dev_get_drvdata(dev);
- return snprintf(page, PAGE_SIZE-1, "%d\n", pmu->type);
+ return scnprintf(page, PAGE_SIZE-1, "%d\n", pmu->type);
}
static DEVICE_ATTR_RO(type);
@@ -10593,7 +10593,7 @@ perf_event_mux_interval_ms_show(struct device *dev,
{
struct pmu *pmu = dev_get_drvdata(dev);
- return snprintf(page, PAGE_SIZE-1, "%d\n", pmu->hrtimer_interval_ms);
+ return scnprintf(page, PAGE_SIZE-1, "%d\n", pmu->hrtimer_interval_ms);
}
static DEFINE_MUTEX(mux_interval_mutex);
--
2.25.1
Since snprintf() returns would-be-output size instead of the actual
output size, replace it with scnprintf(), so the show_smt_control(),
and show_smt_active() routines return the actual size.
Signed-off-by: Shuah Khan <[email protected]>
---
kernel/cpu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 6ff2578ecf17..29a5ceb93cda 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2334,7 +2334,7 @@ show_smt_control(struct device *dev, struct device_attribute *attr, char *buf)
{
const char *state = smt_states[cpu_smt_control];
- return snprintf(buf, PAGE_SIZE - 2, "%s\n", state);
+ return scnprintf(buf, PAGE_SIZE - 2, "%s\n", state);
}
static ssize_t
@@ -2348,7 +2348,7 @@ static DEVICE_ATTR(control, 0644, show_smt_control, store_smt_control);
static ssize_t
show_smt_active(struct device *dev, struct device_attribute *attr, char *buf)
{
- return snprintf(buf, PAGE_SIZE - 2, "%d\n", sched_smt_active());
+ return scnprintf(buf, PAGE_SIZE - 2, "%d\n", sched_smt_active());
}
static DEVICE_ATTR(active, 0444, show_smt_active, NULL);
--
2.25.1
Shuah Khan <[email protected]> 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].
[0] https://marc.info/?l=linux-kernel&m=159874491103969&w=2
Regards,
--
Alex
On 9/9/20 12:45 AM, Alexander Shishkin wrote:
> Shuah Khan <[email protected]> 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.
thanks,
-- Shuah
On Wed, 2020-09-09 at 10:19 -0600, Shuah Khan wrote:
> On 9/9/20 12:45 AM, Alexander Shishkin wrote:
> > Shuah Khan <[email protected]> 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/[email protected]/
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;
...
}
On 09/01/20 17:49, Shuah Khan wrote:
> Since snprintf() returns would-be-output size instead of the actual
> output size, replace it with scnprintf(), so the show_smt_control(),
> and show_smt_active() routines return the actual size.
>
> Signed-off-by: Shuah Khan <[email protected]>
Looks good to me.
Cheers
--
Qais Yousef
> ---
> kernel/cpu.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 6ff2578ecf17..29a5ceb93cda 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -2334,7 +2334,7 @@ show_smt_control(struct device *dev, struct device_attribute *attr, char *buf)
> {
> const char *state = smt_states[cpu_smt_control];
>
> - return snprintf(buf, PAGE_SIZE - 2, "%s\n", state);
> + return scnprintf(buf, PAGE_SIZE - 2, "%s\n", state);
> }
>
> static ssize_t
> @@ -2348,7 +2348,7 @@ static DEVICE_ATTR(control, 0644, show_smt_control, store_smt_control);
> static ssize_t
> show_smt_active(struct device *dev, struct device_attribute *attr, char *buf)
> {
> - return snprintf(buf, PAGE_SIZE - 2, "%d\n", sched_smt_active());
> + return scnprintf(buf, PAGE_SIZE - 2, "%d\n", sched_smt_active());
> }
> static DEVICE_ATTR(active, 0444, show_smt_active, NULL);
>
> --
> 2.25.1
>