KVM-VMI / nitro

GNU General Public License v3.0
46 stars 11 forks source link

Backend dispatch_hook exception handling #46

Open Soft opened 6 years ago

Soft commented 6 years ago

Currently, the code in Backend dispatch_hook is kind of messy. I guess it made sense when there was only a single back end. To be more specific, I would like for us to decide what should be done about exceptions. Currently we simply catch all the exceptions and log them but I wonder if this is the right approach to take. Previously, the method also caught some back end specific errors but that was lost with the move to multiple back ends.

What are your thoughts on the matter?

Wenzel commented 6 years ago

The reason why i was catched all these different exceptions here was just to have some statistic at the beginning.

I wanted to know if a hook failed because of a KeyError exception (so the hook implementation was faulty), or if libvmi failed to read the memory, or even if the memory access was not consistent (dump test on ObjectAttributes fields).

This self.stats was just to give me some visibility after running Nitro.

My idea here would be to replace that code by a generic exception handler, to catch everything.

try:
    hook(syscall, self)
except Exception as e:
    logging.exception(e)

It's critically important to catch any exception that could be raised while a hook is being processed. Otherwise we are not ackowledging the kernel and the VM is frozen and has to be killed

Wenzel commented 6 years ago

Actually we are already catching everything.

I suggest we should remove these specific exception handlers, and just log the exception on the logging output.