Re: [PATCH 2/2] tracing: identify which executable object the userspace address belongs to


* Török Edwin <edwintorok@xxxxxxxxx> wrote:

Impact: modify+improve the userstacktrace tracing visualization feature

+static inline int seq_print_user_ip(struct trace_seq *s, struct mm_struct *mm,
+ unsigned long ip, unsigned long sym_flags)
+{
+ struct file *file = NULL;
+ unsigned long vmstart = 0;
+ int ret = 1;
+
+ if (mm) {
+ const struct vm_area_struct *vma = find_vma(mm, ip);
+ if (vma) {

Style: please put a newline after variable definition blocks.

+ file = vma->vm_file;
+ vmstart = vma->vm_start;

Bug: it's generally unsafe to look up a vma and use it without having
done a down_read(&mm->mmap_sem). Another thread (of this ->mm) could
go and modify it in parallel.

+ }
+ }
+ if (file) {
+ ret = trace_seq_path(s, &file->f_path);
+ if (ret)
+ ret = trace_seq_printf(s, "[+0x%lx]",
+ ip - vmstart);

Style: that linebreak is unnecessary.

+ if (trace_flags & TRACE_ITER_SYM_USEROBJ) {
+ struct task_struct *task;
+ /*
+ * we do the lookup on the thread group leader,
+ * since individual threads might have already quit!
+ */
+ rcu_read_lock();
+ task = find_task_by_vpid(entry->ent.tgid);
+ rcu_read_unlock();
+
+ if (task)
+ mm = get_task_mm(task);

Bug: it is unsafe to look up a task and then drop the RCU lock and use
it - the task could go away the moment the RCU read-lock is dropped.

A safer sequence would be to get get_task_mm(task) reference inside
the RCU critical section.

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages
Loading