Re: patch: fix ata panic with Thinkpad CD and DVD drives

Nate Lawson wrote:
> Søren Schmidt wrote:
> 
>> Nate Lawson wrote:
>>
>>> If you've been having "memory modified after free" panics on -current 
>>> and have a Thinkpad, the attached patch should fix things for you.  A 
>>> quick check of RELENG_5 indicates that the bug is probably there also 
>>> but I haven't tested for it there.
>>>
>>> The bug is triggered by timeouts in the ata_getparam() probe path.  
>>> The ata_timeout() fires and ata_end_transaction() is called to get 
>>> the status.  However, it continues down into ata_pio_read() even 
>>> though there is no data available since we had a timeout, not read 
>>> completion.    ata_pio_read() reads 512 bytes of probably bogus 
>>> data.  The important problem is that it also advances donecount.  On 
>>> subsequent timeouts (note there are 4 below), donecount advances into 
>>> unallocated memory and so subsequent ata_pio_read() calls overwrite 
>>> 512 bytes of someone else's memory.
>>>
>>> The fix is to exit immediately if ATA_R_TIMEOUT is set after reading 
>>> the status in ata_end_transaction().  It shouldn't go into 
>>> ata_pio_read() if there was a timeout.  The patch does this.
>>>
>>> However, it only handles PIO timeouts since I wasn't sure the best 
>>> way to proceed for unwinding DMA state and the like for the other 
>>> cases. This is enough to fix the overwrite and subsequent panic on my 
>>> systems.  I've run heavy IO stress and DVD accesses for a while and 
>>> no further panics.
>>>
>>> While looking into this, I found another potential problem.  In one 
>>> reinjection case, donecount wasn't reset to 0.  The patch for 
>>> ata-queue.c does this and I think it's necessary but don't hit this 
>>> case in testing so I can't be sure.  Finally, there's one whitespace 
>>> nit that helps with clarity.
>>>
>>> These are similar bugs to one found back in August that had the same 
>>> effect.  Here's the closest reference I could find in the mail 
>>> archives for this:
>>> http://lists.freebsd.org/mailman/htdig/freebsd-current/2004-August/033033.html 
>>
>>
>>
>>
>> Just a note from here, these bugs are fixed in ATA mkIII so you could 
>> just have gleaned the solution from there (or maybe you did :))
> 
> 
> Nope, but I'm glad you can corroborate these fixes are correct.

Actually I cant, I havn't looked at what was committed since I already 
did fix these problems in the mkIII patches floating around..
Anyhow its in there and the committer has to deal with it until/if I 
commit mkIII to -current, I'm out of the loop until then...

-- 

-Søren