Kernel Panic on Multiple user_xsi_semget Syscall Invocations

[thosewhowork]

Hi,

It is possible to cause a kernel panic by invoking the user_xsi_semget syscall with the IPC_CREAT (01000 Octal / 512 Decimal) key twice in a row.

The issue is here: ​https://github.com/haiku/haiku/blob/r1beta2/src/system/kernel/posix/xsi_semaphore.cpp#L774-L779

When the semaphoreSet == NULL condition is met, the block is entered, which then causes a dereference on the NULL pointer with semaphoreSet->ID().

The following should reproduce the issue:

#include <dlfcn.h>
#include <stdlib.h>

typedef int (*syscall_kern_xsi_semget)(int, int, int);

int
main(int argc, char** argv) {
    // IPC_CREAT is 01000 (Octal), 512 (Decimal)
    int i = 512;
    syscall_kern_xsi_semget p_kern_xsi_semget = NULL;

    if ((p_kern_xsi_semget = dlsym(RTLD_DEFAULT, "_kern_xsi_semget")) == NULL) {
        return 1;
    }
    
    // First call creates the ipcKey
    p_kern_xsi_semget(i, i, i);
    // Second call results in a NULL semaphoreSet, and subsequent NULL pointer deref.
    p_kern_xsi_semget(i, i, i);
    return 0;
}

Compiled with:

gcc -o xsi_semget_syscall src/X-xsi_semget_syscall.c

System Details

Haiku Version: Haiku R1/Beta2

Guest Machine: VMWare Fusion 8.5.3

Host Machine: Mac OS Mojave

I've attached the backtrace from the kernel debugger.

Thanks!

[pulkomandy]

Proposed fix in ​https://review.haiku-os.org/c/haiku/+/3623

Userland applications are normally not expected to call syscalls directly. May I ask what you're trying to do?

[thosewhowork]

Hi pulkomandy,

Absolutely - As another commenter inferred here: https://dev.haiku-os.org/ticket/16736#comment:5 I'm working on a kernel syscall fuzzer for educational purposes. I'm invoking the syscalls directly for simplicity's sake. I figured I would file the bugs that I find from using it.

If these kinds of reports are too noisy or unhelpful, please let me know and I can stop reporting what I turn up.

Take care!

[pulkomandy]

Oh, that's great :)

Bug reports are appreciated and we'll fix the problems. No application should be able to crash the kernel, whatever it tries, and experimenting with kernel fuzzing was on my TODO list but I never got into it. Please continue reporting your findings.

I was worried that this code would be used in a real application, in which case I would try to find something that makes more sense in that context. But as the output of a fuzzer, it is fine of course.

[waddlesplash]

It is worth noting that nightly builds have more asserts and other checks turned on, whereas the beta has a lot of checks turned down or off, so you may want to use a nightly build for your testing instead of the beta (we have also done a bunch of development since the beta, especially in the VM subsystem.)

[waddlesplash]

Fix merged in hrev54878, but we may want to leave this open to figure out why it even happened in the first place; as PulkoMandy pointed out, it is not clear how this can even occur.

[korli]

semaphoreSet would be NULL when invoking semaphoreSet->ID(). What is not clear exactly?

[korli]

I assume this is fixed in beta3

[pulkomandy]

semaphoreSet would be NULL when invoking semaphoreSet->ID(). What is not clear exactly?

It's not clear how we got in that situation, where the key appears to be valid (it is found in sIpcHashTable), but then it has no semaphoreSetId attached to it, or that semaphoreSetId is not found in sSemaphoreHashTable.

I would have expected that if the key is valid, the data it points to should be valid too. My understanding is that a valid key always goes with a semaphore, and it does not appear to be possible to delete the semaphore without deleting the key.

So, we are in a strange case that should not have been possible here: the key exists, but it points to no semaphore. How and when is it possible to get in that situation?

After looking more closely at the code, here is one thing that could happen:

• The key is created and inserted in the map here: ​https://git.haiku-os.org/haiku/tree/src/system/kernel/posix/xsi_semaphore.cpp#n763
• However, for various reasons, creating the semaphore can fail later on in the function: ​https://git.haiku-os.org/haiku/tree/src/system/kernel/posix/xsi_semaphore.cpp#n798

Probably the fuzzing test triggered this code path, as it calls the syscall with invalid arguments.

So, it is possible to insert invalid keys in the hashmap here. An additional fix would be to move sIpcHashTable.Insert(ipcKey); to the end of the if (create), where all error checks have been made already.

[pulkomandy]

Proposed fix: ​https://review.haiku-os.org/c/haiku/+/4240

[nielx]

Ticket retargeted after milestone closed

[pulkomandy]

Patch applied in hrev55265.