Commit a8c15954 authored by Chris Wilson's avatar Chris Wilson

drm/i915: Protect debugfs per_file_stats with RCU lock

If we make sure we grab a strong reference to each object as we dump it,
we can reduce the locks outside of our iterators to an rcu_read_lock.

This should prevent errors like:
[ 2138.371911] BUG: KASAN: use-after-free in per_file_stats+0x43/0x380 [i915]
[ 2138.371924] Read of size 8 at addr ffff888223651000 by task cat/8293

[ 2138.371947] CPU: 0 PID: 8293 Comm: cat Not tainted 5.3.0-rc6-CI-Custom_4352+ #1
[ 2138.371953] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./J4205-ITX, BIOS P1.40 07/14/2017
[ 2138.371959] Call Trace:
[ 2138.371974]  dump_stack+0x7c/0xbb
[ 2138.372099]  ? per_file_stats+0x43/0x380 [i915]
[ 2138.372108]  print_address_description+0x73/0x3a0
[ 2138.372231]  ? per_file_stats+0x43/0x380 [i915]
[ 2138.372352]  ? per_file_stats+0x43/0x380 [i915]
[ 2138.372362]  __kasan_report+0x14e/0x192
[ 2138.372489]  ? per_file_stats+0x43/0x380 [i915]
[ 2138.372502]  kasan_report+0xe/0x20
[ 2138.372625]  per_file_stats+0x43/0x380 [i915]
[ 2138.372751]  ? i915_panel_show+0x110/0x110 [i915]
[ 2138.372761]  idr_for_each+0xa7/0x160
[ 2138.372773]  ? idr_get_next_ul+0x110/0x110
[ 2138.372782]  ? do_raw_spin_lock+0x10a/0x1d0
[ 2138.372923]  print_context_stats+0x264/0x510 [i915]
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Tested-by: default avatarDavid Weinehall <david.weinehall@linux.intel.com>
Reviewed-by: default avatarMika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190903062133.27360-1-chris@chris-wilson.co.uk
parent aaef8510
...@@ -237,6 +237,9 @@ static int per_file_stats(int id, void *ptr, void *data) ...@@ -237,6 +237,9 @@ static int per_file_stats(int id, void *ptr, void *data)
struct file_stats *stats = data; struct file_stats *stats = data;
struct i915_vma *vma; struct i915_vma *vma;
if (!kref_get_unless_zero(&obj->base.refcount))
return 0;
stats->count++; stats->count++;
stats->total += obj->base.size; stats->total += obj->base.size;
if (!atomic_read(&obj->bind_count)) if (!atomic_read(&obj->bind_count))
...@@ -284,6 +287,7 @@ static int per_file_stats(int id, void *ptr, void *data) ...@@ -284,6 +287,7 @@ static int per_file_stats(int id, void *ptr, void *data)
} }
spin_unlock(&obj->vma.lock); spin_unlock(&obj->vma.lock);
i915_gem_object_put(obj);
return 0; return 0;
} }
...@@ -313,10 +317,12 @@ static void print_context_stats(struct seq_file *m, ...@@ -313,10 +317,12 @@ static void print_context_stats(struct seq_file *m,
i915_gem_context_lock_engines(ctx), it) { i915_gem_context_lock_engines(ctx), it) {
intel_context_lock_pinned(ce); intel_context_lock_pinned(ce);
if (intel_context_is_pinned(ce)) { if (intel_context_is_pinned(ce)) {
rcu_read_lock();
if (ce->state) if (ce->state)
per_file_stats(0, per_file_stats(0,
ce->state->obj, &kstats); ce->state->obj, &kstats);
per_file_stats(0, ce->ring->vma->obj, &kstats); per_file_stats(0, ce->ring->vma->obj, &kstats);
rcu_read_unlock();
} }
intel_context_unlock_pinned(ce); intel_context_unlock_pinned(ce);
} }
...@@ -328,9 +334,9 @@ static void print_context_stats(struct seq_file *m, ...@@ -328,9 +334,9 @@ static void print_context_stats(struct seq_file *m,
struct task_struct *task; struct task_struct *task;
char name[80]; char name[80];
spin_lock(&file->table_lock); rcu_read_lock();
idr_for_each(&file->object_idr, per_file_stats, &stats); idr_for_each(&file->object_idr, per_file_stats, &stats);
spin_unlock(&file->table_lock); rcu_read_unlock();
rcu_read_lock(); rcu_read_lock();
task = pid_task(ctx->pid ?: file->pid, PIDTYPE_PID); task = pid_task(ctx->pid ?: file->pid, PIDTYPE_PID);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment