Messages in this thread | | | Date | Thu, 18 Aug 2022 10:56:42 -0700 | From | Jakub Kicinski <> | Subject | Re: [PATCH net] net/sched: fix netdevice reference leaks in attach_one_default_qdisc() |
| |
On Wed, 17 Aug 2022 18:46:46 +0800 Wang Hai wrote: > In attach_default_qdiscs(), when attach default qdisc (fq_codel) fails > and fallback to noqueue, if the original attached qdisc is not released > and a new one is directly attached, this will cause netdevice reference > leaks.
Could you provide more details on the failure path? My preference would be to try to clean up properly there, if possible.
> The following is the bug log: > > veth0: default qdisc (fq_codel) fail, fallback to noqueue > unregister_netdevice: waiting for veth0 to become free. Usage count = 32 > leaked reference. > qdisc_alloc+0x12e/0x210 > qdisc_create_dflt+0x62/0x140 > attach_one_default_qdisc.constprop.41+0x44/0x70 > dev_activate+0x128/0x290 > __dev_open+0x12a/0x190 > __dev_change_flags+0x1a2/0x1f0 > dev_change_flags+0x23/0x60 > do_setlink+0x332/0x1150 > __rtnl_newlink+0x52f/0x8e0 > rtnl_newlink+0x43/0x70 > rtnetlink_rcv_msg+0x140/0x3b0 > netlink_rcv_skb+0x50/0x100 > netlink_unicast+0x1bb/0x290 > netlink_sendmsg+0x37c/0x4e0 > sock_sendmsg+0x5f/0x70 > ____sys_sendmsg+0x208/0x280 > > In attach_one_default_qdisc(), release the old one before attaching > a new qdisc to fix this bug. > > Fixes: bf6dba76d278 ("net: sched: fallback to qdisc noqueue if default qdisc setup fail") > Signed-off-by: Wang Hai <wanghai38@huawei.com> > --- > net/sched/sch_generic.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c > index d47b9689eba6..87b61ef14497 100644 > --- a/net/sched/sch_generic.c > +++ b/net/sched/sch_generic.c > @@ -1140,6 +1140,11 @@ static void attach_one_default_qdisc(struct net_device *dev, > > if (!netif_is_multiqueue(dev)) > qdisc->flags |= TCQ_F_ONETXQUEUE | TCQ_F_NOPARENT; > + > + if (dev_queue->qdisc_sleeping && > + dev_queue->qdisc_sleeping != &noop_qdisc) > + qdisc_put(dev_queue->qdisc_sleeping); > + > dev_queue->qdisc_sleeping = qdisc; > } >
| |