| Message ID | 1472056742.14381.93.camel@edumazet-glaptop3.roam.corp.google.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On 16-08-24 09:39 AM, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Should qdisc_alloc() fail, we must release the module refcount > we got right before. > > Fixes: 6da7c8fcbcbd ("qdisc: allow setting default queuing discipline") > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > net/sched/sch_generic.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c > index e95b67cd5718..657c13362b19 100644 > --- a/net/sched/sch_generic.c > +++ b/net/sched/sch_generic.c > @@ -643,18 +643,19 @@ struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue, > struct Qdisc *sch; > > if (!try_module_get(ops->owner)) > - goto errout; > + return NULL; > > sch = qdisc_alloc(dev_queue, ops); > - if (IS_ERR(sch)) > - goto errout; > + if (IS_ERR(sch)) { > + module_put(ops->owner); > + return NULL; > + } > sch->parent = parentid; > > if (!ops->init || ops->init(sch, NULL) == 0) > return sch; > > qdisc_destroy(sch); > -errout: > return NULL; > } > EXPORT_SYMBOL(qdisc_create_dflt); > > Thanks! Acked-by: John Fastabend <john.r.fastabend@intel.com>
On 16-08-24 09:39 AM, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Should qdisc_alloc() fail, we must release the module refcount > we got right before. > > Fixes: 6da7c8fcbcbd ("qdisc: allow setting default queuing discipline") > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > net/sched/sch_generic.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c > index e95b67cd5718..657c13362b19 100644 > --- a/net/sched/sch_generic.c > +++ b/net/sched/sch_generic.c > @@ -643,18 +643,19 @@ struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue, > struct Qdisc *sch; > > if (!try_module_get(ops->owner)) > - goto errout; > + return NULL; > > sch = qdisc_alloc(dev_queue, ops); > - if (IS_ERR(sch)) > - goto errout; > + if (IS_ERR(sch)) { > + module_put(ops->owner); > + return NULL; > + } > sch->parent = parentid; > > if (!ops->init || ops->init(sch, NULL) == 0) > return sch; > > qdisc_destroy(sch); > -errout: > return NULL; > } > EXPORT_SYMBOL(qdisc_create_dflt); > > Thanks! Acked-by: John Fastabend <john.r.fastabend@intel.com>
On 08/25/2016 12:39 AM, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Should qdisc_alloc() fail, we must release the module refcount > we got right before. > > Fixes: 6da7c8fcbcbd ("qdisc: allow setting default queuing discipline") > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > net/sched/sch_generic.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c > index e95b67cd5718..657c13362b19 100644 > --- a/net/sched/sch_generic.c > +++ b/net/sched/sch_generic.c > @@ -643,18 +643,19 @@ struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue, > struct Qdisc *sch; > > if (!try_module_get(ops->owner)) > - goto errout; > + return NULL; > > sch = qdisc_alloc(dev_queue, ops); > - if (IS_ERR(sch)) > - goto errout; > + if (IS_ERR(sch)) { > + module_put(ops->owner); > + return NULL; > + } > sch->parent = parentid; > > if (!ops->init || ops->init(sch, NULL) == 0) > return sch; > > qdisc_destroy(sch); ops->init() failed, missing module_put() here. > -errout: > return NULL; > } > EXPORT_SYMBOL(qdisc_create_dflt); > > > . >
On Thu, 2016-08-25 at 21:26 +0800, Wei Yongjun wrote: > On 08/25/2016 12:39 AM, Eric Dumazet wrote: > > From: Eric Dumazet <edumazet@google.com> > > > > Should qdisc_alloc() fail, we must release the module refcount > > we got right before. > > > > Fixes: 6da7c8fcbcbd ("qdisc: allow setting default queuing discipline") > > Signed-off-by: Eric Dumazet <edumazet@google.com> > > --- > > net/sched/sch_generic.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c > > index e95b67cd5718..657c13362b19 100644 > > --- a/net/sched/sch_generic.c > > +++ b/net/sched/sch_generic.c > > @@ -643,18 +643,19 @@ struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue, > > struct Qdisc *sch; > > > > if (!try_module_get(ops->owner)) > > - goto errout; > > + return NULL; > > > > sch = qdisc_alloc(dev_queue, ops); > > - if (IS_ERR(sch)) > > - goto errout; > > + if (IS_ERR(sch)) { > > + module_put(ops->owner); > > + return NULL; > > + } > > sch->parent = parentid; > > > > if (!ops->init || ops->init(sch, NULL) == 0) > > return sch; > > > > qdisc_destroy(sch); > > ops->init() failed, missing module_put() here. qdisc_destroy() is doing this for us, we do not want to have a double module_put()
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Wed, 24 Aug 2016 09:39:02 -0700 > From: Eric Dumazet <edumazet@google.com> > > Should qdisc_alloc() fail, we must release the module refcount > we got right before. > > Fixes: 6da7c8fcbcbd ("qdisc: allow setting default queuing discipline") > Signed-off-by: Eric Dumazet <edumazet@google.com> Applied.
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c index e95b67cd5718..657c13362b19 100644 --- a/net/sched/sch_generic.c +++ b/net/sched/sch_generic.c @@ -643,18 +643,19 @@ struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue, struct Qdisc *sch; if (!try_module_get(ops->owner)) - goto errout; + return NULL; sch = qdisc_alloc(dev_queue, ops); - if (IS_ERR(sch)) - goto errout; + if (IS_ERR(sch)) { + module_put(ops->owner); + return NULL; + } sch->parent = parentid; if (!ops->init || ops->init(sch, NULL) == 0) return sch; qdisc_destroy(sch); -errout: return NULL; } EXPORT_SYMBOL(qdisc_create_dflt);