Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32778 )
Change subject: libpayload: ahci: Prevent memory leaks when failing on init ......................................................................
libpayload: ahci: Prevent memory leaks when failing on init
Free several resources when AHCI initialization fails. Note that it is only safe to free resources when the command engine has stopped, since otherwise they may still be used for DMA.
Found-by: Coverity CID 1260719, 1260727, 1261090, 1261098 Signed-off-by: Jacob Garber jgarber1@ualberta.ca Change-Id: I6826d79338b26ff9696ab6ac9eb4c59f734687d8 Reviewed-on: https://review.coreboot.org/c/coreboot/+/32778 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Reviewed-by: Nico Huber nico.h@gmx.de --- M payloads/libpayload/drivers/storage/ahci.c 1 file changed, 6 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved HAOUAS Elyes: Looks good to me, approved
diff --git a/payloads/libpayload/drivers/storage/ahci.c b/payloads/libpayload/drivers/storage/ahci.c index 40e1008..f74adaf 100644 --- a/payloads/libpayload/drivers/storage/ahci.c +++ b/payloads/libpayload/drivers/storage/ahci.c @@ -108,6 +108,9 @@
const int ncs = HBA_CAPS_DECODE_NCS(ctrl->caps);
+ if (ahci_cmdengine_stop(port)) + return 1; + /* Allocate command list, one command table and received FIS. */ cmd_t *const cmdlist = memalign(1024, ncs * sizeof(cmd_t)); cmdtable_t *const cmdtable = memalign(128, sizeof(cmdtable_t)); @@ -121,12 +124,10 @@ memset((void *)rcvd_fis, '\0', sizeof(*rcvd_fis));
/* Set command list base and received FIS base. */ - if (ahci_cmdengine_stop(port)) - return 1; port->cmdlist_base = virt_to_phys(cmdlist); port->frameinfo_base = virt_to_phys(rcvd_fis); if (ahci_cmdengine_start(port)) - return 1; + goto _cleanup_ret; /* Put port into active state. */ port->cmd_stat |= HBA_PxCMD_ICC_ACTIVE;
@@ -178,6 +179,8 @@ /* Clean up (not reached for initialized devices). */ if (dev) free(dev); + /* Only free if stopping succeeds, since otherwise the controller may + still use the resources for DMA. */ if (!ahci_cmdengine_stop(port)) { port->cmdlist_base = 0; port->frameinfo_base = 0;