Re: [Patch net] net_sched: add max len check for TCA_KIND
From: David Ahern <hidden>
Date: 2019-09-19 02:41:55
On 9/18/19 5:24 PM, Cong Wang wrote:
The TCA_KIND attribute is of NLA_STRING which does not check
the NUL char. KMSAN reported an uninit-value of TCA_KIND which
is likely caused by the lack of NUL.
Change it to NLA_NUL_STRING and add a max len too.
Fixes: 8b4c3cdd9dd8 ("net: sched: Add policy validation for tc attributes")The commit referenced here did not introduce the ability to go beyond memory boundaries with string comparisons. Rather, it was not complete solution for attribute validation. I say that wrt to the fix getting propagated to the correct stable releases.
Reported-and-tested-by: syzbot+618aacd49e8c8b8486bd@syzkaller.appspotmail.com
What is the actual sysbot report?
quoted hunk ↗ jump to hunk
Cc: David Ahern <redacted> Cc: Jamal Hadi Salim <jhs@mojatatu.com> Cc: Jiri Pirko <jiri@resnulli.us> Signed-off-by: Cong Wang <redacted> --- net/sched/sch_api.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index 1047825d9f48..81d58b280612 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c@@ -1390,7 +1390,8 @@ check_loop_fn(struct Qdisc *q, unsigned long cl, struct qdisc_walker *w) } const struct nla_policy rtm_tca_policy[TCA_MAX + 1] = { - [TCA_KIND] = { .type = NLA_STRING }, + [TCA_KIND] = { .type = NLA_NUL_STRING, + .len = IFNAMSIZ - 1 }, [TCA_RATE] = { .type = NLA_BINARY, .len = sizeof(struct tc_estimator) }, [TCA_STAB] = { .type = NLA_NESTED },
This is a better policy so for that: Reviewed-by: David Ahern <redacted>