Linux ATA/IDE development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Paolo Pisati" <p.pisati@gmail.com>
Cc: linux-ide@vger.kernel.org, cassel@kernel.org, dlemoal@kernel.org
Subject: Re: [PATCH v5 1/2] ata: pata_cswarp: Add Amiga cslab ata support
Date: Mon, 24 Aug 2026 15:41:51 +0000	[thread overview]
Message-ID: <20260824154151.9F41B1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260824153339.512487-2-p.pisati@gmail.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing NULL check for ioremap() return value.
- [Medium] Resource leak of I/O mapping on probe error path.
--

--- Patch [1]: [PATCH v5 1/2] ata: pata_cswarp: Add Amiga cslab ata support ---
Note: The format of this report is altered due to recitation restrictions. I
will not quote the original patch code directly, but will instead provide a
free-form summary of the findings.

commit 50c3007b9ddccb0c0a488126c545810ad538a593
Author: Paolo Pisati <p.pisati@gmail.com>

ata: pata_cswarp: Add Amiga cslab ata support

This commit adds support for the on-board IDE interface on the CS-Lab Warp
Expansion Card. It introduces the pata_cswarp driver handling initialization,
data transfer, and configuration for PIO mode.

[Severity: High]
In drivers/ata/pata_cswarp.c:pata_cswarp_probe(), the code calls ioremap()
but does not check the return value before using it.

Could ioremap() return NULL under memory pressure or if the vmalloc area is
exhausted? If so, using the returned base pointer without validation to 
calculate static offsets (such as base + 1 * 4) and assigning them to the 
ATA port's IO address structure might lead to an immediate kernel oops or 
exception vector table corruption on m68k systems. Should there be a NULL
check immediately after the ioremap() call?

[Severity: Medium]
In drivers/ata/pata_cswarp.c:pata_cswarp_probe(), there is a potential
resource leak on the error path for ata_host_activate().

If the call to ata_host_activate() fails and returns an error code, the
function exits directly by returning that error. Because the base pointer was 
mapped manually with ioremap() rather than a managed API like devm_ioremap(), 
does this leave the I/O mapping permanently leaked? There appears to be no 
explicit iounmap(base) in this specific error path to clean up the mapping.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824153339.512487-1-p.pisati@gmail.com?part=1

  reply	other threads:[~2026-08-24 15:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-24 15:33 [PATCH v5 0/2] m68k: pata_cswarp: Add Amiga cslab ata support Paolo Pisati
2026-08-24 15:33 ` [PATCH v5 1/2] ata: " Paolo Pisati
2026-08-24 15:41   ` sashiko-bot [this message]
2026-08-25  7:53   ` Geert Uytterhoeven
2026-08-25  9:55     ` Geert Uytterhoeven
2026-08-26  8:55     ` Paolo Pisati
2026-08-26 19:31     ` Michael Schmitz
2026-08-27  7:29       ` Geert Uytterhoeven
2026-08-24 15:33 ` [PATCH v5 2/2] m68k: defconfig: enable PATA_CSWARP Paolo Pisati
2026-08-24 15:41   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260824154151.9F41B1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=cassel@kernel.org \
    --cc=dlemoal@kernel.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=p.pisati@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox