在 2021/7/1 上午11:32, Linus Torvalds 写道:
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.
Sorry, the calculations here are really confusing. The reason is that
when DRGN read /proc/kcore for ELF file header:
phdr->p_offset = kc_vaddr_to_offset(m->addr) + data_offset;
and DRGN call mmap, use phdr->p_offset passed in, I need to subtract
"data_offset".
So that needs a test, and return -EINVAL or whatever.
There's a problem with not judging "start". I will fix it in a v3.
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.
Yes, this is indeed wrong, I will fix it in a v3.
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
