| Message ID | 20190215121120.4971-1-vladbu@mellanox.com |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: sched: matchall: verify that filter is not NULL in mall_walk() | expand |
On Fri, Feb 15, 2019 at 02:11:20PM +0200, Vlad Buslov wrote: > Check that filter is not NULL before passing it to tcf_walker->fn() > callback. This can happen when mall_change() failed to offload filter to > hardware. > > Signed-off-by: Vlad Buslov <vladbu@mellanox.com> Works for me. Thanks Fixes: ed76f5edccc9 ("net: sched: protect filter_chain list with filter_chain_lock mutex") Reported-by: Ido Schimmel <idosch@mellanox.com> Tested-by: Ido Schimmel <idosch@mellanox.com>
On Fri, Feb 15, 2019 at 4:11 AM Vlad Buslov <vladbu@mellanox.com> wrote: > > Check that filter is not NULL before passing it to tcf_walker->fn() > callback. This can happen when mall_change() failed to offload filter to > hardware. > > Signed-off-by: Vlad Buslov <vladbu@mellanox.com> > --- > net/sched/cls_matchall.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c > index a37137430e61..1f9d481b0fbb 100644 > --- a/net/sched/cls_matchall.c > +++ b/net/sched/cls_matchall.c > @@ -247,6 +247,9 @@ static void mall_walk(struct tcf_proto *tp, struct tcf_walker *arg, > > if (arg->count < arg->skip) > goto skip; > + > + if (!head) > + return; So head==NULL still counts one given that you check NULL after checking arg->count. Is this expected?
From: Vlad Buslov <vladbu@mellanox.com> Date: Fri, 15 Feb 2019 14:11:20 +0200 > Check that filter is not NULL before passing it to tcf_walker->fn() > callback. This can happen when mall_change() failed to offload filter to > hardware. > > Signed-off-by: Vlad Buslov <vladbu@mellanox.com> Applied.
On Sat 16 Feb 2019 at 00:24, Cong Wang <xiyou.wangcong@gmail.com> wrote: > On Fri, Feb 15, 2019 at 4:11 AM Vlad Buslov <vladbu@mellanox.com> wrote: >> >> Check that filter is not NULL before passing it to tcf_walker->fn() >> callback. This can happen when mall_change() failed to offload filter to >> hardware. >> >> Signed-off-by: Vlad Buslov <vladbu@mellanox.com> >> --- >> net/sched/cls_matchall.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c >> index a37137430e61..1f9d481b0fbb 100644 >> --- a/net/sched/cls_matchall.c >> +++ b/net/sched/cls_matchall.c >> @@ -247,6 +247,9 @@ static void mall_walk(struct tcf_proto *tp, struct tcf_walker *arg, >> >> if (arg->count < arg->skip) >> goto skip; >> + >> + if (!head) >> + return; > > So head==NULL still counts one given that you check NULL after > checking arg->count. Is this expected? My intention was to fix the problem (arg->fn() call with NULL filter) without changing any other functionality, and always incrementing arg->count once seemed to be the intended behavior. However, since mall_delete() just returns -EOPNOTSUPP, it might be the case that author of matchall expected to always have single filter configured when cls API calls mall_walk(). What would you suggest?
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c index a37137430e61..1f9d481b0fbb 100644 --- a/net/sched/cls_matchall.c +++ b/net/sched/cls_matchall.c @@ -247,6 +247,9 @@ static void mall_walk(struct tcf_proto *tp, struct tcf_walker *arg, if (arg->count < arg->skip) goto skip; + + if (!head) + return; if (arg->fn(tp, head, arg) < 0) arg->stop = 1; skip:
Check that filter is not NULL before passing it to tcf_walker->fn() callback. This can happen when mall_change() failed to offload filter to hardware. Signed-off-by: Vlad Buslov <vladbu@mellanox.com> --- net/sched/cls_matchall.c | 3 +++ 1 file changed, 3 insertions(+)