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

platform/x86/amd/pmf: fix cleanup in amd_pmf_init_smart_pc()

There are a few problems in this code:

First, if amd_pmf_tee_init() fails then the function returns directly
instead of cleaning up. We cannot simply do a "goto error;" because
the amd_pmf_tee_init() cleanup calls tee_shm_free(dev->fw_shm_pool);
and amd_pmf_tee_deinit() calls it as well leading to a double free.
I have re-written this code to use an unwind ladder to free the
allocations.

Second, if amd_pmf_start_policy_engine() fails on every iteration though
the loop then the code calls amd_pmf_tee_deinit() twice which is also a
double free. Call amd_pmf_tee_deinit() inside the loop for each failed
iteration. Also on that path the error codes are not necessarily
negative kernel error codes. Set the error code to -EINVAL.

There is a very subtle third bug which is that if the call to
input_register_device() in amd_pmf_register_input_device() fails then
we call input_unregister_device() on an input device that wasn't
registered. This will lead to a reference counting underflow
because of the device_del(&dev->dev) in __input_unregister_device().
It's unlikely that anyone would ever hit this bug in real life.

Fixes: 376a8c2a1443 ("platform/x86/amd/pmf: Update PMF Driver for Compatibility with new PMF-TA")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/r/232231fc-6a71-495e-971b-be2a76f6db4c@stanley.mountain
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

authored by

Dan Carpenter and committed by
Ilpo Järvinen
5b1122fc 376a8c2a

+25 -11
+25 -11
drivers/platform/x86/amd/pmf/tee-if.c
··· 510 510 511 511 ret = amd_pmf_set_dram_addr(dev, true); 512 512 if (ret) 513 - goto error; 513 + goto err_cancel_work; 514 514 515 515 dev->policy_base = devm_ioremap_resource(dev->dev, dev->res); 516 516 if (IS_ERR(dev->policy_base)) { 517 517 ret = PTR_ERR(dev->policy_base); 518 - goto error; 518 + goto err_free_dram_buf; 519 519 } 520 520 521 521 dev->policy_buf = kzalloc(dev->policy_sz, GFP_KERNEL); 522 522 if (!dev->policy_buf) { 523 523 ret = -ENOMEM; 524 - goto error; 524 + goto err_free_dram_buf; 525 525 } 526 526 527 527 memcpy_fromio(dev->policy_buf, dev->policy_base, dev->policy_sz); ··· 531 531 dev->prev_data = kzalloc(sizeof(*dev->prev_data), GFP_KERNEL); 532 532 if (!dev->prev_data) { 533 533 ret = -ENOMEM; 534 - goto error; 534 + goto err_free_policy; 535 535 } 536 536 537 537 for (i = 0; i < ARRAY_SIZE(amd_pmf_ta_uuid); i++) { 538 538 ret = amd_pmf_tee_init(dev, &amd_pmf_ta_uuid[i]); 539 539 if (ret) 540 - return ret; 540 + goto err_free_prev_data; 541 541 542 542 ret = amd_pmf_start_policy_engine(dev); 543 543 switch (ret) { ··· 550 550 status = false; 551 551 break; 552 552 default: 553 - goto error; 553 + ret = -EINVAL; 554 + amd_pmf_tee_deinit(dev); 555 + goto err_free_prev_data; 554 556 } 555 557 556 558 if (status) 557 559 break; 558 560 } 559 561 560 - if (!status && !pb_side_load) 561 - goto error; 562 + if (!status && !pb_side_load) { 563 + ret = -EINVAL; 564 + goto err_free_prev_data; 565 + } 562 566 563 567 if (pb_side_load) 564 568 amd_pmf_open_pb(dev, dev->dbgfs_dir); 565 569 566 570 ret = amd_pmf_register_input_device(dev); 567 571 if (ret) 568 - goto error; 572 + goto err_pmf_remove_pb; 569 573 570 574 return 0; 571 575 572 - error: 573 - amd_pmf_deinit_smart_pc(dev); 576 + err_pmf_remove_pb: 577 + if (pb_side_load && dev->esbin) 578 + amd_pmf_remove_pb(dev); 579 + amd_pmf_tee_deinit(dev); 580 + err_free_prev_data: 581 + kfree(dev->prev_data); 582 + err_free_policy: 583 + kfree(dev->policy_buf); 584 + err_free_dram_buf: 585 + kfree(dev->buf); 586 + err_cancel_work: 587 + cancel_delayed_work_sync(&dev->pb_work); 574 588 575 589 return ret; 576 590 }