* [PATCH] perf: arm-ccn: Use scnprintf() for avoiding potential buffer overflow
@ 2020-03-11 9:05 Takashi Iwai
2020-03-13 17:49 ` Mark Rutland
0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2020-03-11 9:05 UTC (permalink / raw)
To: Will Deacon, Mark Rutland; +Cc: linux-arm-kernel
Since snprintf() returns the would-be-output size instead of the
actual output size, the succeeding calls may go beyond the given
buffer limit. Fix it by replacing with scnprintf().
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
drivers/perf/arm-ccn.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/perf/arm-ccn.c b/drivers/perf/arm-ccn.c
index fea354d6fb29..cee579d428e7 100644
--- a/drivers/perf/arm-ccn.c
+++ b/drivers/perf/arm-ccn.c
@@ -330,13 +330,13 @@ static ssize_t arm_ccn_pmu_event_show(struct device *dev,
res = snprintf(buf, PAGE_SIZE, "type=0x%x", event->type);
if (event->event)
- res += snprintf(buf + res, PAGE_SIZE - res, ",event=0x%x",
+ res += scnprintf(buf + res, PAGE_SIZE - res, ",event=0x%x",
event->event);
if (event->def)
- res += snprintf(buf + res, PAGE_SIZE - res, ",%s",
+ res += scnprintf(buf + res, PAGE_SIZE - res, ",%s",
event->def);
if (event->mask)
- res += snprintf(buf + res, PAGE_SIZE - res, ",mask=0x%x",
+ res += scnprintf(buf + res, PAGE_SIZE - res, ",mask=0x%x",
event->mask);
/* Arguments required by an event */
@@ -344,25 +344,25 @@ static ssize_t arm_ccn_pmu_event_show(struct device *dev,
case CCN_TYPE_CYCLES:
break;
case CCN_TYPE_XP:
- res += snprintf(buf + res, PAGE_SIZE - res,
+ res += scnprintf(buf + res, PAGE_SIZE - res,
",xp=?,vc=?");
if (event->event == CCN_EVENT_WATCHPOINT)
- res += snprintf(buf + res, PAGE_SIZE - res,
+ res += scnprintf(buf + res, PAGE_SIZE - res,
",port=?,dir=?,cmp_l=?,cmp_h=?,mask=?");
else
- res += snprintf(buf + res, PAGE_SIZE - res,
+ res += scnprintf(buf + res, PAGE_SIZE - res,
",bus=?");
break;
case CCN_TYPE_MN:
- res += snprintf(buf + res, PAGE_SIZE - res, ",node=%d", ccn->mn_id);
+ res += scnprintf(buf + res, PAGE_SIZE - res, ",node=%d", ccn->mn_id);
break;
default:
- res += snprintf(buf + res, PAGE_SIZE - res, ",node=?");
+ res += scnprintf(buf + res, PAGE_SIZE - res, ",node=?");
break;
}
- res += snprintf(buf + res, PAGE_SIZE - res, "\n");
+ res += scnprintf(buf + res, PAGE_SIZE - res, "\n");
return res;
}
--
2.16.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] perf: arm-ccn: Use scnprintf() for avoiding potential buffer overflow
2020-03-11 9:05 [PATCH] perf: arm-ccn: Use scnprintf() for avoiding potential buffer overflow Takashi Iwai
@ 2020-03-13 17:49 ` Mark Rutland
2020-03-15 8:50 ` Takashi Iwai
0 siblings, 1 reply; 3+ messages in thread
From: Mark Rutland @ 2020-03-13 17:49 UTC (permalink / raw)
To: Takashi Iwai; +Cc: Will Deacon, linux-arm-kernel
On Wed, Mar 11, 2020 at 10:05:55AM +0100, Takashi Iwai wrote:
> Since snprintf() returns the would-be-output size instead of the
> actual output size, the succeeding calls may go beyond the given
> buffer limit. Fix it by replacing with scnprintf().
The buffer limit is PAGE_SIZE bytes here, so I don't think that we can
practically go beyond the buffer limit in this code where we only print
tens of chacters into the buffer. On any system this runs on, PAGE_SIZE
is at least 4096.
I'm happy to make this change as it's generally the right thing to do,
but I do want the commit message to be very clear that this is a
cleanup, and not a fix for a bug in practice.
Can you please either reword the commit message to that effect, or if
this can occur in practice, explain how?
Thanks,
Mark.
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> drivers/perf/arm-ccn.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/perf/arm-ccn.c b/drivers/perf/arm-ccn.c
> index fea354d6fb29..cee579d428e7 100644
> --- a/drivers/perf/arm-ccn.c
> +++ b/drivers/perf/arm-ccn.c
> @@ -330,13 +330,13 @@ static ssize_t arm_ccn_pmu_event_show(struct device *dev,
>
> res = snprintf(buf, PAGE_SIZE, "type=0x%x", event->type);
> if (event->event)
> - res += snprintf(buf + res, PAGE_SIZE - res, ",event=0x%x",
> + res += scnprintf(buf + res, PAGE_SIZE - res, ",event=0x%x",
> event->event);
> if (event->def)
> - res += snprintf(buf + res, PAGE_SIZE - res, ",%s",
> + res += scnprintf(buf + res, PAGE_SIZE - res, ",%s",
> event->def);
> if (event->mask)
> - res += snprintf(buf + res, PAGE_SIZE - res, ",mask=0x%x",
> + res += scnprintf(buf + res, PAGE_SIZE - res, ",mask=0x%x",
> event->mask);
>
> /* Arguments required by an event */
> @@ -344,25 +344,25 @@ static ssize_t arm_ccn_pmu_event_show(struct device *dev,
> case CCN_TYPE_CYCLES:
> break;
> case CCN_TYPE_XP:
> - res += snprintf(buf + res, PAGE_SIZE - res,
> + res += scnprintf(buf + res, PAGE_SIZE - res,
> ",xp=?,vc=?");
> if (event->event == CCN_EVENT_WATCHPOINT)
> - res += snprintf(buf + res, PAGE_SIZE - res,
> + res += scnprintf(buf + res, PAGE_SIZE - res,
> ",port=?,dir=?,cmp_l=?,cmp_h=?,mask=?");
> else
> - res += snprintf(buf + res, PAGE_SIZE - res,
> + res += scnprintf(buf + res, PAGE_SIZE - res,
> ",bus=?");
>
> break;
> case CCN_TYPE_MN:
> - res += snprintf(buf + res, PAGE_SIZE - res, ",node=%d", ccn->mn_id);
> + res += scnprintf(buf + res, PAGE_SIZE - res, ",node=%d", ccn->mn_id);
> break;
> default:
> - res += snprintf(buf + res, PAGE_SIZE - res, ",node=?");
> + res += scnprintf(buf + res, PAGE_SIZE - res, ",node=?");
> break;
> }
>
> - res += snprintf(buf + res, PAGE_SIZE - res, "\n");
> + res += scnprintf(buf + res, PAGE_SIZE - res, "\n");
>
> return res;
> }
> --
> 2.16.4
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] perf: arm-ccn: Use scnprintf() for avoiding potential buffer overflow
2020-03-13 17:49 ` Mark Rutland
@ 2020-03-15 8:50 ` Takashi Iwai
0 siblings, 0 replies; 3+ messages in thread
From: Takashi Iwai @ 2020-03-15 8:50 UTC (permalink / raw)
To: Mark Rutland; +Cc: Will Deacon, linux-arm-kernel
On Fri, 13 Mar 2020 18:49:23 +0100,
Mark Rutland wrote:
>
> On Wed, Mar 11, 2020 at 10:05:55AM +0100, Takashi Iwai wrote:
> > Since snprintf() returns the would-be-output size instead of the
> > actual output size, the succeeding calls may go beyond the given
> > buffer limit. Fix it by replacing with scnprintf().
>
> The buffer limit is PAGE_SIZE bytes here, so I don't think that we can
> practically go beyond the buffer limit in this code where we only print
> tens of chacters into the buffer. On any system this runs on, PAGE_SIZE
> is at least 4096.
Right, please take this rather as a precaution.
> I'm happy to make this change as it's generally the right thing to do,
> but I do want the commit message to be very clear that this is a
> cleanup, and not a fix for a bug in practice.
>
> Can you please either reword the commit message to that effect, or if
> this can occur in practice, explain how?
OK, will resubmit with rephrasing.
thanks,
Takashi
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-15 8:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-11 9:05 [PATCH] perf: arm-ccn: Use scnprintf() for avoiding potential buffer overflow Takashi Iwai
2020-03-13 17:49 ` Mark Rutland
2020-03-15 8:50 ` Takashi Iwai
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.