From 1ffba0c4d1122688268e59832a5e2bbc0917cac7 Mon Sep 17 00:00:00 2001 From: Camelia Groza Date: Wed, 3 Oct 2018 16:37:06 +0300 Subject: [PATCH] sdk_dpaa: ceetm: avoid double frees on error paths The stack calls the destroy() callback when a qdisc init() fails. We stop calling it ourselves and trust the stack do the cleanup. Signed-off-by: Camelia Groza --- .../ethernet/freescale/sdk_dpaa/dpaa_eth_ceetm.c | 94 ++++++++-------------- 1 file changed, 34 insertions(+), 60 deletions(-) --- a/drivers/net/ethernet/freescale/sdk_dpaa/dpaa_eth_ceetm.c +++ b/drivers/net/ethernet/freescale/sdk_dpaa/dpaa_eth_ceetm.c @@ -687,16 +687,13 @@ static int ceetm_init_root(struct Qdisc /* Validate inputs */ if (sch->parent != TC_H_ROOT) { - pr_err("CEETM: a root ceetm qdisc can not be attached to a class\n"); - tcf_block_put(priv->block); - qdisc_class_hash_destroy(&priv->clhash); + pr_err("CEETM: a root ceetm qdisc must be root\n"); return -EINVAL; } if (!mac_dev) { pr_err("CEETM: the interface is lacking a mac\n"); - err = -EINVAL; - goto err_init_root; + return -EINVAL; } /* Pre-allocate underlying pfifo qdiscs. @@ -713,8 +710,7 @@ static int ceetm_init_root(struct Qdisc sizeof(priv->root.qdiscs[0]), GFP_KERNEL); if (!priv->root.qdiscs) { - err = -ENOMEM; - goto err_init_root; + return -ENOMEM; } for (i = 0; i < dev->num_tx_queues; i++) { @@ -724,10 +720,8 @@ static int ceetm_init_root(struct Qdisc qdisc = qdisc_create_dflt(dev_queue, &pfifo_qdisc_ops, parent_id, extack); - if (!qdisc) { - err = -ENOMEM; - goto err_init_root; - } + if (!qdisc) + return -ENOMEM; priv->root.qdiscs[i] = qdisc; qdisc->flags |= TCQ_F_ONETXQUEUE; @@ -739,8 +733,7 @@ static int ceetm_init_root(struct Qdisc if (!priv->root.qstats) { pr_err(KBUILD_BASENAME " : %s : alloc_percpu() failed\n", __func__); - err = -ENOMEM; - goto err_init_root; + return -ENOMEM; } priv->shaped = qopt->shaped; @@ -754,7 +747,7 @@ static int ceetm_init_root(struct Qdisc if (err) { pr_err(KBUILD_BASENAME " : %s : failed to claim the SP\n", __func__); - goto err_init_root; + return err; } priv->root.sp = sp; @@ -766,7 +759,7 @@ static int ceetm_init_root(struct Qdisc if (err) { pr_err(KBUILD_BASENAME " : %s : failed to claim the LNI\n", __func__); - goto err_init_root; + return err; } priv->root.lni = lni; @@ -775,7 +768,7 @@ static int ceetm_init_root(struct Qdisc if (err) { pr_err(KBUILD_BASENAME " : %s : failed to link the SP and LNI\n", __func__); - goto err_init_root; + return err; } lni->sp = sp; @@ -786,7 +779,7 @@ static int ceetm_init_root(struct Qdisc if (err) { pr_err(KBUILD_BASENAME " : %s : failed to configure the LNI shaper\n", __func__); - goto err_init_root; + return err; } bps = priv->root.rate << 3; /* Bps -> bps */ @@ -794,7 +787,7 @@ static int ceetm_init_root(struct Qdisc if (err) { pr_err(KBUILD_BASENAME " : %s : failed to configure the LNI shaper\n", __func__); - goto err_init_root; + return err; } bps = priv->root.ceil << 3; /* Bps -> bps */ @@ -802,7 +795,7 @@ static int ceetm_init_root(struct Qdisc if (err) { pr_err(KBUILD_BASENAME " : %s : failed to configure the LNI shaper\n", __func__); - goto err_init_root; + return err; } } @@ -810,10 +803,6 @@ static int ceetm_init_root(struct Qdisc dpa_enable_ceetm(dev); return 0; - -err_init_root: - ceetm_destroy(sch); - return err; } /* Configure a prio ceetm qdisc */ @@ -830,15 +819,13 @@ static int ceetm_init_prio(struct Qdisc if (sch->parent == TC_H_ROOT) { pr_err("CEETM: a prio ceetm qdisc can not be root\n"); - err = -EINVAL; - goto err_init_prio; + return -EINVAL; } parent_qdisc = qdisc_lookup(dev, TC_H_MAJ(sch->parent)); if (strcmp(parent_qdisc->ops->id, ceetm_qdisc_ops.id)) { pr_err("CEETM: a ceetm qdisc can not be attached to other qdisc/class types\n"); - err = -EINVAL; - goto err_init_prio; + return -EINVAL; } /* Obtain the parent root ceetm_class */ @@ -846,8 +833,7 @@ static int ceetm_init_prio(struct Qdisc if (!parent_cl || parent_cl->type != CEETM_ROOT) { pr_err("CEETM: a prio ceetm qdiscs can be added only under a root ceetm class\n"); - err = -EINVAL; - goto err_init_prio; + return -EINVAL; } priv->prio.parent = parent_cl; @@ -863,8 +849,7 @@ static int ceetm_init_prio(struct Qdisc if (!child_cl) { pr_err(KBUILD_BASENAME " : %s : kzalloc() failed\n", __func__); - err = -ENOMEM; - goto err_init_prio; + return -ENOMEM; } child_cl->prio.cstats = alloc_percpu(struct ceetm_class_stats); @@ -907,8 +892,7 @@ static int ceetm_init_prio(struct Qdisc err_init_prio_cls: ceetm_cls_destroy(sch, child_cl); -err_init_prio: - ceetm_destroy(sch); + /* Note: ceetm_destroy() will be called by our caller */ return err; } @@ -928,16 +912,14 @@ static int ceetm_init_wbfs(struct Qdisc /* Validate inputs */ if (sch->parent == TC_H_ROOT) { pr_err("CEETM: a wbfs ceetm qdiscs can not be root\n"); - err = -EINVAL; - goto err_init_wbfs; + return -EINVAL; } /* Obtain the parent prio ceetm qdisc */ parent_qdisc = qdisc_lookup(dev, TC_H_MAJ(sch->parent)); if (strcmp(parent_qdisc->ops->id, ceetm_qdisc_ops.id)) { pr_err("CEETM: a ceetm qdisc can not be attached to other qdisc/class types\n"); - err = -EINVAL; - goto err_init_wbfs; + return -EINVAL; } /* Obtain the parent prio ceetm class */ @@ -946,28 +928,24 @@ static int ceetm_init_wbfs(struct Qdisc if (!parent_cl || parent_cl->type != CEETM_PRIO) { pr_err("CEETM: a wbfs ceetm qdiscs can be added only under a prio ceetm class\n"); - err = -EINVAL; - goto err_init_wbfs; + return -EINVAL; } if (!qopt->qcount || !qopt->qweight[0]) { pr_err("CEETM: qcount and qweight are mandatory for a wbfs ceetm qdisc\n"); - err = -EINVAL; - goto err_init_wbfs; + return -EINVAL; } priv->shaped = parent_cl->shaped; if (!priv->shaped && (qopt->cr || qopt->er)) { pr_err("CEETM: CR/ER can be enabled only for shaped wbfs ceetm qdiscs\n"); - err = -EINVAL; - goto err_init_wbfs; + return -EINVAL; } if (priv->shaped && !(qopt->cr || qopt->er)) { pr_err("CEETM: either CR or ER must be enabled for shaped wbfs ceetm qdiscs\n"); - err = -EINVAL; - goto err_init_wbfs; + return -EINVAL; } /* Obtain the parent root ceetm class */ @@ -975,16 +953,14 @@ static int ceetm_init_wbfs(struct Qdisc if ((root_cl->root.wbfs_grp_a && root_cl->root.wbfs_grp_b) || root_cl->root.wbfs_grp_large) { pr_err("CEETM: no more wbfs classes are available\n"); - err = -EINVAL; - goto err_init_wbfs; + return -EINVAL; } if ((root_cl->root.wbfs_grp_a || root_cl->root.wbfs_grp_b) && qopt->qcount == CEETM_MAX_WBFS_QCOUNT) { pr_err("CEETM: only %d wbfs classes are available\n", CEETM_MIN_WBFS_QCOUNT); - err = -EINVAL; - goto err_init_wbfs; + return -EINVAL; } priv->wbfs.parent = parent_cl; @@ -1013,7 +989,7 @@ static int ceetm_init_wbfs(struct Qdisc if (err) { pr_err(KBUILD_BASENAME " : %s : failed to get group details\n", __func__); - goto err_init_wbfs; + return err; } small_group = true; @@ -1031,7 +1007,7 @@ static int ceetm_init_wbfs(struct Qdisc if (err) { pr_err(KBUILD_BASENAME " : %s : failed to get group details\n", __func__); - goto err_init_wbfs; + return err; } small_group = true; @@ -1044,7 +1020,7 @@ static int ceetm_init_wbfs(struct Qdisc err = qman_ceetm_channel_set_group(priv->wbfs.ch, small_group, prio_a, prio_b); if (err) - goto err_init_wbfs; + return err; if (priv->shaped) { err = qman_ceetm_channel_set_group_cr_eligibility(priv->wbfs.ch, @@ -1053,7 +1029,7 @@ static int ceetm_init_wbfs(struct Qdisc if (err) { pr_err(KBUILD_BASENAME " : %s : failed to set group CR eligibility\n", __func__); - goto err_init_wbfs; + return err; } err = qman_ceetm_channel_set_group_er_eligibility(priv->wbfs.ch, @@ -1062,7 +1038,7 @@ static int ceetm_init_wbfs(struct Qdisc if (err) { pr_err(KBUILD_BASENAME " : %s : failed to set group ER eligibility\n", __func__); - goto err_init_wbfs; + return err; } } @@ -1072,8 +1048,7 @@ static int ceetm_init_wbfs(struct Qdisc if (!child_cl) { pr_err(KBUILD_BASENAME " : %s : kzalloc() failed\n", __func__); - err = -ENOMEM; - goto err_init_wbfs; + return -ENOMEM; } child_cl->wbfs.cstats = alloc_percpu(struct ceetm_class_stats); @@ -1130,8 +1105,7 @@ static int ceetm_init_wbfs(struct Qdisc err_init_wbfs_cls: ceetm_cls_destroy(sch, child_cl); -err_init_wbfs: - ceetm_destroy(sch); + /* Note: ceetm_destroy() will be called by our caller */ return err; } @@ -1202,7 +1176,7 @@ static int ceetm_init(struct Qdisc *sch, break; default: pr_err(KBUILD_BASENAME " : %s : invalid qdisc\n", __func__); - ceetm_destroy(sch); + /* Note: ceetm_destroy() will be called by our caller */ ret = -EINVAL; } @@ -1549,7 +1523,7 @@ static int ceetm_cls_change(struct Qdisc } if (!cl && priv->type != CEETM_ROOT) { - pr_err("CEETM: only root ceetm classes can be attached to the root ceetm qdisc\n"); + pr_err("CEETM: root ceetm classes can be attached to the root ceetm qdisc only\n"); return -EINVAL; }