From nobody Fri Dec 19 15:49:19 2025 Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 58C6F1A5BA3; Mon, 19 May 2025 10:43:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.53 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747651415; cv=none; b=OqOkRhpTny/a84WqtIh4qSEhiP1xC5HOVjbVOqHEYOzlkJTOk2Kk2H4mF6Ti57SSLibi7Zut7bfvY1b7+bSZP2gSLOtlB0U3Fl5N1NAlzA7gaYuxY/aIk66+txNxMIjFskegPTi18v9lKHjzgHiH4IefdIOz6uIb3ibZ4608R4k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747651415; c=relaxed/simple; bh=5DZJQH9f+EDCNfMqJwWp25Moj1Eypz5H0FohbsMrH8M=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=rMTTyfI9/+CN9q4YV7CtPIuvZzeClcQTZko3GtPV41osfyqsy2a7a4cD5WKBclrV8IhVq3eozS9GIWegF7uBDZKi+FLHaeB9pnF6V+9EmovJH4hN3kbnQtkpPkwK3OalFiPkUIlvobq7dfN7ULIN3nL82JCMuKNaHtSkNahtjss= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ZY0D3jiw; arc=none smtp.client-ip=209.85.218.53 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ZY0D3jiw" Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-ad55d6aeb07so250985066b.0; Mon, 19 May 2025 03:43:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1747651411; x=1748256211; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=YQM7iljAqtUi8QBWa7n8hdQ0xpGaEntDhRn6guivn0Q=; b=ZY0D3jiwGwMTgyl+FMFYQcF21bm92SlMB/HJEgwP6x7p6ZKePS9RyRYi0Qa4/IpY60 KdMq3rJQiJ8RWSapMYZqhUrRN3Q91yENRmNgADO7HC4wgHoxT/8Vq+U8hc9/sjY11gfw Vh4GogU5QRhFZiNXrrp6EoK1nrBDXEkxoSRllMfVXSuohagxWyATHako32VMGg9XgC/5 8Yn2eZRKivlvzwJjNyd+8IFJdZxThHhy4rsR/3amAT8l1Z4ClyCChxlh4MDRRU42FhIb tVUjxPffcbLa55PjmxT9CYnMLJA5zW+GwPjovpVPLzMGgscw6sqE/uqzHrHg0PUiMt65 UlKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747651411; x=1748256211; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=YQM7iljAqtUi8QBWa7n8hdQ0xpGaEntDhRn6guivn0Q=; b=GmF9fO1qWaMiFGtO9mWlFWPks9Ftt73CsJ7cDNeSbGtKtJRSDAi0h9av28pRH7FcsK pMSspthNnIiR8DZuT8Tspz+UMK/LAdPaSYFt7/OYRC04ifhMKvwSjLhlB2pXJOmhvKo6 AAxD7pCpSeJH842nuUJkXGVs/lhfppAOa771K5HJqmP3PRQ3o446IbMd2S6WshhEamHe /QnMrRvvR6KR3h4txUZgK4utCSpSqg3LDkVmIe087w5zukkaQ4d4cpSixMmdOG1LNW0U e6wPdz+fo3ya2Q5jgYPnmeE+zcbn1j8OMJY5QyWWjDVYj5TSe+zsd2UpyL62jG+Fmpl3 EP2Q== X-Gm-Message-State: AOJu0YwrKoitW61/PXg6+JgJRckikEPTswexoZG5NTcTRyQc0M7uVGXk 9szk6WviwjUsPa+lrPoNG8LE7pQL6I0SdN61KF+6eE6Mwp3cjEzdGJ/bx42W8cSn X-Gm-Gg: ASbGncvF2WZAXHkS3vqjof/sTafqF3sJjIEJcTSHFW6t1KBAD+e2Tn03oA7aAtd5LyU p4cthSngaUcPJpF+3mPfk8fHAE2S3DPTLSGWNsdD2FH+P1fiJgXQzRbSTewKP4wElmzkILSOTfr QlIf5qjzTqpTwiefxYVj1wlLn+LT0nC9uL+w8+7sXo/G3UFLog4dik/yi5bo95ycLSoD+S1500M gNj0eOTM15Lw6wKH+NS9dgPnsgAaw7lZt9De3DC5Al/S6XC3mukFui488/m7ov0xg9e0gCoJGUj sKLYfdItsZHTRfNof7yaZzoJe//9BewrIqXYUTq0J/zrIGZDNznKclj1T+Z+ye8xnKflkFedpQ8 9EZlTDXFjT3g= X-Google-Smtp-Source: AGHT+IEQvL/8apnFGoJ+7YEnVzq8HintJc/e15UdD76320QhiHZWBMWwA6zkp3+FowBYXIBiBCd3MQ== X-Received: by 2002:a17:906:c14d:b0:ad2:26f0:a76 with SMTP id a640c23a62f3a-ad536b7f1d2mr1047148166b.13.1747651399879; Mon, 19 May 2025 03:43:19 -0700 (PDT) Received: from localhost.localdomain ([102.164.100.0]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ad52d498909sm566682766b.126.2025.05.19.03.43.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 May 2025 03:43:19 -0700 (PDT) From: monderasdor@gmail.com To: linux-ide@vger.kernel.org Cc: linux-kernel@vger.kernel.org Subject: [PATCH] ahci: enhance error handling and resource management in ahci_init_one Date: Mon, 19 May 2025 03:43:04 -0700 Message-ID: <20250519104304.1828-1-monderasdor@gmail.com> X-Mailer: git-send-email 2.49.0.windows.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Problem: The current implementation of ahci_init_one has several issues with error h= andling and resource management. It lacks proper error cleanup paths, doesn't initi= alize pointers to NULL, and has inconsistent error handling patterns throughout t= he code. This can lead to resource leaks and make debugging initialization failures = difficult. Solution: This patch enhances the error handling and resource management in ahci_init= _one by: - Adding comprehensive error checking with descriptive error messages - Improving error propagation through return codes - Adding proper error cleanup paths for all resource allocations - Initializing pointers to NULL to prevent use-after-free bugs - Implementing proper cleanup of allocated resources in error paths - Adding more descriptive error messages for all failure points - Including error codes in log messages for better diagnostics - Adding warning messages for potential system issues - Improving code structure with proper error handling paths - Adding proper error return labels - Making code more maintainable with consistent error handling patterns Technical Details: - Added proper initialization of pointers (hpriv, host, mmio) to NULL - Added error cleanup paths with proper resource release - Improved error messages to include specific error codes - Added proper error handling for all resource allocation failures - Added proper cleanup of allocated resources in error paths - Improved code organization with clear error handling paths - Added proper error return labels for better code flow Signed-off-by: Alexander Roman monderasdor@gmail.com --- drivers/ata/ahci.c | 150 ++++++++++++++++++++++++++++++--------------------= --- 1 file changed, 85 insertions(+), 65 deletions(-) diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -1611,460 +1611,555 @@ static int ahci_init_one(struct pci_dev *pdev, co= nst struct pci_device_id *ent) { struct ahci_host_priv *hpriv =3D NULL; struct ata_host *host =3D NULL; void __iomem *mmio =3D NULL; + int n_ports, i, rc =3D -ENOMEM; - int n_ports, i, rc; u32 tmp, cap, port_map; u32 saved_cap; struct device *dev =3D &pdev->dev; VPRINTK("ahci_init_one enter\n"); + /* acquire resources with proper error handling */ - /* acquire resources */ rc =3D pcim_enable_device(pdev); if (rc) { + dev_err(dev, "Failed to enable PCI device: %d\n", rc); + goto err_out; - return rc; } rc =3D pcim_iomap_regions(pdev, 1 << AHCI_PCI_BAR_STANDARD, DRV_NAME); if (rc) { + dev_err(dev, "Failed to map PCI regions: %d\n", rc); + goto err_out; - return rc; } mmio =3D pcim_iomap_table(pdev)[AHCI_PCI_BAR_STANDARD]; + if (!mmio) { + dev_err(dev, "Failed to get mmio table\n"); + rc =3D -ENOMEM; + goto err_out; + } rc =3D pci_alloc_irq_vectors(pdev, 1, AHCI_MAX_PORTS, PCI_IRQ_ALL_TYPE= S); if (rc < 0) { + dev_err(dev, "Failed to allocate IRQ vectors: %d\n", rc); + goto err_out; - return rc; } + /* allocate and initialize host private data */ hpriv =3D devm_kzalloc(dev, sizeof(*hpriv), GFP_KERNEL); if (!hpriv) { + dev_err(dev, "Failed to allocate host private data\n"); + goto err_out; - return -ENOMEM; } hpriv->mmio =3D mmio; hpriv->flags =3D (unsigned long)ent->driver_data; hpriv->irq =3D pdev->irq; + /* apply board quirks */ if (pdev->vendor =3D=3D PCI_VENDOR_ID_INTEL) { + rc =3D ahci_intel_pcs_quirk(pdev, hpriv); + if (rc) { + dev_err(dev, "Failed to apply Intel PCS quirk: %d\n", rc); + goto err_out; + } - ahci_intel_pcs_quirk(pdev, hpriv); } + /* apply port map mask if present */ ahci_get_port_map_mask(dev, hpriv); + /* save initial config */ rc =3D ahci_pci_save_initial_config(pdev, hpriv); if (rc) { + dev_err(dev, "Failed to save initial configuration: %d\n", rc); + goto err_out; - return rc; } + /* prepare host */ cap =3D hpriv->cap; saved_cap =3D cap; port_map =3D hpriv->port_map; n_ports =3D ahci_calc_n_ports(cap, port_map); host =3D ata_host_alloc_pinfo(dev, ahci_port_info + ent->driver_data, = n_ports); if (!host) { + dev_err(dev, "Failed to allocate ATA host\n"); + goto err_out; - return -ENOMEM; } host->private_data =3D hpriv; + /* configure DMA masks */ rc =3D ahci_configure_dma_masks(pdev, hpriv); if (rc) { + dev_err(dev, "Failed to configure DMA masks: %d\n", rc); + goto err_host; - return rc; } + /* initialize adapter */ ahci_pci_init_controller(host); rc =3D ahci_reset_controller(host); if (rc) { + dev_err(dev, "Failed to reset controller: %d\n", rc); + goto err_host; - return rc; } + /* apply fixups for broken systems */ if (ahci_broken_system_poweroff(pdev)) { + dev_err(dev, "WARNING: System may need to power cycle after shutdo= wn\n"); - dev_info(dev, "quirky BIOS, skipping spindown on poweroff\n"); } + /* configure LPM policy */ for (i =3D 0; i < n_ports; i++) { ahci_update_initial_lpm_policy(host->ports[i]); } + /* apply platform-specific workarounds */ if (pdev->vendor =3D=3D PCI_VENDOR_ID_INTEL) { + rc =3D ahci_intel_pcs_quirk(pdev, hpriv); + if (rc) { + dev_err(dev, "Failed to apply Intel PCS quirk: %d\n", rc); + goto err_host; + } - ahci_intel_pcs_quirk(pdev, hpriv); } + /* apply Apple MCP89 workaround */ if (is_mcp89_apple(pdev)) { + rc =3D ahci_mcp89_apple_enable(pdev); + if (rc) { + dev_err(dev, "Failed to enable MCP89 Apple: %d\n", rc); + goto err_host; + } - ahci_mcp89_apple_enable(pdev); } + /* apply Acer SA5-271 workaround */ acer_sa5_271_workaround(hpriv, pdev); + /* initialize and enable interrupts */ ahci_init_irq(pdev, n_ports, hpriv); ahci_pci_enable_interrupts(host); + /* print information */ ahci_pci_print_info(host); + /* register with libata */ rc =3D ata_host_activate(host, hpriv->irq, ahci_interrupt, IRQF_SHARED, + &ahci_sht); - &ahci_sht); if (rc) { + dev_err(dev, "Failed to activate ATA host: %d\n", rc); + goto err_host; - return rc; } return 0; +err_host: + if (host) + ata_host_detach(host); +err_out: + return rc; - return 0; }