On Wed, Jun 30, 2021 at 6:54 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > When we do the kernel monitor, use the DRGN > (https://github.com/osandov/drgn) access to kernel data structures, found > that the system calls a lot. DRGN is implemented by reading /proc/kcore. > After looking at the kcore code, it is found that kcore does not implement > mmap, resulting in frequent context switching triggered by read. > Therefore, we want to add mmap interface to optimize performance. Ok, this is funky, but I'm going to drop this patch because I think it's buggy as is. Since > +static int mmap_kcore(struct file *file, struct vm_area_struct *vma) > +{ > + size_t size = vma->vm_end - vma->vm_start; Ok. But then: > + start = kc_offset_to_vaddr(((u64)vma->vm_pgoff << PAGE_SHIFT) - > + ((data_offset >> PAGE_SHIFT) << PAGE_SHIFT)); Not only is that ((data_offset >> PAGE_SHIFT) << PAGE_SHIFT) a very strange calculation (did you mean "data_offset & PAGE_MASK"?), but I don't see anything that protects against underflow in that calculation. pg_off can easily be arbitrarily small (eg zero), so that subtraction can underflow afaik. So that needs a test, and return -EINVAL or whatever. But even if that is fixed, this test is entirely broken: > + list_for_each_entry(m, &kclist_head, list) { > + if (start >= m->addr && size <= m->size) > + break; > + } No, that's wrong. You allow 'size' to be as big as 'm->size', but you do that even if 'start' isn't 'm->start'. The proper check would be something like u64 end = start + size; if (start >= m->addr && end <= m->addr+m->size) .. or similar (and that should check that "start+size" hasn't overflowed). So I see what appears to be multiple problems, and while I hand-waved some fixes for them, those are very much "maybe something like this", and I'm going to drop this patch. Not for 5.14. Linus
