[Bug runtime/22155] kernel panic due to NULL vma_cache_p->f_path.dentry

["dsmith at redhat dot com" <sourceware-bugzilla at sourceware dot org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=22155

David Smith <dsmith at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dsmith at redhat dot com

--- Comment #1 from David Smith <dsmith at redhat dot com> ---
(In reply to Tetsuo Handa from comment #0)
> I encountered an oops with systemtap-3.1-3.el7.x86_64. The location was
> 
>   if (path->dentry->d_op && path->dentry->d_op->d_dname &&
> 
> in d_path(), which means that path->dentry == NULL for some reason.
> 
> Looking at __stp_call_mmap_callbacks_for_task() (runtime/linux/task_finder.c
> or runtime/linux/task_finder2.c , which one is in use?),

Knowing whether you are using task_finder.c or task_finder2.c is based on your
kernel. Since you are using 3.10.0-514.26.2.el7.x86_64 which doesn't have
CONFIG_UTRACE, you'll be using task_finder2.c.

> it counts number of
> vma->vm_file != NULL entries in tsk->mm->mmap and allocates memory for such
> entries and remembers the address of vma->vm_file->f_path like
> 
>   vma_cache_p->f_path = &(vma->vm_file->f_path);
> 
> with tsk->mm->mmap_sem held for read, and the address of f_path
> (instead of f_path.dentry and f_path.mnt) is later passed to d_path()
> after tsk->mm->mmap_sem is released.
> 
> Is tsk->mm->mmap_sem held for read sufficient for guaranteeing that
> 
>   // First find the number of file-based vmas.
>   vma = mm->mmap;
>   while (vma) {
>     if (vma->vm_file)
>       file_based_vmas++;
>       vma = vma->vm_next;
>   }
> 
> and
> 
>   vma = mm->mmap;
>   vma_cache_p = vma_cache;
>   while (vma) {
>     if (vma->vm_file) {
>       // Notice we're increasing the reference
>       // count for 'f_path'.  This way it won't
>       // get deleted from out under us.
>       vma_cache_p->f_path = &(vma->vm_file->f_path);
>       path_get(vma_cache_p->f_path);
>       vma_cache_p->dentry = vma->vm_file->f_path.dentry;
>       vma_cache_p->addr = vma->vm_start;
>       vma_cache_p->length = vma->vm_end - vma->vm_start;
>       vma_cache_p->offset = (vma->vm_pgoff << PAGE_SHIFT);
>       vma_cache_p->vm_flags = vma->vm_flags;
>       vma_cache_p++;
>     }
>     vma = vma->vm_next;
>   }
> 
> fills all entries in vma_cache_p ?

We used to call get_task_mm() and mmput() in that function, but since mmput()
can sleep and that function can get called in atomic context, we had to remove
those calls.

But, when that function gets called, the task is stopped. So, hopefully the
task->mm won't be changing while the task is stopped.

> Assuming that above is correct, although we took reference on
> f_path.dentry and f_path.mnt via path_get(), what guarantees that
> vma->vm_file->f_path is still valid (i.e. f_path.dentry and f_path.mnt
> do not change) between tsk->mm->mmap_sem is released and d_path() is called?
> Isn't there a race window that the memory region pointed by vma->vm_file
> changes (i.e. vma->vm_file->f_path == { garbage, garbage }) ?

If path_get() isn't sufficient to keep the f_path.dentry and f_path.mnt valid,
the there is some generic bug in the kernel. That's kind of the point for a
reference-counted structure.

> I feel that "struct vma_cache_t" needs to use "struct path" rather than
> "struct path *".

Hmm, that's an interesting suggestion.

... console output deleted ...

Can you show us the script that caused this panic?

Is this panic intermittent or does it happen every time?

-- 
You are receiving this mail because:
You are the assignee for the bug.