* [PATCH] mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats
@ 2014-10-17 18:34 Karl Beldan
2014-10-18 9:27 ` Felix Fietkau
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Karl Beldan @ 2014-10-17 18:34 UTC (permalink / raw)
To: Johannes Berg; +Cc: Karl Beldan, linux-wireless, karl.beldan, Felix Fietkau
From: Karl Beldan <karl.beldan@rivierawaves.com>
ATM an HT rc_stats line is 106 chars.
Times 8(MCS_GROUP_RATES)*3(SS)*2(GI)*2(BW) + CCK(4), i.e. x100, this is
well above the current 8192 - sizeof(*ms) currently allocated.
Fix this by squeezing the output as follows (not that we're short on
memory but this also improves readability and range, the new format adds
one more digit to *ok/*cum and ok/cum):
- Before (HT) (106 ch):
type rate throughput ewma prob this prob retry this succ/attempt success attempts
CCK/LP 5.5M 0.0 0.0 0.0 0 0( 0) 0 0
HT20/LGI ABCDP MCS0 0.0 0.0 0.0 1 0( 0) 0 0
- After (75 ch):
type rate tpt eprob *prob ret *ok(*cum) ok( cum)
CCK/LP 5.5M 0.0 0.0 0.0 0 0( 0) 0( 0)
HT20/LGI ABCDP MCS0 0.0 0.0 0.0 1 0( 0) 0( 0)
- Align non-HT format Before (non-HT) (83 ch):
rate throughput ewma prob this prob this succ/attempt success attempts
ABCDP 6 0.0 0.0 0.0 0( 0) 0 0
54 0.0 0.0 0.0 0( 0) 0 0
- After (61 ch):
rate tpt eprob *prob *ok(*cum) ok( cum)
ABCDP 1 0.0 0.0 0.0 0( 0) 0( 0)
54 0.0 0.0 0.0 0( 0) 0( 0)
*This also adds dynamic checks for overflow, lowers the size of the
non-HT request (allowing > 30 entries) and replaces the buddy-rounded
allocations (s/sizeof(*ms) + 8192/8192).
Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Cc: Felix Fietkau <nbd@openwrt.org>
---
Send this one-line patch to stable ?
- ms = kmalloc(sizeof(*ms) + 8192, GFP_KERNEL);
+ ms = kmalloc(16384, GFP_KERNEL);
net/mac80211/rc80211_minstrel_debugfs.c | 12 +++++++-----
net/mac80211/rc80211_minstrel_ht_debugfs.c | 13 ++++++++-----
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
index edde723..b33be0f 100644
--- a/net/mac80211/rc80211_minstrel_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_debugfs.c
@@ -62,14 +62,14 @@ minstrel_stats_open(struct inode *inode, struct file *file)
unsigned int i, tp, prob, eprob;
char *p;
- ms = kmalloc(sizeof(*ms) + 4096, GFP_KERNEL);
+ ms = kmalloc(2048, GFP_KERNEL);
if (!ms)
return -ENOMEM;
file->private_data = ms;
p = ms->buf;
- p += sprintf(p, "rate throughput ewma prob this prob "
- "this succ/attempt success attempts\n");
+ p += sprintf(p, "rate tpt eprob *prob"
+ " *ok(*cum) ok( cum)\n");
for (i = 0; i < mi->n_rates; i++) {
struct minstrel_rate *mr = &mi->r[i];
struct minstrel_rate_stats *mrs = &mi->r[i].stats;
@@ -86,8 +86,8 @@ minstrel_stats_open(struct inode *inode, struct file *file)
prob = MINSTREL_TRUNC(mrs->cur_prob * 1000);
eprob = MINSTREL_TRUNC(mrs->probability * 1000);
- p += sprintf(p, " %6u.%1u %6u.%1u %6u.%1u "
- " %3u(%3u) %8llu %8llu\n",
+ p += sprintf(p, " %4u.%1u %3u.%1u %3u.%1u"
+ " %4u(%4u) %9llu(%9llu)\n",
tp / 10, tp % 10,
eprob / 10, eprob % 10,
prob / 10, prob % 10,
@@ -102,6 +102,8 @@ minstrel_stats_open(struct inode *inode, struct file *file)
mi->sample_packets);
ms->len = p - ms->buf;
+ WARN_ON(ms->len > 2048 - sizeof(*ms));
+
return 0;
}
diff --git a/net/mac80211/rc80211_minstrel_ht_debugfs.c b/net/mac80211/rc80211_minstrel_ht_debugfs.c
index a72ad46..b51b4ec 100644
--- a/net/mac80211/rc80211_minstrel_ht_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_ht_debugfs.c
@@ -63,8 +63,8 @@ minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
prob = MINSTREL_TRUNC(mr->cur_prob * 1000);
eprob = MINSTREL_TRUNC(mr->probability * 1000);
- p += sprintf(p, " %6u.%1u %6u.%1u %6u.%1u "
- "%3u %3u(%3u) %8llu %8llu\n",
+ p += sprintf(p, " %4u.%1u %3u.%1u %3u.%1u "
+ "%3u %4u(%4u) %9llu(%9llu)\n",
tp / 10, tp % 10,
eprob / 10, eprob % 10,
prob / 10, prob % 10,
@@ -96,14 +96,15 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
return ret;
}
- ms = kmalloc(sizeof(*ms) + 8192, GFP_KERNEL);
+ ms = kmalloc(8192, GFP_KERNEL);
if (!ms)
return -ENOMEM;
file->private_data = ms;
p = ms->buf;
- p += sprintf(p, "type rate throughput ewma prob "
- "this prob retry this succ/attempt success attempts\n");
+ p += sprintf(p, "type rate tpt eprob *prob "
+ "ret *ok(*cum) ok( cum)\n");
+
p = minstrel_ht_stats_dump(mi, max_mcs, p);
for (i = 0; i < max_mcs; i++)
@@ -118,6 +119,8 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
MINSTREL_TRUNC(mi->avg_ampdu_len * 10) % 10);
ms->len = p - ms->buf;
+ WARN_ON(ms->len > 8192 - sizeof(*ms));
+
return nonseekable_open(inode, file);
}
--
2.0.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats
2014-10-17 18:34 [PATCH] mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats Karl Beldan
@ 2014-10-18 9:27 ` Felix Fietkau
2014-10-19 10:22 ` Karl Beldan
2014-10-20 8:54 ` [PATCH v2] " Karl Beldan
2 siblings, 0 replies; 6+ messages in thread
From: Felix Fietkau @ 2014-10-18 9:27 UTC (permalink / raw)
To: Karl Beldan, Johannes Berg; +Cc: Karl Beldan, linux-wireless
On 2014-10-17 20:34, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
>
> ATM an HT rc_stats line is 106 chars.
> Times 8(MCS_GROUP_RATES)*3(SS)*2(GI)*2(BW) + CCK(4), i.e. x100, this is
> well above the current 8192 - sizeof(*ms) currently allocated.
>
> Fix this by squeezing the output as follows (not that we're short on
> memory but this also improves readability and range, the new format adds
> one more digit to *ok/*cum and ok/cum):
>
> - Before (HT) (106 ch):
> type rate throughput ewma prob this prob retry this succ/attempt success attempts
> CCK/LP 5.5M 0.0 0.0 0.0 0 0( 0) 0 0
> HT20/LGI ABCDP MCS0 0.0 0.0 0.0 1 0( 0) 0 0
> - After (75 ch):
> type rate tpt eprob *prob ret *ok(*cum) ok( cum)
> CCK/LP 5.5M 0.0 0.0 0.0 0 0( 0) 0( 0)
> HT20/LGI ABCDP MCS0 0.0 0.0 0.0 1 0( 0) 0( 0)
>
> - Align non-HT format Before (non-HT) (83 ch):
> rate throughput ewma prob this prob this succ/attempt success attempts
> ABCDP 6 0.0 0.0 0.0 0( 0) 0 0
> 54 0.0 0.0 0.0 0( 0) 0 0
> - After (61 ch):
> rate tpt eprob *prob *ok(*cum) ok( cum)
> ABCDP 1 0.0 0.0 0.0 0( 0) 0( 0)
> 54 0.0 0.0 0.0 0( 0) 0( 0)
>
> *This also adds dynamic checks for overflow, lowers the size of the
> non-HT request (allowing > 30 entries) and replaces the buddy-rounded
> allocations (s/sizeof(*ms) + 8192/8192).
>
> Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
> Cc: Felix Fietkau <nbd@openwrt.org>
Acked-by: Felix Fietkau <nbd@openwrt.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats
2014-10-17 18:34 [PATCH] mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats Karl Beldan
2014-10-18 9:27 ` Felix Fietkau
@ 2014-10-19 10:22 ` Karl Beldan
2014-10-20 7:28 ` Johannes Berg
2014-10-20 8:54 ` [PATCH v2] " Karl Beldan
2 siblings, 1 reply; 6+ messages in thread
From: Karl Beldan @ 2014-10-19 10:22 UTC (permalink / raw)
To: Johannes Berg; +Cc: Karl Beldan, linux-wireless, Felix Fietkau
Hi,
If that's not too late I'd like to fix some coding style, replace:
On Fri, Oct 17, 2014 at 08:34:00PM +0200, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
> @@ -102,6 +102,8 @@ minstrel_stats_open(struct inode *inode, struct file *file)
> mi->sample_packets);
> ms->len = p - ms->buf;
>
> + WARN_ON(ms->len > 2048 - sizeof(*ms));
with:
WARN_ON(ms->len + sizeof(*ms) > 2048);
and,
> @@ -118,6 +119,8 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
> MINSTREL_TRUNC(mi->avg_ampdu_len * 10) % 10);
> ms->len = p - ms->buf;
>
> + WARN_ON(ms->len > 8192 - sizeof(*ms));
with:
WARN_ON(ms->len + sizeof(*ms) > 8192);
This would require rebasing "[PATCH v3 4/4] mac80211: minstrel_ht: add
basic support for VHT rates <= 3SS@80MHz", and send
[PATCH v2] mac80211: minstrels: fix buffer overflow in HT debugfs
[PATCH v4 1/4] mac80211: minstrel_ht: Increase the range of handled
[PATCH v4 2/4] mac80211: minstrel_ht: macros adjustments for future
[PATCH v4 3/4] mac80211: minstrel_ht: include type (cck/ht) in rates
[PATCH v4 4/4] mac80211: minstrel_ht: add basic support for VHT rates
Is it ok ?
Karl
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats
2014-10-19 10:22 ` Karl Beldan
@ 2014-10-20 7:28 ` Johannes Berg
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2014-10-20 7:28 UTC (permalink / raw)
To: Karl Beldan; +Cc: Karl Beldan, linux-wireless, Felix Fietkau
On Sun, 2014-10-19 at 12:22 +0200, Karl Beldan wrote:
> This would require rebasing "[PATCH v3 4/4] mac80211: minstrel_ht: add
> basic support for VHT rates <= 3SS@80MHz", and send
>
> [PATCH v2] mac80211: minstrels: fix buffer overflow in HT debugfs
> [PATCH v4 1/4] mac80211: minstrel_ht: Increase the range of handled
> [PATCH v4 2/4] mac80211: minstrel_ht: macros adjustments for future
> [PATCH v4 3/4] mac80211: minstrel_ht: include type (cck/ht) in rates
> [PATCH v4 4/4] mac80211: minstrel_ht: add basic support for VHT rates
>
> Is it ok ?
Who are you asking? Those are your own patches, no? So are you asking
yourself if you can rebase? ;-)
For me that's fine.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats
2014-10-17 18:34 [PATCH] mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats Karl Beldan
2014-10-18 9:27 ` Felix Fietkau
2014-10-19 10:22 ` Karl Beldan
@ 2014-10-20 8:54 ` Karl Beldan
2014-10-20 14:38 ` Johannes Berg
2 siblings, 1 reply; 6+ messages in thread
From: Karl Beldan @ 2014-10-20 8:54 UTC (permalink / raw)
To: Johannes Berg; +Cc: Karl Beldan, linux-wireless, karl.beldan
From: Karl Beldan <karl.beldan@rivierawaves.com>
ATM an HT rc_stats line is 106 chars.
Times 8(MCS_GROUP_RATES)*3(SS)*2(GI)*2(BW) + CCK(4), i.e. x100, this is
well above the current 8192 - sizeof(*ms) currently allocated.
Fix this by squeezing the output as follows (not that we're short on
memory but this also improves readability and range, the new format adds
one more digit to *ok/*cum and ok/cum):
- Before (HT) (106 ch):
type rate throughput ewma prob this prob retry this succ/attempt success attempts
CCK/LP 5.5M 0.0 0.0 0.0 0 0( 0) 0 0
HT20/LGI ABCDP MCS0 0.0 0.0 0.0 1 0( 0) 0 0
- After (75 ch):
type rate tpt eprob *prob ret *ok(*cum) ok( cum)
CCK/LP 5.5M 0.0 0.0 0.0 0 0( 0) 0( 0)
HT20/LGI ABCDP MCS0 0.0 0.0 0.0 1 0( 0) 0( 0)
- Align non-HT format Before (non-HT) (83 ch):
rate throughput ewma prob this prob this succ/attempt success attempts
ABCDP 6 0.0 0.0 0.0 0( 0) 0 0
54 0.0 0.0 0.0 0( 0) 0 0
- After (61 ch):
rate tpt eprob *prob *ok(*cum) ok( cum)
ABCDP 1 0.0 0.0 0.0 0( 0) 0( 0)
54 0.0 0.0 0.0 0( 0) 0( 0)
*This also adds dynamic checks for overflow, lowers the size of the
non-HT request (allowing > 30 entries) and replaces the buddy-rounded
allocations (s/sizeof(*ms) + 8192/8192).
Signed-off-by: Karl Beldan <karl.beldan@rivierawaves.com>
Acked-by: Felix Fietkau <nbd@openwrt.org>
---
v2:
- s/WARN_ON(ms->len > X - sizeof(*ms))/WARN_ON(ms->len + sizeof(*ms) > X)/
net/mac80211/rc80211_minstrel_debugfs.c | 12 +++++++-----
net/mac80211/rc80211_minstrel_ht_debugfs.c | 13 ++++++++-----
2 files changed, 15 insertions(+), 10 deletions(-)
diff --git a/net/mac80211/rc80211_minstrel_debugfs.c b/net/mac80211/rc80211_minstrel_debugfs.c
index edde723..2acab1b 100644
--- a/net/mac80211/rc80211_minstrel_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_debugfs.c
@@ -62,14 +62,14 @@ minstrel_stats_open(struct inode *inode, struct file *file)
unsigned int i, tp, prob, eprob;
char *p;
- ms = kmalloc(sizeof(*ms) + 4096, GFP_KERNEL);
+ ms = kmalloc(2048, GFP_KERNEL);
if (!ms)
return -ENOMEM;
file->private_data = ms;
p = ms->buf;
- p += sprintf(p, "rate throughput ewma prob this prob "
- "this succ/attempt success attempts\n");
+ p += sprintf(p, "rate tpt eprob *prob"
+ " *ok(*cum) ok( cum)\n");
for (i = 0; i < mi->n_rates; i++) {
struct minstrel_rate *mr = &mi->r[i];
struct minstrel_rate_stats *mrs = &mi->r[i].stats;
@@ -86,8 +86,8 @@ minstrel_stats_open(struct inode *inode, struct file *file)
prob = MINSTREL_TRUNC(mrs->cur_prob * 1000);
eprob = MINSTREL_TRUNC(mrs->probability * 1000);
- p += sprintf(p, " %6u.%1u %6u.%1u %6u.%1u "
- " %3u(%3u) %8llu %8llu\n",
+ p += sprintf(p, " %4u.%1u %3u.%1u %3u.%1u"
+ " %4u(%4u) %9llu(%9llu)\n",
tp / 10, tp % 10,
eprob / 10, eprob % 10,
prob / 10, prob % 10,
@@ -102,6 +102,8 @@ minstrel_stats_open(struct inode *inode, struct file *file)
mi->sample_packets);
ms->len = p - ms->buf;
+ WARN_ON(ms->len + sizeof(*ms) > 2048);
+
return 0;
}
diff --git a/net/mac80211/rc80211_minstrel_ht_debugfs.c b/net/mac80211/rc80211_minstrel_ht_debugfs.c
index a72ad46..d537bec 100644
--- a/net/mac80211/rc80211_minstrel_ht_debugfs.c
+++ b/net/mac80211/rc80211_minstrel_ht_debugfs.c
@@ -63,8 +63,8 @@ minstrel_ht_stats_dump(struct minstrel_ht_sta *mi, int i, char *p)
prob = MINSTREL_TRUNC(mr->cur_prob * 1000);
eprob = MINSTREL_TRUNC(mr->probability * 1000);
- p += sprintf(p, " %6u.%1u %6u.%1u %6u.%1u "
- "%3u %3u(%3u) %8llu %8llu\n",
+ p += sprintf(p, " %4u.%1u %3u.%1u %3u.%1u "
+ "%3u %4u(%4u) %9llu(%9llu)\n",
tp / 10, tp % 10,
eprob / 10, eprob % 10,
prob / 10, prob % 10,
@@ -96,14 +96,15 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
return ret;
}
- ms = kmalloc(sizeof(*ms) + 8192, GFP_KERNEL);
+ ms = kmalloc(8192, GFP_KERNEL);
if (!ms)
return -ENOMEM;
file->private_data = ms;
p = ms->buf;
- p += sprintf(p, "type rate throughput ewma prob "
- "this prob retry this succ/attempt success attempts\n");
+ p += sprintf(p, "type rate tpt eprob *prob "
+ "ret *ok(*cum) ok( cum)\n");
+
p = minstrel_ht_stats_dump(mi, max_mcs, p);
for (i = 0; i < max_mcs; i++)
@@ -118,6 +119,8 @@ minstrel_ht_stats_open(struct inode *inode, struct file *file)
MINSTREL_TRUNC(mi->avg_ampdu_len * 10) % 10);
ms->len = p - ms->buf;
+ WARN_ON(ms->len + sizeof(*ms) > 8192);
+
return nonseekable_open(inode, file);
}
--
2.0.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats
2014-10-20 8:54 ` [PATCH v2] " Karl Beldan
@ 2014-10-20 14:38 ` Johannes Berg
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2014-10-20 14:38 UTC (permalink / raw)
To: Karl Beldan; +Cc: Karl Beldan, linux-wireless
On Mon, 2014-10-20 at 10:54 +0200, Karl Beldan wrote:
> From: Karl Beldan <karl.beldan@rivierawaves.com>
>
> ATM an HT rc_stats line is 106 chars.
> Times 8(MCS_GROUP_RATES)*3(SS)*2(GI)*2(BW) + CCK(4), i.e. x100, this is
> well above the current 8192 - sizeof(*ms) currently allocated.
Applied.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-20 14:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-17 18:34 [PATCH] mac80211: minstrels: fix buffer overflow in HT debugfs rc_stats Karl Beldan
2014-10-18 9:27 ` Felix Fietkau
2014-10-19 10:22 ` Karl Beldan
2014-10-20 7:28 ` Johannes Berg
2014-10-20 8:54 ` [PATCH v2] " Karl Beldan
2014-10-20 14:38 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).