| Message ID | 20230420175952.1114302-1-ivecera@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net/sched: cls_api: Initialize miss_cookie_node when action miss is not used | expand |
On 20/04/2023 14:59, Ivan Vecera wrote: > Function tcf_exts_init_ex() sets exts->miss_cookie_node ptr only > when use_action_miss is true so it assumes in other case that > the field is set to NULL by the caller. If not then the field > contains garbage and subsequent tcf_exts_destroy() call results > in a crash. > Initialize .miss_cookie_node pointer to NULL when use_action_miss > parameter is false to avoid this potential scenario. > > Fixes: 80cd22c35c90 ("net/sched: cls_api: Support hardware miss to tc action") > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > net/sched/cls_api.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c > index 35785a36c80298..8bc5b9d6a2916e 100644 > --- a/net/sched/cls_api.c > +++ b/net/sched/cls_api.c > @@ -3224,8 +3224,12 @@ int tcf_exts_init_ex(struct tcf_exts *exts, struct net *net, int action, > exts->action = action; > exts->police = police; > > - if (!use_action_miss) > + if (!use_action_miss) { > +#ifdef CONFIG_NET_CLS_ACT > + exts->miss_cookie_node = NULL; > +#endif > return 0; > + } > > err = tcf_exts_miss_cookie_base_alloc(exts, tp, handle); > if (err) The problem described here also happens in the case some error happens if the action array allocation fails and before the 'miss_cookie_node' assignment inside 'tcf_exts_miss_cookie_base_alloc()'. Seems like a better way to solve this issue is to just: diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 35785a36c802..3c3629c9e7b6 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -3211,6 +3211,7 @@ int tcf_exts_init_ex(struct tcf_exts *exts, struct net *net, int action, #ifdef CONFIG_NET_CLS_ACT exts->type = 0; exts->nr_actions = 0; + exts->miss_cookie_node = NULL; /* Note: we do not own yet a reference on net. * This reference might be taken later from tcf_exts_get_net(). */
On 20. 04. 23 20:21, Pedro Tammela wrote: > On 20/04/2023 14:59, Ivan Vecera wrote: >> Function tcf_exts_init_ex() sets exts->miss_cookie_node ptr only >> when use_action_miss is true so it assumes in other case that >> the field is set to NULL by the caller. If not then the field >> contains garbage and subsequent tcf_exts_destroy() call results >> in a crash. >> Initialize .miss_cookie_node pointer to NULL when use_action_miss >> parameter is false to avoid this potential scenario. >> >> Fixes: 80cd22c35c90 ("net/sched: cls_api: Support hardware miss to tc >> action") >> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >> --- >> net/sched/cls_api.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c >> index 35785a36c80298..8bc5b9d6a2916e 100644 >> --- a/net/sched/cls_api.c >> +++ b/net/sched/cls_api.c >> @@ -3224,8 +3224,12 @@ int tcf_exts_init_ex(struct tcf_exts *exts, >> struct net *net, int action, >> exts->action = action; >> exts->police = police; >> - if (!use_action_miss) >> + if (!use_action_miss) { >> +#ifdef CONFIG_NET_CLS_ACT >> + exts->miss_cookie_node = NULL; >> +#endif >> return 0; >> + } >> err = tcf_exts_miss_cookie_base_alloc(exts, tp, handle); >> if (err) > > The problem described here also happens in the case some error happens > if the action array allocation fails and before the 'miss_cookie_node' > assignment inside 'tcf_exts_miss_cookie_base_alloc()'. > > Seems like a better way to solve this issue is to just: > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c > index 35785a36c802..3c3629c9e7b6 100644 > --- a/net/sched/cls_api.c > +++ b/net/sched/cls_api.c > @@ -3211,6 +3211,7 @@ int tcf_exts_init_ex(struct tcf_exts *exts, struct > net *net, int action, > #ifdef CONFIG_NET_CLS_ACT > exts->type = 0; > exts->nr_actions = 0; > + exts->miss_cookie_node = NULL; > /* Note: we do not own yet a reference on net. > * This reference might be taken later from tcf_exts_get_net(). > */ Looks better, will repost... Thanks Ivan
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 35785a36c80298..8bc5b9d6a2916e 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -3224,8 +3224,12 @@ int tcf_exts_init_ex(struct tcf_exts *exts, struct net *net, int action, exts->action = action; exts->police = police; - if (!use_action_miss) + if (!use_action_miss) { +#ifdef CONFIG_NET_CLS_ACT + exts->miss_cookie_node = NULL; +#endif return 0; + } err = tcf_exts_miss_cookie_base_alloc(exts, tp, handle); if (err)
Function tcf_exts_init_ex() sets exts->miss_cookie_node ptr only when use_action_miss is true so it assumes in other case that the field is set to NULL by the caller. If not then the field contains garbage and subsequent tcf_exts_destroy() call results in a crash. Initialize .miss_cookie_node pointer to NULL when use_action_miss parameter is false to avoid this potential scenario. Fixes: 80cd22c35c90 ("net/sched: cls_api: Support hardware miss to tc action") Signed-off-by: Ivan Vecera <ivecera@redhat.com> --- net/sched/cls_api.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)