| Message ID | 20230830110402.386898-1-joel@joelfernandes.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] mm/vmalloc: Add a safer version of find_vm_area() for debug | expand |
On Wed, Aug 30, 2023 at 11:03:59AM +0000, Joel Fernandes (Google) wrote: > It is unsafe to dump vmalloc area information when trying to do so from > some contexts. Add a safer trylock version of the same function to do a > best-effort VMA finding and use it from vmalloc_dump_obj(). > > Reported-by: Zhen Lei <thunder.leizhen@huaweicloud.com> > Cc: Paul E. McKenney <paulmck@kernel.org> > Cc: rcu@vger.kernel.org > Cc: Zqiang <qiang.zhang1211@gmail.com> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> I once started writing something similar, but got distracted and the immediate problem got solved a different way. It does make me wonder if we couldn't make this tree RCU-safe, but that's obviously a much larger job.
On Wed, Aug 30, 2023 at 8:08 AM Matthew Wilcox <willy@infradead.org> wrote: > > On Wed, Aug 30, 2023 at 11:03:59AM +0000, Joel Fernandes (Google) wrote: > > It is unsafe to dump vmalloc area information when trying to do so from > > some contexts. Add a safer trylock version of the same function to do a > > best-effort VMA finding and use it from vmalloc_dump_obj(). > > > > Reported-by: Zhen Lei <thunder.leizhen@huaweicloud.com> > > Cc: Paul E. McKenney <paulmck@kernel.org> > > Cc: rcu@vger.kernel.org > > Cc: Zqiang <qiang.zhang1211@gmail.com> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > > Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org> Thanks! > I once started writing something similar, but got distracted and the > immediate problem got solved a different way. > > It does make me wonder if we couldn't make this tree RCU-safe, but > that's obviously a much larger job. Yes, that would be nice. - Joel
On Wed, Aug 30, 2023 at 11:03:59AM +0000, Joel Fernandes (Google) wrote: > It is unsafe to dump vmalloc area information when trying to do so from > some contexts. Add a safer trylock version of the same function to do a > best-effort VMA finding and use it from vmalloc_dump_obj(). Seems like 2/2 to actually use this is missing?
Hi Christoph, On Wed, Aug 30, 2023 at 8:48 AM Christoph Hellwig <hch@infradead.org> wrote: > > On Wed, Aug 30, 2023 at 11:03:59AM +0000, Joel Fernandes (Google) wrote: > > It is unsafe to dump vmalloc area information when trying to do so from > > some contexts. Add a safer trylock version of the same function to do a > > best-effort VMA finding and use it from vmalloc_dump_obj(). > > Seems like 2/2 to actually use this is missing? 1/2 itself uses it. And then 2/2 is here which does additional improvement courtesy of Zqiang: https://lore.kernel.org/all/20230830110402.386898-2-joel@joelfernandes.org/ I used git send-email with cc-cmd set to get_maintainer.pl script. thanks, - Joel
diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 93cf99aba335..dae347e446e6 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -1865,6 +1865,18 @@ struct vmap_area *find_vmap_area(unsigned long addr) return va; } +static struct vmap_area *find_vmap_area_trylock(unsigned long addr) +{ + struct vmap_area *va; + + if (!spin_trylock(&vmap_area_lock)) + return NULL; + va = __find_vmap_area(addr, &vmap_area_root); + spin_unlock(&vmap_area_lock); + + return va; +} + static struct vmap_area *find_unlink_vmap_area(unsigned long addr) { struct vmap_area *va; @@ -2671,6 +2683,27 @@ struct vm_struct *find_vm_area(const void *addr) return va->vm; } +/** + * try_to_find_vm_area - find a continuous kernel virtual area + * @addr: base address + * + * This function is the same as find_vm_area() except that it is + * safe to call if vmap_area_lock is already held and returns NULL + * if it is. See comments in find_vmap_area() for other details. + * + * Return: the area descriptor on success or %NULL on failure. + */ +static struct vm_struct *try_to_find_vm_area(const void *addr) +{ + struct vmap_area *va; + + va = find_vmap_area_trylock((unsigned long)addr); + if (!va) + return NULL; + + return va->vm; +} + /** * remove_vm_area - find and remove a continuous kernel virtual area * @addr: base address @@ -4277,7 +4310,7 @@ bool vmalloc_dump_obj(void *object) struct vm_struct *vm; void *objp = (void *)PAGE_ALIGN((unsigned long)object); - vm = find_vm_area(objp); + vm = try_to_find_vm_area(objp); if (!vm) return false; pr_cont(" %u-page vmalloc region starting at %#lx allocated at %pS\n",
It is unsafe to dump vmalloc area information when trying to do so from some contexts. Add a safer trylock version of the same function to do a best-effort VMA finding and use it from vmalloc_dump_obj(). Reported-by: Zhen Lei <thunder.leizhen@huaweicloud.com> Cc: Paul E. McKenney <paulmck@kernel.org> Cc: rcu@vger.kernel.org Cc: Zqiang <qiang.zhang1211@gmail.com> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> --- mm/vmalloc.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)