| b.liu | e958203 | 2025-04-17 19:18:16 +0800 | [diff] [blame] | 1 | From 1ffba0c4d1122688268e59832a5e2bbc0917cac7 Mon Sep 17 00:00:00 2001 | 
 | 2 | From: Camelia Groza <camelia.groza@nxp.com> | 
 | 3 | Date: Wed, 3 Oct 2018 16:37:06 +0300 | 
 | 4 | Subject: [PATCH] sdk_dpaa: ceetm: avoid double frees on error paths | 
 | 5 |  | 
 | 6 | The stack calls the destroy() callback when a qdisc init() fails. | 
 | 7 | We stop calling it ourselves and trust the stack do the cleanup. | 
 | 8 |  | 
 | 9 | Signed-off-by: Camelia Groza <camelia.groza@nxp.com> | 
 | 10 | --- | 
 | 11 |  .../ethernet/freescale/sdk_dpaa/dpaa_eth_ceetm.c   | 94 ++++++++-------------- | 
 | 12 |  1 file changed, 34 insertions(+), 60 deletions(-) | 
 | 13 |  | 
 | 14 | --- a/drivers/net/ethernet/freescale/sdk_dpaa/dpaa_eth_ceetm.c | 
 | 15 | +++ b/drivers/net/ethernet/freescale/sdk_dpaa/dpaa_eth_ceetm.c | 
 | 16 | @@ -687,16 +687,13 @@ static int ceetm_init_root(struct Qdisc | 
 | 17 |   | 
 | 18 |  	/* Validate inputs */ | 
 | 19 |  	if (sch->parent != TC_H_ROOT) { | 
 | 20 | -		pr_err("CEETM: a root ceetm qdisc can not be attached to a class\n"); | 
 | 21 | -		tcf_block_put(priv->block); | 
 | 22 | -		qdisc_class_hash_destroy(&priv->clhash); | 
 | 23 | +		pr_err("CEETM: a root ceetm qdisc must be root\n"); | 
 | 24 |  		return -EINVAL; | 
 | 25 |  	} | 
 | 26 |   | 
 | 27 |  	if (!mac_dev) { | 
 | 28 |  		pr_err("CEETM: the interface is lacking a mac\n"); | 
 | 29 | -		err = -EINVAL; | 
 | 30 | -		goto err_init_root; | 
 | 31 | +		return -EINVAL; | 
 | 32 |  	} | 
 | 33 |   | 
 | 34 |  	/* Pre-allocate underlying pfifo qdiscs. | 
 | 35 | @@ -713,8 +710,7 @@ static int ceetm_init_root(struct Qdisc | 
 | 36 |  				    sizeof(priv->root.qdiscs[0]), | 
 | 37 |  				    GFP_KERNEL); | 
 | 38 |  	if (!priv->root.qdiscs) { | 
 | 39 | -		err = -ENOMEM; | 
 | 40 | -		goto err_init_root; | 
 | 41 | +		return -ENOMEM; | 
 | 42 |  	} | 
 | 43 |   | 
 | 44 |  	for (i = 0; i < dev->num_tx_queues; i++) { | 
 | 45 | @@ -724,10 +720,8 @@ static int ceetm_init_root(struct Qdisc | 
 | 46 |   | 
 | 47 |  		qdisc = qdisc_create_dflt(dev_queue, &pfifo_qdisc_ops, | 
 | 48 |  					  parent_id, extack); | 
 | 49 | -		if (!qdisc) { | 
 | 50 | -			err = -ENOMEM; | 
 | 51 | -			goto err_init_root; | 
 | 52 | -		} | 
 | 53 | +		if (!qdisc) | 
 | 54 | +			return -ENOMEM; | 
 | 55 |   | 
 | 56 |  		priv->root.qdiscs[i] = qdisc; | 
 | 57 |  		qdisc->flags |= TCQ_F_ONETXQUEUE; | 
 | 58 | @@ -739,8 +733,7 @@ static int ceetm_init_root(struct Qdisc | 
 | 59 |  	if (!priv->root.qstats) { | 
 | 60 |  		pr_err(KBUILD_BASENAME " : %s : alloc_percpu() failed\n", | 
 | 61 |  		       __func__); | 
 | 62 | -		err = -ENOMEM; | 
 | 63 | -		goto err_init_root; | 
 | 64 | +		return -ENOMEM; | 
 | 65 |  	} | 
 | 66 |   | 
 | 67 |  	priv->shaped = qopt->shaped; | 
 | 68 | @@ -754,7 +747,7 @@ static int ceetm_init_root(struct Qdisc | 
 | 69 |  	if (err) { | 
 | 70 |  		pr_err(KBUILD_BASENAME " : %s : failed to claim the SP\n", | 
 | 71 |  		       __func__); | 
 | 72 | -		goto err_init_root; | 
 | 73 | +		return err; | 
 | 74 |  	} | 
 | 75 |   | 
 | 76 |  	priv->root.sp = sp; | 
 | 77 | @@ -766,7 +759,7 @@ static int ceetm_init_root(struct Qdisc | 
 | 78 |  	if (err) { | 
 | 79 |  		pr_err(KBUILD_BASENAME " : %s : failed to claim the LNI\n", | 
 | 80 |  		       __func__); | 
 | 81 | -		goto err_init_root; | 
 | 82 | +		return err; | 
 | 83 |  	} | 
 | 84 |   | 
 | 85 |  	priv->root.lni = lni; | 
 | 86 | @@ -775,7 +768,7 @@ static int ceetm_init_root(struct Qdisc | 
 | 87 |  	if (err) { | 
 | 88 |  		pr_err(KBUILD_BASENAME " : %s : failed to link the SP and LNI\n", | 
 | 89 |  		       __func__); | 
 | 90 | -		goto err_init_root; | 
 | 91 | +		return err; | 
 | 92 |  	} | 
 | 93 |   | 
 | 94 |  	lni->sp = sp; | 
 | 95 | @@ -786,7 +779,7 @@ static int ceetm_init_root(struct Qdisc | 
 | 96 |  		if (err) { | 
 | 97 |  			pr_err(KBUILD_BASENAME " : %s : failed to configure the LNI shaper\n", | 
 | 98 |  			       __func__); | 
 | 99 | -			goto err_init_root; | 
 | 100 | +			return err; | 
 | 101 |  		} | 
 | 102 |   | 
 | 103 |  		bps = priv->root.rate << 3; /* Bps -> bps */ | 
 | 104 | @@ -794,7 +787,7 @@ static int ceetm_init_root(struct Qdisc | 
 | 105 |  		if (err) { | 
 | 106 |  			pr_err(KBUILD_BASENAME " : %s : failed to configure the LNI shaper\n", | 
 | 107 |  			       __func__); | 
 | 108 | -			goto err_init_root; | 
 | 109 | +			return err; | 
 | 110 |  		} | 
 | 111 |   | 
 | 112 |  		bps = priv->root.ceil << 3; /* Bps -> bps */ | 
 | 113 | @@ -802,7 +795,7 @@ static int ceetm_init_root(struct Qdisc | 
 | 114 |  		if (err) { | 
 | 115 |  			pr_err(KBUILD_BASENAME " : %s : failed to configure the LNI shaper\n", | 
 | 116 |  			       __func__); | 
 | 117 | -			goto err_init_root; | 
 | 118 | +			return err; | 
 | 119 |  		} | 
 | 120 |  	} | 
 | 121 |   | 
 | 122 | @@ -810,10 +803,6 @@ static int ceetm_init_root(struct Qdisc | 
 | 123 |   | 
 | 124 |  	dpa_enable_ceetm(dev); | 
 | 125 |  	return 0; | 
 | 126 | - | 
 | 127 | -err_init_root: | 
 | 128 | -	ceetm_destroy(sch); | 
 | 129 | -	return err; | 
 | 130 |  } | 
 | 131 |   | 
 | 132 |  /* Configure a prio ceetm qdisc */ | 
 | 133 | @@ -830,15 +819,13 @@ static int ceetm_init_prio(struct Qdisc | 
 | 134 |   | 
 | 135 |  	if (sch->parent == TC_H_ROOT) { | 
 | 136 |  		pr_err("CEETM: a prio ceetm qdisc can not be root\n"); | 
 | 137 | -		err = -EINVAL; | 
 | 138 | -		goto err_init_prio; | 
 | 139 | +		return -EINVAL; | 
 | 140 |  	} | 
 | 141 |   | 
 | 142 |  	parent_qdisc = qdisc_lookup(dev, TC_H_MAJ(sch->parent)); | 
 | 143 |  	if (strcmp(parent_qdisc->ops->id, ceetm_qdisc_ops.id)) { | 
 | 144 |  		pr_err("CEETM: a ceetm qdisc can not be attached to other qdisc/class types\n"); | 
 | 145 | -		err = -EINVAL; | 
 | 146 | -		goto err_init_prio; | 
 | 147 | +		return -EINVAL; | 
 | 148 |  	} | 
 | 149 |   | 
 | 150 |  	/* Obtain the parent root ceetm_class */ | 
 | 151 | @@ -846,8 +833,7 @@ static int ceetm_init_prio(struct Qdisc | 
 | 152 |   | 
 | 153 |  	if (!parent_cl || parent_cl->type != CEETM_ROOT) { | 
 | 154 |  		pr_err("CEETM: a prio ceetm qdiscs can be added only under a root ceetm class\n"); | 
 | 155 | -		err = -EINVAL; | 
 | 156 | -		goto err_init_prio; | 
 | 157 | +		return -EINVAL; | 
 | 158 |  	} | 
 | 159 |   | 
 | 160 |  	priv->prio.parent = parent_cl; | 
 | 161 | @@ -863,8 +849,7 @@ static int ceetm_init_prio(struct Qdisc | 
 | 162 |  		if (!child_cl) { | 
 | 163 |  			pr_err(KBUILD_BASENAME " : %s : kzalloc() failed\n", | 
 | 164 |  			       __func__); | 
 | 165 | -			err = -ENOMEM; | 
 | 166 | -			goto err_init_prio; | 
 | 167 | +			return -ENOMEM; | 
 | 168 |  		} | 
 | 169 |   | 
 | 170 |  		child_cl->prio.cstats = alloc_percpu(struct ceetm_class_stats); | 
 | 171 | @@ -907,8 +892,7 @@ static int ceetm_init_prio(struct Qdisc | 
 | 172 |   | 
 | 173 |  err_init_prio_cls: | 
 | 174 |  	ceetm_cls_destroy(sch, child_cl); | 
 | 175 | -err_init_prio: | 
 | 176 | -	ceetm_destroy(sch); | 
 | 177 | +	/* Note: ceetm_destroy() will be called by our caller */ | 
 | 178 |  	return err; | 
 | 179 |  } | 
 | 180 |   | 
 | 181 | @@ -928,16 +912,14 @@ static int ceetm_init_wbfs(struct Qdisc | 
 | 182 |  	/* Validate inputs */ | 
 | 183 |  	if (sch->parent == TC_H_ROOT) { | 
 | 184 |  		pr_err("CEETM: a wbfs ceetm qdiscs can not be root\n"); | 
 | 185 | -		err = -EINVAL; | 
 | 186 | -		goto err_init_wbfs; | 
 | 187 | +		return -EINVAL; | 
 | 188 |  	} | 
 | 189 |   | 
 | 190 |  	/* Obtain the parent prio ceetm qdisc */ | 
 | 191 |  	parent_qdisc = qdisc_lookup(dev, TC_H_MAJ(sch->parent)); | 
 | 192 |  	if (strcmp(parent_qdisc->ops->id, ceetm_qdisc_ops.id)) { | 
 | 193 |  		pr_err("CEETM: a ceetm qdisc can not be attached to other qdisc/class types\n"); | 
 | 194 | -		err = -EINVAL; | 
 | 195 | -		goto err_init_wbfs; | 
 | 196 | +		return -EINVAL; | 
 | 197 |  	} | 
 | 198 |   | 
 | 199 |  	/* Obtain the parent prio ceetm class */ | 
 | 200 | @@ -946,28 +928,24 @@ static int ceetm_init_wbfs(struct Qdisc | 
 | 201 |   | 
 | 202 |  	if (!parent_cl || parent_cl->type != CEETM_PRIO) { | 
 | 203 |  		pr_err("CEETM: a wbfs ceetm qdiscs can be added only under a prio ceetm class\n"); | 
 | 204 | -		err = -EINVAL; | 
 | 205 | -		goto err_init_wbfs; | 
 | 206 | +		return -EINVAL; | 
 | 207 |  	} | 
 | 208 |   | 
 | 209 |  	if (!qopt->qcount || !qopt->qweight[0]) { | 
 | 210 |  		pr_err("CEETM: qcount and qweight are mandatory for a wbfs ceetm qdisc\n"); | 
 | 211 | -		err = -EINVAL; | 
 | 212 | -		goto err_init_wbfs; | 
 | 213 | +		return -EINVAL; | 
 | 214 |  	} | 
 | 215 |   | 
 | 216 |  	priv->shaped = parent_cl->shaped; | 
 | 217 |   | 
 | 218 |  	if (!priv->shaped && (qopt->cr || qopt->er)) { | 
 | 219 |  		pr_err("CEETM: CR/ER can be enabled only for shaped wbfs ceetm qdiscs\n"); | 
 | 220 | -		err = -EINVAL; | 
 | 221 | -		goto err_init_wbfs; | 
 | 222 | +		return -EINVAL; | 
 | 223 |  	} | 
 | 224 |   | 
 | 225 |  	if (priv->shaped && !(qopt->cr || qopt->er)) { | 
 | 226 |  		pr_err("CEETM: either CR or ER must be enabled for shaped wbfs ceetm qdiscs\n"); | 
 | 227 | -		err = -EINVAL; | 
 | 228 | -		goto err_init_wbfs; | 
 | 229 | +		return -EINVAL; | 
 | 230 |  	} | 
 | 231 |   | 
 | 232 |  	/* Obtain the parent root ceetm class */ | 
 | 233 | @@ -975,16 +953,14 @@ static int ceetm_init_wbfs(struct Qdisc | 
 | 234 |  	if ((root_cl->root.wbfs_grp_a && root_cl->root.wbfs_grp_b) || | 
 | 235 |  	    root_cl->root.wbfs_grp_large) { | 
 | 236 |  		pr_err("CEETM: no more wbfs classes are available\n"); | 
 | 237 | -		err = -EINVAL; | 
 | 238 | -		goto err_init_wbfs; | 
 | 239 | +		return -EINVAL; | 
 | 240 |  	} | 
 | 241 |   | 
 | 242 |  	if ((root_cl->root.wbfs_grp_a || root_cl->root.wbfs_grp_b) && | 
 | 243 |  	    qopt->qcount == CEETM_MAX_WBFS_QCOUNT) { | 
 | 244 |  		pr_err("CEETM: only %d wbfs classes are available\n", | 
 | 245 |  		       CEETM_MIN_WBFS_QCOUNT); | 
 | 246 | -		err = -EINVAL; | 
 | 247 | -		goto err_init_wbfs; | 
 | 248 | +		return -EINVAL; | 
 | 249 |  	} | 
 | 250 |   | 
 | 251 |  	priv->wbfs.parent = parent_cl; | 
 | 252 | @@ -1013,7 +989,7 @@ static int ceetm_init_wbfs(struct Qdisc | 
 | 253 |  		if (err) { | 
 | 254 |  			pr_err(KBUILD_BASENAME " : %s : failed to get group details\n", | 
 | 255 |  			       __func__); | 
 | 256 | -			goto err_init_wbfs; | 
 | 257 | +			return err; | 
 | 258 |  		} | 
 | 259 |   | 
 | 260 |  		small_group = true; | 
 | 261 | @@ -1031,7 +1007,7 @@ static int ceetm_init_wbfs(struct Qdisc | 
 | 262 |  		if (err) { | 
 | 263 |  			pr_err(KBUILD_BASENAME " : %s : failed to get group details\n", | 
 | 264 |  			       __func__); | 
 | 265 | -			goto err_init_wbfs; | 
 | 266 | +			return err; | 
 | 267 |  		} | 
 | 268 |   | 
 | 269 |  		small_group = true; | 
 | 270 | @@ -1044,7 +1020,7 @@ static int ceetm_init_wbfs(struct Qdisc | 
 | 271 |  	err = qman_ceetm_channel_set_group(priv->wbfs.ch, small_group, prio_a, | 
 | 272 |  					   prio_b); | 
 | 273 |  	if (err) | 
 | 274 | -		goto err_init_wbfs; | 
 | 275 | +		return err; | 
 | 276 |   | 
 | 277 |  	if (priv->shaped) { | 
 | 278 |  		err = qman_ceetm_channel_set_group_cr_eligibility(priv->wbfs.ch, | 
 | 279 | @@ -1053,7 +1029,7 @@ static int ceetm_init_wbfs(struct Qdisc | 
 | 280 |  		if (err) { | 
 | 281 |  			pr_err(KBUILD_BASENAME " : %s : failed to set group CR eligibility\n", | 
 | 282 |  			       __func__); | 
 | 283 | -			goto err_init_wbfs; | 
 | 284 | +			return err; | 
 | 285 |  		} | 
 | 286 |   | 
 | 287 |  		err = qman_ceetm_channel_set_group_er_eligibility(priv->wbfs.ch, | 
 | 288 | @@ -1062,7 +1038,7 @@ static int ceetm_init_wbfs(struct Qdisc | 
 | 289 |  		if (err) { | 
 | 290 |  			pr_err(KBUILD_BASENAME " : %s : failed to set group ER eligibility\n", | 
 | 291 |  			       __func__); | 
 | 292 | -			goto err_init_wbfs; | 
 | 293 | +			return err; | 
 | 294 |  		} | 
 | 295 |  	} | 
 | 296 |   | 
 | 297 | @@ -1072,8 +1048,7 @@ static int ceetm_init_wbfs(struct Qdisc | 
 | 298 |  		if (!child_cl) { | 
 | 299 |  			pr_err(KBUILD_BASENAME " : %s : kzalloc() failed\n", | 
 | 300 |  			       __func__); | 
 | 301 | -			err = -ENOMEM; | 
 | 302 | -			goto err_init_wbfs; | 
 | 303 | +			return -ENOMEM; | 
 | 304 |  		} | 
 | 305 |   | 
 | 306 |  		child_cl->wbfs.cstats = alloc_percpu(struct ceetm_class_stats); | 
 | 307 | @@ -1130,8 +1105,7 @@ static int ceetm_init_wbfs(struct Qdisc | 
 | 308 |   | 
 | 309 |  err_init_wbfs_cls: | 
 | 310 |  	ceetm_cls_destroy(sch, child_cl); | 
 | 311 | -err_init_wbfs: | 
 | 312 | -	ceetm_destroy(sch); | 
 | 313 | +	/* Note: ceetm_destroy() will be called by our caller */ | 
 | 314 |  	return err; | 
 | 315 |  } | 
 | 316 |   | 
 | 317 | @@ -1202,7 +1176,7 @@ static int ceetm_init(struct Qdisc *sch, | 
 | 318 |  		break; | 
 | 319 |  	default: | 
 | 320 |  		pr_err(KBUILD_BASENAME " : %s : invalid qdisc\n", __func__); | 
 | 321 | -		ceetm_destroy(sch); | 
 | 322 | +		/* Note: ceetm_destroy() will be called by our caller */ | 
 | 323 |  		ret = -EINVAL; | 
 | 324 |  	} | 
 | 325 |   | 
 | 326 | @@ -1549,7 +1523,7 @@ static int ceetm_cls_change(struct Qdisc | 
 | 327 |  	} | 
 | 328 |   | 
 | 329 |  	if (!cl && priv->type != CEETM_ROOT) { | 
 | 330 | -		pr_err("CEETM: only root ceetm classes can be attached to the root ceetm qdisc\n"); | 
 | 331 | +		pr_err("CEETM: root ceetm classes can be attached to the root ceetm qdisc only\n"); | 
 | 332 |  		return -EINVAL; | 
 | 333 |  	} | 
 | 334 |   |