[PATCH 0/3] ata: pata_ep93xx: COMPILE_TEST and ARM fixups

[PATCH 0/3] ata: pata_ep93xx: COMPILE_TEST and ARM fixups

[Rosen Penev]

This small series cleans up pata_ep93xx to allow COMPILE_TEST on
non-ARM platforms. Patch 1 avoids including ARM-specific asm headers,
patch 2 switches to unsigned long for the data variable, and patch 3
enables COMPILE_TEST.

Rosen Penev (3):
  ata: pata_ep93xx: avoid asm on non ARM
  ata: pata_ep93xx: use unsigned long for data
  ata: pata_ep93xx: add COMPILE_TEST support

 drivers/ata/Kconfig       | 2 +-
 drivers/ata/pata_ep93xx.c | 7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.54.0

[PATCH 1/3] ata: pata_ep93xx: avoid asm on non ARM

[Rosen Penev]

The raw ARM asm delay loop prevents COMPILE_TEST builds on
non-ARM architectures. Guard it with CONFIG_ARM and provide a
cpu_relax() fallback for compilation on other architectures.

Assisted-by: Opencode:Big-pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/ata/pata_ep93xx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
index b2b9e0058333..f7e87d6af264 100644
--- a/drivers/ata/pata_ep93xx.c
+++ b/drivers/ata/pata_ep93xx.c
@@ -204,6 +204,7 @@ static void ep93xx_pata_enable_pio(void __iomem *base, int pio_mode)
  */
 static void ep93xx_pata_delay(unsigned long count)
 {
+#ifdef CONFIG_ARM
 	__asm__ volatile (
 		"0:\n"
 		"mov r0, r0\n"
@@ -212,6 +213,10 @@ static void ep93xx_pata_delay(unsigned long count)
 		: "=r" (count)
 		: "0" (count)
 	);
+#else
+	while (count--)
+		cpu_relax();
+#endif
 }
 
 static unsigned long ep93xx_pata_wait_for_iordy(void __iomem *base,
-- 
2.54.0

Re: [PATCH 1/3] ata: pata_ep93xx: avoid asm on non ARM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing condition code clobber in inline assembly.
- [High] Double free and invalid pointer dereference on DMA probe error path.
--

commit 161745bc82337864b2d7b61cafb8fabbb474d029
Author: Rosen Penev <rosenp@gmail.com>

ata: pata_ep93xx: avoid asm on non ARM

The raw ARM asm delay loop prevents COMPILE_TEST builds on
non-ARM architectures. Guard it with CONFIG_ARM and provide a
cpu_relax() fallback for compilation on other architectures.

> diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> index b2b9e0058333e..f7e87d6af2641 100644
> --- a/drivers/ata/pata_ep93xx.c
> +++ b/drivers/ata/pata_ep93xx.c
> @@ -204,6 +204,7 @@ static void ep93xx_pata_enable_pio(void __iomem *base, int pio_mode)
>   */
>  static void ep93xx_pata_delay(unsigned long count)
>  {
> +#ifdef CONFIG_ARM
>  	__asm__ volatile (
>  		"0:\n"
>  		"mov r0, r0\n"
>  		"subs %0, %1, #1\n"
>  		"bge 0b\n"
>  		: "=r" (count)
>  		: "0" (count)
>  	);

[Severity: High]
This is a pre-existing issue, but since this inline assembly uses the
"subs" instruction which alters the ARM condition code flags, does it
need a "cc" clobber?

Without the "cc" clobber, the compiler might assume that condition flags
set prior to the inline assembly remain valid across it. Because
ep93xx_pata_delay() is a small static function, it's likely to be
inlined into callers like ep93xx_pata_wait_for_iordy(). Could this lead
to unpredictable control flow corruption if the compiler schedules
flag-dependent instructions across the assembly block?

> +#else
> +	while (count--)
> +		cpu_relax();
> +#endif
>  }

[Severity: High]
This is a pre-existing issue, but I noticed a potential double free and
invalid pointer dereference on the DMA probe error path in this driver.

If dma_request_chan(&pdev->dev, "tx") fails during ep93xx_pata_dma_init(),
it returns an ERR_PTR which is stored in drv_data->dma_tx_channel.

The error path jumps to fail_release_rx, which releases dma_rx_channel but
doesn't clear the pointer, and then falls through to ep93xx_pata_release_dma():

drivers/ata/pata_ep93xx.c:ep93xx_pata_dma_init() {
    ...
fail_release_rx:
	dma_release_channel(drv_data->dma_rx_channel);
fail_release_dma:
	ep93xx_pata_release_dma(drv_data);
    ...
}

Inside ep93xx_pata_release_dma(), the code attempts to free both channels:

drivers/ata/pata_ep93xx.c:ep93xx_pata_release_dma() {
    ...
	if (drv_data->dma_rx_channel) {
		dma_release_channel(drv_data->dma_rx_channel);
		drv_data->dma_rx_channel = NULL;
	}
	if (drv_data->dma_tx_channel) {
		dma_release_channel(drv_data->dma_tx_channel);
		drv_data->dma_tx_channel = NULL;
	}
    ...
}

Since drv_data->dma_rx_channel was never set to NULL, does this cause a
double free of dma_rx_channel?

Additionally, since drv_data->dma_tx_channel holds an ERR_PTR rather than
NULL, this check evaluates to true and dma_release_channel() is called with
the ERR_PTR. Could this cause a kernel panic when the DMA subsystem attempts
to dereference the error pointer?

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

[PATCH 2/3] ata: pata_ep93xx: use unsigned long for data

[Rosen Penev]

An int is being encoded as a void pointer but that breaks on 64-bit
systems as the type needs to match pointer size.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/ata/pata_ep93xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
index f7e87d6af264..1663dcd00a93 100644
--- a/drivers/ata/pata_ep93xx.c
+++ b/drivers/ata/pata_ep93xx.c
@@ -977,7 +977,7 @@ static int ep93xx_pata_probe(struct platform_device *pdev)
 
 		match = soc_device_match(ep93xx_soc_table);
 		if (match)
-			ap->udma_mask = (unsigned int) match->data;
+			ap->udma_mask = (unsigned long) match->data;
 		else
 			ap->udma_mask = ATA_UDMA2;
 	}
-- 
2.54.0

Re: [PATCH 2/3] ata: pata_ep93xx: use unsigned long for data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Double free and invalid pointer dereference on DMA TX channel request failure
--

commit 29937e8c8082c869105691e9f38d16e45bbd37fb
Author: Rosen Penev <rosenp@gmail.com>

ata: pata_ep93xx: use unsigned long for data

An int is being encoded as a void pointer but that breaks on 64-bit
systems as the type needs to match pointer size.

> diff --git a/drivers/ata/pata_ep93xx.c b/drivers/ata/pata_ep93xx.c
> index f7e87d6af2641..1663dcd00a932 100644
> --- a/drivers/ata/pata_ep93xx.c
> +++ b/drivers/ata/pata_ep93xx.c
> @@ -977,7 +977,7 @@ static int ep93xx_pata_probe(struct platform_device *pdev)
>  
>  		match = soc_device_match(ep93xx_soc_table);
>  		if (match)
> -			ap->udma_mask = (unsigned int) match->data;
> +			ap->udma_mask = (unsigned long) match->data;

[Severity: High]
This is a pre-existing issue, but while reviewing this file I noticed a
potential double free and invalid pointer dereference in the DMA error
handling paths.

If dma_request_chan() for the "tx" channel fails in ep93xx_pata_dma_init(),
it returns an ERR_PTR which is stored in drv_data->dma_tx_channel, and jumps
to fail_release_rx:

drivers/ata/pata_ep93xx.c:ep93xx_pata_dma_init() {
    ...
	drv_data->dma_tx_channel = dma_request_chan(&pdev->dev, "tx");
	if (IS_ERR(drv_data->dma_tx_channel)) {
		ret = dev_err_probe(dev, PTR_ERR(drv_data->dma_tx_channel),
				    "tx DMA setup failed\n");
		goto fail_release_rx;
	}
    ...
fail_release_rx:
	dma_release_channel(drv_data->dma_rx_channel);
fail_release_dma:
	ep93xx_pata_release_dma(drv_data);
}

The fail_release_rx path calls dma_release_channel() on the rx channel but
doesn't set drv_data->dma_rx_channel to NULL. It then falls through to
fail_release_dma which calls ep93xx_pata_release_dma():

drivers/ata/pata_ep93xx.c:ep93xx_pata_release_dma() {
	if (drv_data->dma_rx_channel) {
		dma_release_channel(drv_data->dma_rx_channel);
		drv_data->dma_rx_channel = NULL;
	}
	if (drv_data->dma_tx_channel) {
		dma_release_channel(drv_data->dma_tx_channel);
		drv_data->dma_tx_channel = NULL;
	}
}

Will this cause dma_release_channel() to be called a second time on the rx
channel?

Also, since drv_data->dma_tx_channel holds an ERR_PTR rather than NULL,
won't the second if block try to dereference that ERR_PTR when
dma_release_channel() is called on it?

>  		else
>  			ap->udma_mask = ATA_UDMA2;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530003627.42473-1-rosenp@gmail.com?part=2

[PATCH 3/3] ata: pata_ep93xx: add COMPILE_TEST support

[Rosen Penev]

Now that the build failures have been fixed, we can add COMPILE_TEST so
the buildbots can find potentially more problems.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/ata/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 462cbb01567c..6e25e9fcadb2 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -692,7 +692,7 @@ config PATA_EFAR
 
 config PATA_EP93XX
 	tristate "Cirrus Logic EP93xx PATA support"
-	depends on ARCH_EP93XX
+	depends on ARCH_EP93XX || COMPILE_TEST
 	select PATA_TIMINGS
 	help
 	  This option enables support for the PATA controller in
-- 
2.54.0

Re: [PATCH 3/3] ata: pata_ep93xx: add COMPILE_TEST support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Double free of 'rx' DMA channel and invalid pointer dereference of `ERR_PTR` in the 'tx' DMA channel error path.
--

commit b636a1b70934f2b41b7dce1e54946eb6d1bed3b4
Author: Rosen Penev <rosenp@gmail.com>

ata: pata_ep93xx: add COMPILE_TEST support

This commit enables COMPILE_TEST support for the Cirrus Logic EP93xx PATA
controller. This allows the buildbots to compile the driver on other
architectures to find potential issues.

> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index fff305ec1e788..28ca856ecc754 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -692,7 +692,7 @@ config PATA_EFAR
>  
>  config PATA_EP93XX
>  	tristate "Cirrus Logic EP93xx PATA support"
> -	depends on ARCH_EP93XX
> +	depends on ARCH_EP93XX || COMPILE_TEST

[Severity: High]
This isn't a bug introduced by this patch, but enabling COMPILE_TEST might
expose a pre-existing error handling issue in drivers/ata/pata_ep93xx.c.

In ep93xx_pata_dma_init(), if dma_request_chan() for "tx" fails, it stores
the ERR_PTR in drv_data->dma_tx_channel and jumps to fail_release_rx:

drivers/ata/pata_ep93xx.c:ep93xx_pata_dma_init() {
    ...
	drv_data->dma_tx_channel = dma_request_chan(&pdev->dev, "tx");
	if (IS_ERR(drv_data->dma_tx_channel)) {
		ret = dev_err_probe(dev, PTR_ERR(drv_data->dma_tx_channel),
				    "tx DMA setup failed\n");
		goto fail_release_rx;
	}
    ...
fail_release_rx:
	dma_release_channel(drv_data->dma_rx_channel);
fail_release_dma:
	ep93xx_pata_release_dma(drv_data);

	return ret;
}

Could this sequence lead to a double-free and an invalid pointer dereference?

The fail_release_rx path releases dma_rx_channel but doesn't clear the
pointer to NULL before falling through to ep93xx_pata_release_dma(). Also,
dma_tx_channel holds an ERR_PTR which is non-NULL.

When ep93xx_pata_release_dma() is called, it checks if the pointers are
non-NULL:

drivers/ata/pata_ep93xx.c:ep93xx_pata_release_dma() {
    ...
	if (drv_data->dma_rx_channel) {
		dma_release_channel(drv_data->dma_rx_channel);
		drv_data->dma_rx_channel = NULL;
	}
	if (drv_data->dma_tx_channel) {
		dma_release_channel(drv_data->dma_tx_channel);
		drv_data->dma_tx_channel = NULL;
	}
    ...
}

Would this result in dma_release_channel() being called a second time on
the rx channel, and being called with the ERR_PTR for the tx channel?

>  	select PATA_TIMINGS
>  	help
>  	  This option enables support for the PATA controller in

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530003627.42473-1-rosenp@gmail.com?part=3

Re: [PATCH 0/3] ata: pata_ep93xx: COMPILE_TEST and ARM fixups

[Niklas Cassel]

Hello Rosen,

On Fri, May 29, 2026 at 05:36:24PM -0700, Rosen Penev wrote:
> This small series cleans up pata_ep93xx to allow COMPILE_TEST on
> non-ARM platforms. Patch 1 avoids including ARM-specific asm headers,
> patch 2 switches to unsigned long for the data variable, and patch 3
> enables COMPILE_TEST.
> 
> Rosen Penev (3):
>   ata: pata_ep93xx: avoid asm on non ARM
>   ata: pata_ep93xx: use unsigned long for data
>   ata: pata_ep93xx: add COMPILE_TEST support
> 
>  drivers/ata/Kconfig       | 2 +-
>  drivers/ata/pata_ep93xx.c | 7 ++++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)

Your series looks good to me.

I do not think that you need to address the pre-existing issues reported
by Sashiko.

Intend to wait 1-2 days before picking this up, to hopefully get R-b tag
or two.


Kind regards,
Niklas

Re: [PATCH 0/3] ata: pata_ep93xx: COMPILE_TEST and ARM fixups

[Rosen Penev]

On Mon, Jun 1, 2026 at 1:54 AM Niklas Cassel <cassel@kernel.org> wrote:
>
> Hello Rosen,
>
> On Fri, May 29, 2026 at 05:36:24PM -0700, Rosen Penev wrote:
> > This small series cleans up pata_ep93xx to allow COMPILE_TEST on
> > non-ARM platforms. Patch 1 avoids including ARM-specific asm headers,
> > patch 2 switches to unsigned long for the data variable, and patch 3
> > enables COMPILE_TEST.
> >
> > Rosen Penev (3):
> >   ata: pata_ep93xx: avoid asm on non ARM
> >   ata: pata_ep93xx: use unsigned long for data
> >   ata: pata_ep93xx: add COMPILE_TEST support
> >
> >  drivers/ata/Kconfig       | 2 +-
> >  drivers/ata/pata_ep93xx.c | 7 ++++++-
> >  2 files changed, 7 insertions(+), 2 deletions(-)
>
> Your series looks good to me.
>
> I do not think that you need to address the pre-existing issues reported
> by Sashiko.
In retrospect, it might make sense to do so such that those fixes get
backported to stable kernels.
>
> Intend to wait 1-2 days before picking this up, to hopefully get R-b tag
> or two.
>
>
> Kind regards,
> Niklas

Re: [PATCH 0/3] ata: pata_ep93xx: COMPILE_TEST and ARM fixups

[Niklas Cassel]

On Mon, Jun 01, 2026 at 11:12:33AM -0700, Rosen Penev wrote:
> On Mon, Jun 1, 2026 at 1:54 AM Niklas Cassel <cassel@kernel.org> wrote:
> >
> > Hello Rosen,
> >
> > On Fri, May 29, 2026 at 05:36:24PM -0700, Rosen Penev wrote:
> > > This small series cleans up pata_ep93xx to allow COMPILE_TEST on
> > > non-ARM platforms. Patch 1 avoids including ARM-specific asm headers,
> > > patch 2 switches to unsigned long for the data variable, and patch 3
> > > enables COMPILE_TEST.
> > >
> > > Rosen Penev (3):
> > >   ata: pata_ep93xx: avoid asm on non ARM
> > >   ata: pata_ep93xx: use unsigned long for data
> > >   ata: pata_ep93xx: add COMPILE_TEST support
> > >
> > >  drivers/ata/Kconfig       | 2 +-
> > >  drivers/ata/pata_ep93xx.c | 7 ++++++-
> > >  2 files changed, 7 insertions(+), 2 deletions(-)
> >
> > Your series looks good to me.
> >
> > I do not think that you need to address the pre-existing issues reported
> > by Sashiko.
> In retrospect, it might make sense to do so such that those fixes get
> backported to stable kernels.

Oh, don't get me wrong.

If you want to fix up the pre-existing issues, we are happy to accept
patches :)

I just meant that it is not a prerequisite to get this series merged.


Kind regards,
Niklas

Re: [PATCH 0/3] ata: pata_ep93xx: COMPILE_TEST and ARM fixups

[Rosen Penev]

On Mon, Jun 1, 2026 at 11:34 AM Niklas Cassel <cassel@kernel.org> wrote:
>
> On Mon, Jun 01, 2026 at 11:12:33AM -0700, Rosen Penev wrote:
> > On Mon, Jun 1, 2026 at 1:54 AM Niklas Cassel <cassel@kernel.org> wrote:
> > >
> > > Hello Rosen,
> > >
> > > On Fri, May 29, 2026 at 05:36:24PM -0700, Rosen Penev wrote:
> > > > This small series cleans up pata_ep93xx to allow COMPILE_TEST on
> > > > non-ARM platforms. Patch 1 avoids including ARM-specific asm headers,
> > > > patch 2 switches to unsigned long for the data variable, and patch 3
> > > > enables COMPILE_TEST.
> > > >
> > > > Rosen Penev (3):
> > > >   ata: pata_ep93xx: avoid asm on non ARM
> > > >   ata: pata_ep93xx: use unsigned long for data
> > > >   ata: pata_ep93xx: add COMPILE_TEST support
> > > >
> > > >  drivers/ata/Kconfig       | 2 +-
> > > >  drivers/ata/pata_ep93xx.c | 7 ++++++-
> > > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > >
> > > Your series looks good to me.
> > >
> > > I do not think that you need to address the pre-existing issues reported
> > > by Sashiko.
> > In retrospect, it might make sense to do so such that those fixes get
> > backported to stable kernels.
>
> Oh, don't get me wrong.
>
> If you want to fix up the pre-existing issues, we are happy to accept
> patches :)
Just sent. Hopefully no issues this time.
>
> I just meant that it is not a prerequisite to get this series merged.
One of those patches conflicts with the first here.
>
>
> Kind regards,
> Niklas

Re: [PATCH 0/3] ata: pata_ep93xx: COMPILE_TEST and ARM fixups

[Niklas Cassel]

On Fri, 29 May 2026 17:36:24 -0700, Rosen Penev wrote:
> This small series cleans up pata_ep93xx to allow COMPILE_TEST on
> non-ARM platforms. Patch 1 avoids including ARM-specific asm headers,
> patch 2 switches to unsigned long for the data variable, and patch 3
> enables COMPILE_TEST.
> 
> Rosen Penev (3):
>   ata: pata_ep93xx: avoid asm on non ARM
>   ata: pata_ep93xx: use unsigned long for data
>   ata: pata_ep93xx: add COMPILE_TEST support
> 
> [...]

Hello Rosen,

I applied this series:

Applied to libata/linux.git (for-7.2), thanks!

[1/3] ata: pata_ep93xx: avoid asm on non ARM
      https://git.kernel.org/libata/linux/c/cde1e86e
[2/3] ata: pata_ep93xx: use unsigned long for data
      https://git.kernel.org/libata/linux/c/8bbb394e
[3/3] ata: pata_ep93xx: add COMPILE_TEST support
      https://git.kernel.org/libata/linux/c/590da526



I can see that you have also sent out a bunch of stand-alone patches for
this driver. I'm not sure why you have an interest in this driver
specifically, but considering that a bunch of your fixes were sent out
with only a few minutes apart, I doubt that you can have tested them on
real hardware.

For these ancient drivers, we are mostly interested in real world problems
that you can reproduce on real hardware.

We don't really care that much for theoretical problems reported by AI.
If these problems were serious enough, most likely someone would have
encountered and fixed the problem by now.

One option would be to do like netdev and just remove some of these
ancient PATA drivers.


Kind regards,
Niklas