Bug 189905 - ide_release_iomio_dma may release same region twice, and leave another dangling
Summary: ide_release_iomio_dma may release same region twice, and leave another dangling
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat Enterprise Linux 4
Classification: Red Hat
Component: kernel
Version: 4.0
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
: ---
Assignee: Aristeu Rozanski
QA Contact: Brian Brock
URL:
Whiteboard:
Depends On:
Blocks: 176344
TreeView+ depends on / blocked
 
Reported: 2006-04-25 17:16 UTC by Kimball Murray
Modified: 2007-11-30 22:07 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-08-08 14:35:15 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
the original patch (495 bytes, patch)
2006-08-08 14:28 UTC, Aristeu Rozanski
no flags Details | Diff
patch used to trigger the bug (482 bytes, patch)
2006-08-08 14:29 UTC, Aristeu Rozanski
no flags Details | Diff

Description Kimball Murray 2006-04-25 17:16:17 UTC
Description of problem:
This was found via code inspection by Stratus.  It does not affect Stratus, but
I'm entering it here because it may hurt somebody else.  Here's the code for
ide_release_iomio_dma:

int ide_release_iomio_dma (ide_hwif_t *hwif)
{
        if ((hwif->dma_extra) && (hwif->channel == 0))
                release_region((hwif->dma_base + 16), hwif->dma_extra);
        release_region(hwif->dma_base, 8);
        if (hwif->dma_base2)
                release_region(hwif->dma_base, 8);
        return 1;
}

Notice the second and third calls to release_region pass in exactly the same
address.  I suspect the third call to release_region ought to be passing
hwif->dma_base2.

Version-Release number of selected component (if applicable):
RHEL4, and upstream

How reproducible:
Our platform does not expose this bug, because on our platform dma_base2 does
not exist.  I am merely entering this BUG on behalf of other ide users.

Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:
Code inspection reveals that this BUG is present upstream as well.  I hope
someone with the right IDE hardware can demonstrate this bug and fix it.  The
code change I suggest above seems like common sense, but I don't have the right
hardware to verify either the BUG or the solution.

Comment 1 Andrius Benokraitis 2006-06-20 18:07:13 UTC
This was posted as a courtesy by Stratus, they found this, but this bug isn't
being exhibited on their hardware, and can't fix/test this. Suggesting someone
else take ownership of this.

Comment 5 Aristeu Rozanski 2006-07-24 20:56:14 UTC
I submitted it upstream, but according to Sergei Shtylyov
(http://marc.theaimsgroup.com/?l=linux-ide&m=115377116801390&w=2) the usage of
dma_base2 will be removed soon, so this patch won't be accepted. The patch used
to remove dma_base2
(http://marc.theaimsgroup.com/?l=linux-ide&m=115377284117091&w=2) is a lot more
intrusive than the one to fix ide_release_iomio_dma(), so, I'll submit the last
one for inclusion on RHEL-4.


Comment 6 Aristeu Rozanski 2006-08-07 13:06:54 UTC
The only in-tree drivers that use dma_base2 are siimage and sgiioc4. Both
drivers don't have a cleanup function, so, even if them was compiled as modules,
they wouldn't be able to be removed, thus never touching the code path this
patch fixes.


Comment 7 Aristeu Rozanski 2006-08-07 13:30:06 UTC
It's still possible to trigger this if ide_alloc_dma_engine() fails. Submitting
the patch to list.


Comment 8 Aristeu Rozanski 2006-08-08 14:28:02 UTC
Created attachment 133788 [details]
the original patch

Comment 9 Aristeu Rozanski 2006-08-08 14:29:40 UTC
Created attachment 133789 [details]
patch used to trigger the bug

Comment 10 Aristeu Rozanski 2006-08-08 14:35:15 UTC
Closing this bug for these reasons:
- triggering the bug is very unlikely as the IDE modules are built-in and the
only affected ones (sgiioc4 and siimage) don't have cleanup functions and the
only way to trigger it would failing ide_alloc_dma_engine() at boot time.
- at least siimage driver can't handle a failure in ide_alloc_dma_engine(),
causing an oops while being tested with patch from comment #9.




Note You need to log in before you can comment on or make changes to this bug.