Re: [Patch net] net_sched: add max len check for TCA_KIND

[David Ahern <dsahern@gmail.com>]

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?

> Cc: David Ahern <dsahern@gmail.com>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  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 <dsahern@gmail.com>