| Message ID | 1458721906-10625-1-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 23, 2016 at 10:31:46AM +0200, Mika Kuoppala wrote: > Commit d15d7538c6d2 ("drm/i915: Tune down init error message due > to failure injection") added i915_load_error message to failure > path on device initialization. The message is printed > after the device is freed. And as the message printing helper > uses the device structure, this leads to use after free. > > Spotted by Kasan. Oops. I don't think it's practical to run the entire igt with kasan enabled (anything that stresses memory operations takes forever), even just one machine running BAT with kasan would be enough to catch silly errors like this one. How much heterogenity do we have in configs across CI? I guess not enough because as always we will be limited upon machines. > Cc: Imre Deak <imre.deak@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
Patchwork <patchwork@emeril.freedesktop.org> writes: > [ text/plain ] > == Series Details == > > Series: drm/i915: Fix use after free when printing load failure > URL : https://patchwork.freedesktop.org/series/4781/ > State : failure > > == Summary == > > Series 4781v1 drm/i915: Fix use after free when printing load failure > http://patchwork.freedesktop.org/api/1.0/series/4781/revisions/1/mbox/ > > Test kms_flip: > Subgroup basic-flip-vs-dpms: > pass -> DMESG-WARN (ilk-hp8440p) UNSTABLE > Subgroup basic-flip-vs-wf_vblank: > pass -> FAIL (bsw-nuc-2) https://bugs.freedesktop.org/show_bug.cgi?id=94294 -Mika > Test kms_pipe_crc_basic: > Subgroup nonblocking-crc-pipe-b-frame-sequence: > dmesg-warn -> PASS (ilk-hp8440p) > Test pm_rpm: > Subgroup basic-pci-d3-state: > dmesg-warn -> PASS (bsw-nuc-2) > > bdw-nuci7 total:192 pass:180 dwarn:0 dfail:0 fail:0 skip:12 > bdw-ultra total:192 pass:171 dwarn:0 dfail:0 fail:0 skip:21 > bsw-nuc-2 total:192 pass:153 dwarn:1 dfail:0 fail:1 skip:37 > byt-nuc total:192 pass:156 dwarn:1 dfail:0 fail:0 skip:35 > hsw-brixbox total:192 pass:170 dwarn:0 dfail:0 fail:0 skip:22 > hsw-gt2 total:192 pass:175 dwarn:0 dfail:0 fail:0 skip:17 > ilk-hp8440p total:192 pass:128 dwarn:1 dfail:0 fail:0 skip:63 > ivb-t430s total:192 pass:167 dwarn:0 dfail:0 fail:0 skip:25 > skl-i7k-2 total:192 pass:169 dwarn:0 dfail:0 fail:0 skip:23 > skl-nuci5 total:192 pass:181 dwarn:0 dfail:0 fail:0 skip:11 > snb-dellxps total:192 pass:158 dwarn:0 dfail:0 fail:0 skip:34 > snb-x220t total:192 pass:158 dwarn:0 dfail:0 fail:1 skip:33 > > Results at /archive/results/CI_IGT_test/Patchwork_1683/ > > 83ed25fa1b956275542da63eb98dc8fd2291329d drm-intel-nightly: 2016y-03m-22d-15h-20m-55s UTC integration manifest > b1cab272c8d7ea1f52cbe10d684308ef0ea2481a drm/i915: Fix use after free when printing load failure
Chris Wilson <chris@chris-wilson.co.uk> writes: > [ text/plain ] > On Wed, Mar 23, 2016 at 10:31:46AM +0200, Mika Kuoppala wrote: >> Commit d15d7538c6d2 ("drm/i915: Tune down init error message due >> to failure injection") added i915_load_error message to failure >> path on device initialization. The message is printed >> after the device is freed. And as the message printing helper >> uses the device structure, this leads to use after free. >> >> Spotted by Kasan. > > Oops. I don't think it's practical to run the entire igt with kasan > enabled (anything that stresses memory operations takes forever), even > just one machine running BAT with kasan would be enough to catch silly > errors like this one. How much heterogenity do we have in configs across > CI? I guess not enough because as always we will be limited upon > machines. > You don't want to compile kernel on Kasan enabled. But it is not so bad with igt/bat. time ./run-tests.sh -t basic [191/191] skip: 20, pass: 167, dmesg-warn: 3, dmesg-fail: 1 \ Thank you for running Piglit! Results have been written to /home/miku/src/intel-gpu-tools/results real 13m22.860s user 0m39.503s sys 5m48.932s I don't have the numbers for most recent CI/bat runs but according to Tomi, the slowest ones are in this ballpark. So wiring Kasan on of the fastest one, and we are good. >> Cc: Imre Deak <imre.deak@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Patch pushed to dinq, thanks for review. -Mika > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index a3458fcd83dc..fc8ac98c12d7 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1398,10 +1398,10 @@ out_runtime_pm_put: intel_runtime_pm_put(dev_priv); i915_driver_cleanup_early(dev_priv); out_free_priv: - kfree(dev_priv); - i915_load_error(dev_priv, "Device initialization failed (%d)\n", ret); + kfree(dev_priv); + return ret; }
Commit d15d7538c6d2 ("drm/i915: Tune down init error message due to failure injection") added i915_load_error message to failure path on device initialization. The message is printed after the device is freed. And as the message printing helper uses the device structure, this leads to use after free. Spotted by Kasan. Cc: Imre Deak <imre.deak@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_dma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)