Re: [PATCH] net: sched: sch: Bounds check priority
From: Simon Horman <hidden>
Date: 2023-01-28 13:43:42
Also in:
linux-hardening, lkml
On Fri, Jan 27, 2023 at 02:40:37PM -0800, Kees Cook wrote:
Nothing was explicitly bounds checking the priority index used to access
clpriop[]. WARN and bail out early if it's pathological. Seen with GCC 13:
../net/sched/sch_htb.c: In function 'htb_activate_prios':
../net/sched/sch_htb.c:437:44: warning: array subscript [0, 31] is outside array bounds of 'struct htb_prio[8]' [-Warray-bounds=]
437 | if (p->inner.clprio[prio].feed.rb_node)
| ~~~~~~~~~~~~~~~^~~~~~
../net/sched/sch_htb.c:131:41: note: while referencing 'clprio'
131 | struct htb_prio clprio[TC_HTB_NUMPRIO];
| ^~~~~~...
Signed-off-by: Kees Cook <redacted> --- net/sched/sch_htb.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
I'm not sure what will happen if we hit the 'break' case. But I also think that warning and bailing out is an improvement on whatever happens now if that scenario is hit. Reviewed-by: Simon Horman <redacted>
quoted hunk ↗ jump to hunk
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index f46643850df8..cc28e41fb745 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c@@ -431,7 +431,10 @@ static void htb_activate_prios(struct htb_sched *q, struct htb_class *cl) while (cl->cmode == HTB_MAY_BORROW && p && mask) { m = mask; while (m) { - int prio = ffz(~m); + unsigned int prio = ffz(~m); + + if (WARN_ON_ONCE(prio > ARRAY_SIZE(p->inner.clprio))) + break; m &= ~(1 << prio); if (p->inner.clprio[prio].feed.rb_node)-- 2.34.1