Messages in this thread Patch in this message | | | From | Eric Dumazet <> | Date | Wed, 31 May 2023 17:03:37 +0200 | Subject | Re: [PATCH 1/1] net/sched: cls_u32: Fix reference counter leak leading to overflow |
| |
On Wed, May 31, 2023 at 4:16 PM Lee Jones <lee@kernel.org> wrote: > > In the event of a failure in tcf_change_indev(), u32_set_parms() will > immediately return without decrementing the recently incremented > reference counter. If this happens enough times, the counter will > rollover and the reference freed, leading to a double free which can be > used to do 'bad things'. > > Cc: stable@kernel.org # v4.14+
Please add a Fixes: tag.
> Signed-off-by: Lee Jones <lee@kernel.org> > --- > net/sched/cls_u32.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c > index 4e2e269f121f8..fad61ca5e90bf 100644 > --- a/net/sched/cls_u32.c > +++ b/net/sched/cls_u32.c > @@ -762,8 +762,11 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp, > if (tb[TCA_U32_INDEV]) { > int ret; > ret = tcf_change_indev(net, tb[TCA_U32_INDEV], extack);
This call should probably be done earlier in the function, next to tcf_exts_validate_ex()
Otherwise we might ask why the tcf_bind_filter() does not need to be undone.
Something like:
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 4e2e269f121f8a301368b9783753e055f5af6a4e..ac957ff2216ae18bcabdd3af3b0e127447ef8f91 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -718,13 +718,18 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp, struct nlattr *est, u32 flags, u32 fl_flags, struct netlink_ext_ack *extack) { - int err; + int err, ifindex = -1;
err = tcf_exts_validate_ex(net, tp, tb, est, &n->exts, flags, fl_flags, extack); if (err < 0) return err;
+ if (tb[TCA_U32_INDEV]) { + ifindex = tcf_change_indev(net, tb[TCA_U32_INDEV], extack); + if (ifindex < 0) + return -EINVAL; + } if (tb[TCA_U32_LINK]) { u32 handle = nla_get_u32(tb[TCA_U32_LINK]); struct tc_u_hnode *ht_down = NULL, *ht_old; @@ -759,13 +764,9 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp, tcf_bind_filter(tp, &n->res, base); }
- if (tb[TCA_U32_INDEV]) { - int ret; - ret = tcf_change_indev(net, tb[TCA_U32_INDEV], extack); - if (ret < 0) - return -EINVAL; - n->ifindex = ret; - } + if (ifindex >= 0) + n->ifindex = ifindex; + return 0; }
| |