ApolloTeam-dev/ApolloOS

ATA driver Crash following "avail flush"

Closed this issue · 3 comments

Wawa of the AROS team spotted an enforcer hit in the ATA code following the execution of "avail flush". Kalametee spotted a potential flaw and wrote a sample patch for the problem. This needs to be tested and the fix varified for both Vampire and non-vampire Amigas.

ata.patch.txt

So if I am naughty and cut and paste from the chat log where Kalamatee explains the issue......

Kalamatee 14:17
Yes, that code looks somewhat broken there are a couple of issues there.
The ata.device creates the deamon task in its init code, which causes buses to perform a presence detect.
In the expunge code, the daemon is killed if a bus isn't in use.
If no buses have been registered, the daemon isn't killed.
problem 1 - if any bus is successfully removed, the daemon is killed. if another bus is in use after this, the daemon has already been killed and it will no longer have presence detect triggered.
problem 2 - the crash you witness, ata device doesn't have any registered buses (because no units where found), so it does not kill the daemon task resulting in it illegally accessing memory after the device is expunged.
Problem 2 can be easily fixed, but 1 is a little more tricky

Kalamatee 15:13
instead of a single daemon task, remove the code from ata init/expunge that deals with it and instead do it in ata_controllerclass.c's Root__New and Root__Disppose methods
so that there is a daemon per controller, and it is destroyed with the controller

The feature_ata_fix branch should not need to be pushed back to the AROS main project because this patch came from them. It will anyway be included into their master following testing.

This is patched in the up and coming main branch. Nothing needs to be done now.