On Fri, Jan 10, 2014 at 6:31 AM, Ted Unangst <t...@tedunangst.com> wrote: > On Fri, Jan 10, 2014 at 05:14, Miod Vallat wrote: >>> The only caller of kcopy is uiomove. There is no way a function like >>> this can ever work. If you need to rely on your copy function to save >>> you from pointers outside the address space, it means you don't know >>> what garbage you're passing it. Meaning you may well be passing it >>> pointers inside the address space, but to something unexpected, which >>> you will then shit on. >>> >>> Replace with memcpy. >> >> Vetoed. >> >> kcopy() is not only used to move data from the kernel data section to >> the kernel data section. >> >> It is used to move data *within the kernel address space* to data >> *within the kernel address space*. Think dd if=/dev/mem ... > > isn't that an example of kernel address space to userland? > > i did dig around a bit into uvm_io and callers, but didn't see > anything that depended on kcopy fault protection. there were some > comments indicating it is perhaps a holdover from swappable upage?
uvm_io maps userland map entries into kernel_map leaving them in the exact same state as in userland. Even if it wasn't possible to create valid userland map entires that always fault (it is, see below) they can still fault on errors. Here's a test that will crash the kernel without kcopy: https://github.com/art4711/stuff/blob/master/pttest/pttest.c Instead of ptrace we can trigger this with sysctl(KERN_PROC_ARGS) or dumping core. Instead of mmap of unallocated file space you can use revoke(2), mprotect, PT_WRITE to an mmap:ed hole in a file on a full filesystem, etc. Add to that a combinatorial explosion of other situations where errors propagate back to the fault and it is almost impossible to make sure that whatever goes through uvm_io will never fault (I guess you could try with vslock). This is not just limited to uvm_io. I bet this can be triggered through exec on tmpfs and the pageable mappings of its aobj too (out of memory) and anything else that can somehow end up being an uiomove to/from kernel_map, exec_map or some other pageable map. Don't do it. //art
