Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

remoteproc: pru: Fix and cleanup firmware interrupt mapping logic

The PRU firmware interrupt mappings are configured and unconfigured in
.start() and .stop() callbacks respectively using the variables 'evt_count'
and a 'mapped_irq' pointer. These variables are modified only during these
callbacks but are not re-initialized/reset properly during unwind or
failure paths. These stale values caused a kernel crash while stopping a
PRU remoteproc running a different firmware with no events on a subsequent
run after a previous run that was running a firmware with events.

Fix this crash by ensuring that the evt_count is 0 and the mapped_irq
pointer is set to NULL in pru_dispose_irq_mapping(). Also, reset these
variables properly during any failures in the .start() callback. While
at this, the pru_dispose_irq_mapping() callsites are all made to look
the same, moving any conditional logic to inside the function.

Reviewed-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific interrupt configuration")
Reported-by: Vignesh Raghavendra <vigneshr@ti.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
Link: https://lore.kernel.org/r/20210407155641.5501-4-s-anna@ti.com
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>

authored by

Suman Anna and committed by
Bjorn Andersson
880a66e0 1fe72bcf

+17 -5
+17 -5
drivers/remoteproc/pru_rproc.c
··· 266 266 267 267 static void pru_dispose_irq_mapping(struct pru_rproc *pru) 268 268 { 269 - while (pru->evt_count--) { 269 + if (!pru->mapped_irq) 270 + return; 271 + 272 + while (pru->evt_count) { 273 + pru->evt_count--; 270 274 if (pru->mapped_irq[pru->evt_count] > 0) 271 275 irq_dispose_mapping(pru->mapped_irq[pru->evt_count]); 272 276 } 273 277 274 278 kfree(pru->mapped_irq); 279 + pru->mapped_irq = NULL; 275 280 } 276 281 277 282 /* ··· 312 307 pru->evt_count = rsc->num_evts; 313 308 pru->mapped_irq = kcalloc(pru->evt_count, sizeof(unsigned int), 314 309 GFP_KERNEL); 315 - if (!pru->mapped_irq) 310 + if (!pru->mapped_irq) { 311 + pru->evt_count = 0; 316 312 return -ENOMEM; 313 + } 317 314 318 315 /* 319 316 * parse and fill in system event to interrupt channel and ··· 324 317 * corresponding sibling PRUSS INTC node. 325 318 */ 326 319 parent = of_get_parent(dev_of_node(pru->dev)); 327 - if (!parent) 320 + if (!parent) { 321 + kfree(pru->mapped_irq); 322 + pru->mapped_irq = NULL; 323 + pru->evt_count = 0; 328 324 return -ENODEV; 325 + } 329 326 330 327 irq_parent = of_get_child_by_name(parent, "interrupt-controller"); 331 328 of_node_put(parent); 332 329 if (!irq_parent) { 333 330 kfree(pru->mapped_irq); 331 + pru->mapped_irq = NULL; 332 + pru->evt_count = 0; 334 333 return -ENODEV; 335 334 } 336 335 ··· 411 398 pru_control_write_reg(pru, PRU_CTRL_CTRL, val); 412 399 413 400 /* dispose irq mapping - new firmware can provide new mapping */ 414 - if (pru->mapped_irq) 415 - pru_dispose_irq_mapping(pru); 401 + pru_dispose_irq_mapping(pru); 416 402 417 403 return 0; 418 404 }