On Sat, Jul 25, 2015 at 06:56:20PM -0700, Hanno Böck wrote:
> Hi,
>
> I was trying to track down an out of bounds read issue in the intel drm
> driver that got reported by kasan.
>
> It happens in the function validate_cmds_sorted (i915_cmd_parser.c),
> where there are two nested loops, this is the relevant code part:
> for (i = 0; i < cmd_table_count; i++) {
> const struct drm_i915_cmd_table *table = &cmd_tables[i];
> u32 previous = 0;
> int j;
>
> for (j = 0; j < table->count; j++) {
> const struct drm_i915_cmd_descriptor *desc =
> &table->table[i];
>
>
> Now that &table->table[i] should probably really be &table->table[j],
> because that's the counter variable of the inner loop. Otherwise it
> doesn't make any sense (the inner loop would just repeat doing the same
> thing multiple times).
> However if I try to change [i] to [j] here my system doesn't boot any
> more, I just get a black screen. So I assume this bug is somehow hiding
> another more severe bug.
The tables aren't sorted, that is worth fixing.
This should get you booting with minimal fuss if you care to track down
the error.
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 430571b..688e814 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -672,6 +672,13 @@ static void fini_hash_table(struct intel_engine_cs *ring)
}
}
+#define DRM_ERROR_ON(cond, fmt, ...) ({ \
+ bool __cond = !!(cond); \
+ if (unlikely(__cond)) \
+ drm_err("assertion failed, %s: " fmt, #cond, ##__VA_ARGS__); \
+ unlikely(__cond); \
+})
+
/**
* i915_cmd_parser_init_ring() - set cmd parser related fields for a ringbuffer
* @ring: the ringbuffer to initialize
@@ -751,11 +758,16 @@ int i915_cmd_parser_init_ring(struct intel_engine_cs *ring)
default:
DRM_ERROR("CMD: cmd_parser_init with unknown ring: %d\n",
ring->id);
- BUG();
+ return -ENODEV;
}
- BUG_ON(!validate_cmds_sorted(ring, cmd_tables, cmd_table_count));
- BUG_ON(!validate_regs_sorted(ring));
+ if (DRM_ERROR_ON(!validate_cmds_sorted(ring, cmd_tables, cmd_table_count),
+ "command parser table is not sorted - required for bisetion searching\n"))
+ return -ENODEV;
+
+ if (DRM_ERROR_ON(!validate_regs_sorted(ring),
+ "register lists are not sorted - required for bisection searching\n"))
+ return -ENODEV;
WARN_ON(!hash_empty(ring->cmd_hash));
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
