null_audio: "page fault, but interrupts were disabled" after ~5 minutes of system idle.

[siarzhuk]

Test System: Haiku hrev36939 with null_audio driver included in image and with removed hda driver. The Media server must be configured to use Virtual Audio for inputs and outputs.

The Problem: Wait after the system boot about 5-10 minutes. The system falls to KDL in null_audio/media_addons. The screenshot is attached.

Note: The same behaviour is observed with the draft version of my sis7018 audio driver: ​http://dev.haiku-os.org/browser/haiku/branches/developer/siarzhuk/sis7018. But the backtrace points to ExchangeBuffers call at line 203 of Stream.cpp. I can attach this screenshot too, if you need it.

I suspect that there is something wrong with implementation of null_audio and sis7018 because HDA driver on the same system do not force me to such problems.

[siarzhuk]

Screen shot of KDL backtrace after ~5 minutes of idel with null_audio driver.

[bonefish]

The problem is that the driver accesses the multi_buffer_info* passed in from userland not only unchecked, but even with interrupts disabled. In this case the page in question (belonging to the thread's stack) has not been used for a while and has been unmapped by the page daemon. The driver tries to access it in its buffer_exchange() function, thus causing a page fault. Since interrupts are disabled at that point, the kernel has to panic.

If you trace the call for the hda driver, you'll see that it copies the buffer passed from userland onto the kernel stack via user_memcpy() (before disabling interrupts). That is safer behavior, though generally the hda driver is not at all good following the userland-safe protocol, so it's not really a good reference.

Rule number one for kernel/driver code that interfaces with the userland is: Never trust anything you get from userland. Anything passed in from userland must be checked before using it. For a driver's ioctl() hook that means:

1. Check whether called from userland. There's no public API for that yet. You have to include the kernel private <thread.h> and check (thread_get_current_thread()->flags & THREAD_FLAGS_SYSCALL) != 0. If that evaluates to false, the call came from within the kernel and the parameters can be considered safe. If true the following needs to be done.
2. Check whether the pointer to the buffer is a null pointer. If it is non-null, check IS_USER_ADDRESS(<pointer>) (include <kernel.h>). If false, the pointer does not point to userland memory and B_BAD_ADDRESS should be returned immediately.
3. The memory the pointer points to can be accessed via two different mechanism:
   • user_memcpy(): It works like memcpy(), but has a status_t return code. Either, both, or neither of the pointers passed to it can be userland pointers. If copying went fine, B_OK is returned. If invalid memory is accessed while copying, the function returns an error code. In the latter case return B_BAD_ADDRESS back to userland. user_memcpy() can only be used with interrupts enabled!
   • lock_memory()/unlock_memory(): lock_memory() is called with a range of memory, and it makes sure all memory can be accessed without causing a page fault. I.e. after the call the memory behaves like locked kernel memory. It is possible to access the memory freely even with interrupts disabled. Note that if that is userland memory, you can freely access it, but you cannot rely on the contents of the memory -- i.e. assume that a userland thread can change the data therein at any time. After done playing with the memory the original lock_memory() call must be balance with an unlock_memory() call.

user_memcpy() is the cheaper mechanism and should generally be preferred. If data do not only have to be read, but also passed back to userland, two user_memcpy() calls are needed. One at the beginning to copy the userland input data to a kernel buffer (on the stack or, if larger, on the heap) and one at the end to copy the output data back. Note that the second user_memcpy() can fail even if the first one succeeded.

[siarzhuk]

Thank you for the explanation. Well, I'll take care on this problem in null_audio. Just to take some practise. ;-)

[siarzhuk]

Fixed in hrev36975.