4 ms·

It's worth noting that in the kernel there's more to "user data" than just the fact that its content might be crafted maliciously: - it might get swapped out o

It's worth noting that in the kernel there's more to "user data" than just the fact that its content might be crafted maliciously: - it might get swapped out or migrated, meaning your thread goes to sleep if you touch it. - it can always change concurrently. - the pointer is only valid within the current context (coz it's a pointer into the user address space). - I've actually never thought about this before but also the user could free it concurrently I think? So there's actually already C APIs in the kernel that force you to be aware of this, at least for strings (stuff where the user passes a pointer to their memory). There's also fancy compiler stuff for marking struct fields as pointing to user data, which IIUC can be used to detect if you're forgetting to use the right conversion APIs. This isn't the case for values passed in registers though so I think Trusted<u64> would be totally new. ANYWAY, overall: this is cool and good but it's actually one of the lower-impact things from Rust in the kernel IMO. I don't think the bugs that this prevents are actually all that common in practice. TOCTOU would be the biggest one by far but they are rare compared to the daily deluge of incredibly basic UAF bugs that Rust also prevents.

tux3 1y ago
>I've actually never thought about this before but also the user could free it concurrently I think? Yup. But if you only send it back to the malloc heap, the kernel doesn't notice, it will happily read data that has been freed. Unless the data is unmapped, then the kernel is going to get a page fault while trying to read your input. What goes up must come down, so that gets translated to SIGSEGV, and the program stops.
- cyphar 1y ago
  The kernel doesn't give you SIGSEGV in that case -- you will usually just get -EFAULT. All user pointer accesses are scoped with user_access_begin() and user_access_end() (or something equivalent) which stops the block of data from being unmapped by another thread -- of course, if this mechanism didn't exist then you would get a kernel oops if there was a concurrent munmap(2).
  - cyphar 1y ago
    I looked at it again and the story is more complicated -- user_access_begin() is actually just a memory barrier on x86 and so doesn't take mmap_sem like I thought. It looks like each get_user() independently checks that the pointer is valid and access_ok() is actually just a fast path. You will still get -EFAULT in the unmapped case though.
- bjackman 1y ago
  But the question mark was coz I was wondering if there's some process global lock that would prevent it. I think I was thinking of the mmap lock. But this was a dumb question mark, this lock isn't just randomly taken when threads enter syscalls. So yeah this is indeed another reason why copy_from_user etc is needed.

tux3 1y ago

>I've actually never thought about this before but also the user could free it concurrently I think? Yup. But if you only send it back to the malloc heap, the kernel doesn't notice, it will happily read data that has been freed. Unless the data is unmapped, then the kernel is going to get a page fault while trying to read your input. What goes up must come down, so that gets translated to SIGSEGV, and the program stops.

cyphar 1y ago
The kernel doesn't give you SIGSEGV in that case -- you will usually just get -EFAULT. All user pointer accesses are scoped with user_access_begin() and user_access_end() (or something equivalent) which stops the block of data from being unmapped by another thread -- of course, if this mechanism didn't exist then you would get a kernel oops if there was a concurrent munmap(2).
- cyphar 1y ago
  I looked at it again and the story is more complicated -- user_access_begin() is actually just a memory barrier on x86 and so doesn't take mmap_sem like I thought. It looks like each get_user() independently checks that the pointer is valid and access_ok() is actually just a fast path. You will still get -EFAULT in the unmapped case though.
bjackman 1y ago
But the question mark was coz I was wondering if there's some process global lock that would prevent it. I think I was thinking of the mmap lock. But this was a dumb question mark, this lock isn't just randomly taken when threads enter syscalls. So yeah this is indeed another reason why copy_from_user etc is needed.