ACK/Cmnt: [GIT PULL][SRU][B] CVE-2020-8832: i915 incomplete fix for CVE-2019-14615

[Tyler Hicks]

On 2020-02-13 17:17:21, Kleber Souza wrote:
> On 13.02.20 02:04, Tyler Hicks wrote:
> > BugLink: https://bugs.launchpad.net/bugs/1862840
> > CVE-2020-8832
> > 
> > [Impact]
> > 
> > Gregory Herrero reported that the proof-of-concept for CVE-2019-14615
> > indicates that the information leak is not fixed in the Bionic 4.15
> > kernel as indicated by USN-4255-1:
> > 
> >  https://usn.ubuntu.com/4255-1/
> > 
> > This only affects Ubuntu's 4.15 kernel series. Xenial (4.4), Disco
> > (5.0), Eoan (5.3), and Focal (5.4) are not affected by this incomplete
> > fix issue.
> > 
> > I've verified this by testing each Ubuntu release with the
> > proof-of-concept. I then tested vanilla 4.15 with commit bc8a76a152c5
> > ("drm/i915/gen9: Clear residual context state on context switch")
> > applied, which is the fix for CVE-2019-14615, and verified that the
> > proof-of-concept showed that the info leak was still possible. I then
> > tested vanilla 4.16 with commit bc8a76a152c5 applied to verify that the
> > proof-of-concept showed that the info leak was fixed.
> > 
> > After bisecting changes to the DRM subsystem as well as the i915 driver,
> > it looks like commit d2b4b97933f5 ("drm/i915: Record the default hw
> > state after reset upon load") as well as its prerequisites are necessary
> > to fully fix CVE-2019-14615 in 4.15 based kernels.
> > 
> > [Test Case]
> > 
> > A proof-of-concept for CVE-2019-14615 became available once the issue
> > was made public. It can be found here:
> > 
> >  https://github.com/HE-Wenjian/iGPU-Leak
> > 
> > Steps to use the proof-of-concept:
> > 
> >  $ git clone https://github.com/HE-Wenjian/iGPU-Leak.git
> > 
> >  # In one terminal
> >  $ cd iGPU-Leak/demo/SLM_Leak/
> >  $ ./run_victim.sh
> > 
> >  # In another terminal
> >  $ cd iGPU-Leak/demo/SLM_Leak/
> >  $ ./run_attacker.sh
> > 
> >  # In the terminal running run_attacker.sh, ensure that all data dumped
> >  # to the terminal is zeros and that there is no non-zero data. You'll
> >  # have to closely monitor the script for a minute or so to ensure that
> >  # the information leak is not possible.
> > 
> > [Regression Potential]
> > 
> > High as the changes are complex in comparison to the typical SRU.
> > However, the bulk of the change is to the initialization stages of the
> > driver and we're just pulling back changes that landed in 4.16-rc1 to
> > our 4.15 kernel. I don't see any later Fixes tags that reference the
> > needed commits.
> > 
> > [Pull Request]
> > 
> > The following changes since commit a8c1e6affa4262808b062e3422686a85efa541b5:
> > 
> >   UBUNTU: Ubuntu-4.15.0-76.86 (2020-01-17 10:59:22 -0300)
> > 
> > are available in the Git repository at:
> > 
> >   https://git.launchpad.net/~tyhicks/ubuntu/+source/linux/+git/bionic cves/CVE-2020-8832
> > 
> > for you to fetch changes up to 25df7fc598b4b0fe0713915686596cf8bc8087bf:
> > 
> >   drm/i915: Record the default hw state after reset upon load (2020-02-11 23:17:42 +0000)
> > 
> > ----------------------------------------------------------------
> > Chris Wilson (7):
> >       drm/i915: Use same test for eviction and submitting kernel context
> >       drm/i915: Force the switch to the i915->kernel_context
> >       drm/i915: Move GT powersaving init to i915_gem_init()
> >       drm/i915: Move intel_init_clock_gating() to i915_gem_init()
> >       drm/i915: Inline intel_modeset_gem_init()
> >       drm/i915: Mark the context state as dirty/written
> >       drm/i915: Record the default hw state after reset upon load
> > 
> > Tvrtko Ursulin (1):
> >       drm/i915: Define an engine class enum for the uABI
> > 
> >  drivers/gpu/drm/i915/gvt/scheduler.c    |   2 -
> >  drivers/gpu/drm/i915/i915_debugfs.c     |   1 -
> >  drivers/gpu/drm/i915/i915_drv.c         |   5 +-
> >  drivers/gpu/drm/i915/i915_drv.h         |   1 -
> >  drivers/gpu/drm/i915/i915_gem.c         | 144 ++++++++++++++++++++++++++++++--
> >  drivers/gpu/drm/i915/i915_gem_context.c |  62 ++++----------
> >  drivers/gpu/drm/i915/i915_gem_context.h |   4 +-
> >  drivers/gpu/drm/i915/i915_gem_evict.c   |   3 +-
> >  drivers/gpu/drm/i915/intel_display.c    |  11 ---
> >  drivers/gpu/drm/i915/intel_engine_cs.c  |  55 +++++++++++-
> >  drivers/gpu/drm/i915/intel_lrc.c        |  70 +++++++++++-----
> >  drivers/gpu/drm/i915/intel_pm.c         |   2 -
> >  drivers/gpu/drm/i915/intel_ringbuffer.c |  51 +++++++----
> >  drivers/gpu/drm/i915/intel_ringbuffer.h |   9 +-
> >  include/uapi/drm/i915_drm.h             |  31 +++++++
> >  15 files changed, 339 insertions(+), 112 deletions(-)
> > 
> 
> These changes look good to me. We have good test results with
> the reproducer and most of the patches are clean cherry-picks
> and the backport was trivial.
> 
> I'm adding Timo to CC, it would be good to have his words about
> these changes.

I'll mention that I did reach out to Chris Wilson at Intel, with Timo on
CC, about these changes. To summarize his response, he said that it does
seem plausible that these patches are needed for 4.15 kernels and that
there was not an easier solution available as a more minimal backport
for 4.15.

> Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>

Thanks!

Tyler