Zeke-OS / zeke

A POSIX-like OS for ARM processors.
Other
88 stars 9 forks source link

copyout() race condition with userspace free() #139

Closed OlliV closed 4 years ago

OlliV commented 7 years ago

The return code of copyout() is rarely checked and it could make syscalls return 0 even though the copy operation failed. That could happen if another thread of the process frees the buffer.

OlliV commented 4 years ago

It should be probably documented that syscalls that only read but don't change any state will fail with EFAULT, while syscalls that mutate the system state will silently ignore such errors. The rationale is that the error condition is always or in most of the cases a user error.

However, what should be done in the code side is that there shouldn't be checks like

    if (!useracc((__user void *)args->buf, sizeof(struct statvfs),
                 VM_PROT_WRITE)) {
        set_errno(EFAULT);
        goto out;
    }

because it's always redundant (and waste of CPU time because it's rather heavy operation) as copyout() will do the same anyway and there is a race condition with the other user space threads of the same process.

OlliV commented 4 years ago

22cca61169fb7577cb4d893219045044fb4fc84e addresses most of these issues.