| Message ID | 20260824153339.512487-3-p.pisati@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | m68k: pata_cswarp: Add Amiga cslab ata support | expand |
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Missing check for ioremap() failure in pata_cswarp_probe(). - [Medium] Missing iounmap() on the ata_host_activate() error path. -- --- Patch [2]: [PATCH v5 2/2] m68k: defconfig: enable PATA_CSWARP --- commit 50c3007b9ddccb0c0a488126c545810ad538a593 Author: Paolo Pisati <p.pisati@gmail.com> ata: pata_cswarp: Add Amiga cslab ata support This commit introduces the pata_cswarp driver to provide support for the on-board IDE interface on the CS-Lab Warp Expansion Card. > diff --git a/drivers/ata/pata_cswarp.c b/drivers/ata/pata_cswarp.c > new file mode 100644 > index 0000000000000..55f000f427384 > --- /dev/null > +++ b/drivers/ata/pata_cswarp.c > @@ -0,0 +1,180 @@ [ ... ] > +static int pata_cswarp_probe(struct zorro_dev *z, > + const struct zorro_device_id *ent) > +{ [ ... ] > + host = ata_host_alloc(&z->dev, 1); > + if (!host) > + return -ENXIO; > + > + ap = host->ports[0]; > + base = ioremap(board + WARP_OFFSET_ATA, 0x1800); > + > + ap->ops = &pata_cswarp_ops; [Severity: High] Is it possible for ioremap() to fail and return a NULL pointer here? If memory mapping fails during device probe, base would be NULL. This NULL pointer is then used to calculate and assign various ATA port I/O addresses (e.g., ap->ioaddr.error_addr = base + 1 * 4). When the block layer or ATA framework later attempts to access these I/O ports, will this result in a kernel panic due to an invalid pointer dereference? > + > + ap->pio_mask = ATA_PIO4; [ ... ] > + ata_port_desc(ap, " cmd 0x%lx ctl 0x%lx", (unsigned long)base, > + (unsigned long)ap->ioaddr.ctl_addr); > + > + return ata_host_activate(host, 0, NULL, > + IRQF_SHARED, &pata_cswarp_sht); > +} [Severity: Medium] Does this error path leak the I/O memory mapping? If ata_host_activate() fails, it returns a non-zero error code which is immediately returned by pata_cswarp_probe(). Since this driver uses manual memory mapping rather than devm_ioremap() (as seen by the manual iounmap() call in pata_cswarp_remove()), should iounmap(base) be called before returning the error?
diff --git a/arch/m68k/configs/amiga_defconfig b/arch/m68k/configs/amiga_defconfig index 5ed08e0fdf88..b769be5a0cbb 100644 --- a/arch/m68k/configs/amiga_defconfig +++ b/arch/m68k/configs/amiga_defconfig @@ -284,6 +284,7 @@ CONFIG_ATA=y # CONFIG_ATA_VERBOSE_ERROR is not set # CONFIG_ATA_BMDMA is not set CONFIG_PATA_GAYLE=y +CONFIG_PATA_CSWARP=y CONFIG_PATA_BUDDHA=y CONFIG_MD=y CONFIG_MD_LINEAR=m diff --git a/arch/m68k/configs/multi_defconfig b/arch/m68k/configs/multi_defconfig index 615e2dcad3d3..078d71e9b7cb 100644 --- a/arch/m68k/configs/multi_defconfig +++ b/arch/m68k/configs/multi_defconfig @@ -310,6 +310,7 @@ CONFIG_ATA=y # CONFIG_ATA_BMDMA is not set CONFIG_PATA_FALCON=y CONFIG_PATA_GAYLE=y +CONFIG_PATA_CSWARP=y CONFIG_PATA_BUDDHA=y CONFIG_PATA_PLATFORM=y CONFIG_MD=y