4.14.264-rc1 review

From: Tvrtko Ursulin <[email protected]>

commit 7938d61591d33394a21bdd7797a245b65428f44c upstream.

We need to flush TLBs before releasing backing store otherwise userspace is able to encounter stale entries if a) it is not declaring access to certain buffers and b) it races with the backing store release from a such undeclared execution already executing on the GPU in parallel.

The approach taken is to mark any buffer objects which were ever bound to the GPU and to trigger a serialized TLB flush when their backing store is released.

Alternatively the flushing could be done on VMA unbind, at which point we would be able to ascertain whether there is potential a parallel GPU execution (which could race), but essentially it boils down to paying the cost of TLB flushes potentially needlessly at VMA unbind time (when the backing store is not known to be going away so not needed for safety), versus potentially needlessly at backing store relase time (since we at that point cannot tell whether there is anything executing on the GPU which uses that object).

Thereforce simplicity of implementation has been chosen for now with scope to benchmark and refine later as required.

---
 drivers/gpu/drm/i915/i915_drv.h        |    2 
 drivers/gpu/drm/i915/i915_gem.c        |   84 ++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_gem_object.h |    1 
 drivers/gpu/drm/i915/i915_reg.h        |    6 ++
 drivers/gpu/drm/i915/i915_vma.c        |    4 +
 5 files changed, 96 insertions(+), 1 deletion(-)

@@ -2166,6 +2166,8 @@ struct drm_i915_private {
 
 	struct intel_uncore uncore;
 
+	struct mutex tlb_invalidate_lock;
+
 	struct i915_virtual_gpu vgpu;
 
 	struct intel_gvt *gvt;

@@ -135,6 +135,7 @@ struct drm_i915_gem_object {
 	 * activity?
 	 */
 #define I915_BO_ACTIVE_REF 0
+#define I915_BO_WAS_BOUND_BIT    1
 
 	/*
 	 * Is the object to be mapped as read-only to the GPU

@@ -2380,6 +2380,12 @@ enum i915_power_well_id {
 #define GAMT_CHKN_BIT_REG	_MMIO(0x4ab8)
 #define   GAMT_CHKN_DISABLE_DYNAMIC_CREDIT_SHARING	(1<<28)
 
+#define GEN8_RTCR	_MMIO(0x4260)
+#define GEN8_M1TCR	_MMIO(0x4264)
+#define GEN8_M2TCR	_MMIO(0x4268)
+#define GEN8_BTCR	_MMIO(0x426c)
+#define GEN8_VTCR	_MMIO(0x4270)
+
 #if 0
 #define PRB0_TAIL	_MMIO(0x2030)
 #define PRB0_HEAD	_MMIO(0x2034)

@@ -272,6 +272,10 @@ int i915_vma_bind(struct i915_vma *vma,
 		return ret;
 
 	vma->flags |= bind_flags;
+
+	if (vma->obj)
+		set_bit(I915_BO_WAS_BOUND_BIT, &vma->obj->flags);
+
 	return 0;
 }
 

This is the start of the stable review cycle for the 4.14.264 release. There are 2 patches in this series, all will be posted as a response to this one. If anyone has any issues with these being applied, please let me know.

Responses should be made by Sat, 29 Jan 2022 18:02:51 +0000. Anything received after that time might be too late.

The whole patch series can be found in one patch at:

	https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.14.264-rc1.gz

or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-4.14.y
and the diffstat can be found below.

thanks,

greg k-h

-------------
Pseudo-Shortlog of commits:

Greg Kroah-Hartman <[email protected]>

    Linux 4.14.264-rc1

Ziyang Xuan <[email protected]>
can: bcm: fix UAF of bcm op

Tvrtko Ursulin <[email protected]>
drm/i915: Flush TLBs before releasing backing store

-------------

Diffstat:

 Makefile                               |  4 +-
 drivers/gpu/drm/i915/i915_drv.h        |  2 +
 drivers/gpu/drm/i915/i915_gem.c        | 84 +++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_gem_object.h |  1 +
 drivers/gpu/drm/i915/i915_reg.h        |  6 +++
 drivers/gpu/drm/i915/i915_vma.c        |  4 ++
 net/can/bcm.c                          | 20 ++++----
 7 files changed, 108 insertions(+), 13 deletions(-)

From: Ziyang Xuan <[email protected]>

Stopping tasklet and hrtimer rely on the active state of tasklet and hrtimer sequentially in bcm_remove_op(), the op object will be freed if they are all unactive. Assume the hrtimer timeout is short, the hrtimer cb has been excuted after tasklet conditional judgment which must be false after last round tasklet_kill() and before condition hrtimer_active(), it is false when execute to hrtimer_active(). Bug is triggerd, because the stopping action is end and the op object will be freed, but the tasklet is scheduled. The resources of the op object will occur UAF bug.

Move hrtimer_cancel() behind tasklet_kill() and switch 'while () {...}' to 'do {...} while ()' to fix the op UAF problem.

---
 net/can/bcm.c |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

@@ -762,21 +762,21 @@ static struct bcm_op *bcm_find_op(struct
 static void bcm_remove_op(struct bcm_op *op)
 {
 	if (op->tsklet.func) {
-		while (test_bit(TASKLET_STATE_SCHED, &op->tsklet.state) ||
-		       test_bit(TASKLET_STATE_RUN, &op->tsklet.state) ||
-		       hrtimer_active(&op->timer)) {
-			hrtimer_cancel(&op->timer);
+		do {
 			tasklet_kill(&op->tsklet);
-		}
+			hrtimer_cancel(&op->timer);
+		} while (test_bit(TASKLET_STATE_SCHED, &op->tsklet.state) ||
+			 test_bit(TASKLET_STATE_RUN, &op->tsklet.state) ||
+			 hrtimer_active(&op->timer));
 	}
 
 	if (op->thrtsklet.func) {
-		while (test_bit(TASKLET_STATE_SCHED, &op->thrtsklet.state) ||
-		       test_bit(TASKLET_STATE_RUN, &op->thrtsklet.state) ||
-		       hrtimer_active(&op->thrtimer)) {
-			hrtimer_cancel(&op->thrtimer);
+		do {
 			tasklet_kill(&op->thrtsklet);
-		}
+			hrtimer_cancel(&op->thrtimer);
+		} while (test_bit(TASKLET_STATE_SCHED, &op->thrtsklet.state) ||
+			 test_bit(TASKLET_STATE_RUN, &op->thrtsklet.state) ||
+			 hrtimer_active(&op->thrtimer));
 	}
 
 	if ((op->frames) && (op->frames != &op->sframe))

All tests passing for Tegra ...

Test results for stable-v4.14:

    8 builds:	8 pass, 0 fail
    16 boots:	16 pass, 0 fail
    32 tests:	32 pass, 0 fail

Linux version: 4.14.264-rc1-ga816c082cb80
Boards tested: tegra124-jetson-tk1, tegra20-ventana,

                tegra210-p2371-2180, tegra30-cardhu-a04

Jon

Build results:

	total: 168 pass: 168 fail: 0
Qemu test results:
	total: 424 pass: 424 fail: 0

Guenter

Results from Linaro’s test farm.
No regressions on arm64, arm, x86_64, and i386.

## Build
* kernel: 4.14.264-rc1
* git: https://gitlab.com/Linaro/lkft/mirrors/stable/linux-stable-rc
* git branch: linux-4.14.y
* git commit: a816c082cb802807f6548940bb78b806ad74ca90
* git describe: v4.14.263-3-ga816c082cb80
* test details:
https://qa-reports.linaro.org/lkft/linux-stable-rc-linux-4.14.y/build/v4.14.263-3-ga816c082cb80

## Test Regressions (compared to v4.14.262-185-g1cb564222633) No test regressions found.

## Metric Regressions (compared to v4.14.262-185-g1cb564222633) No metric regressions found.

## Test Fixes (compared to v4.14.262-185-g1cb564222633) No test fixes found.

## Metric Fixes (compared to v4.14.262-185-g1cb564222633) No metric fixes found.

## Test result summary
total: 69622, pass: 55697, fail: 666, skip: 11294, xfail: 1965

## Build Summary
* arm: 250 total, 242 passed, 8 failed
* arm64: 32 total, 32 passed, 0 failed
* dragonboard-410c: 1 total, 1 passed, 0 failed
* hi6220-hikey: 1 total, 1 passed, 0 failed
* i386: 19 total, 19 passed, 0 failed
* juno-r2: 1 total, 1 passed, 0 failed
* mips: 22 total, 22 passed, 0 failed
* powerpc: 52 total, 0 passed, 52 failed
* sparc: 12 total, 12 passed, 0 failed
* x15: 1 total, 1 passed, 0 failed
* x86: 1 total, 1 passed, 0 failed
* x86_64: 31 total, 31 passed, 0 failed

## Test suites summary
* fwts
* igt-gpu-tools
* kselftest-android
* kselftest-arm64
* kselftest-arm64/arm64.btitest.bti_c_func
* kselftest-arm64/arm64.btitest.bti_j_func
* kselftest-arm64/arm64.btitest.bti_jc_func
* kselftest-arm64/arm64.btitest.bti_none_func
* kselftest-arm64/arm64.btitest.nohint_func
* kselftest-arm64/arm64.btitest.paciasp_func
* kselftest-arm64/arm64.nobtitest.bti_c_func
* kselftest-arm64/arm64.nobtitest.bti_j_func
* kselftest-arm64/arm64.nobtitest.bti_jc_func
* kselftest-arm64/arm64.nobtitest.bti_none_func
* kselftest-arm64/arm64.nobtitest.nohint_func
* kselftest-arm64/arm64.nobtitest.paciasp_func
* kselftest-bpf
* kselftest-breakpoints
* kselftest-capabilities
* kselftest-cgroup
* kselftest-clone3
* kselftest-core
* kselftest-cpu-hotplug
* kselftest-cpufreq
* kselftest-drivers
* kselftest-efivarfs
* kselftest-filesystems
* kselftest-firmware
* kselftest-fpu
* kselftest-futex
* kselftest-gpio
* kselftest-intel_pstate
* kselftest-ipc
* kselftest-ir
* kselftest-kcmp
* kselftest-kexec
* kselftest-kvm
* kselftest-lib
* kselftest-livepatch
* kselftest-membarrier
* kselftest-net
* kselftest-netfilter
* kselftest-nsfs
* kselftest-openat2
* kselftest-pid_namespace
* kselftest-pidfd
* kselftest-proc
* kselftest-pstore
* kselftest-ptrace
* kselftest-rseq
* kselftest-rtc
* kselftest-seccomp
* kselftest-sigaltstack
* kselftest-size
* kselftest-splice
* kselftest-static_keys
* kselftest-sync
* kselftest-sysctl
* kselftest-tc-testing
* kselftest-timens
* kselftest-timers
* kselftest-tmpfs
* kselftest-tpm2
* kselftest-user
* kselftest-vm
* kselftest-x86
* kselftest-zram
* kvm-unit-tests
* libhugetlbfs
* linux-log-parser
* ltp-cap_bounds-tests
* ltp-commands-tests
* ltp-containers-tests
* ltp-controllers-tests
* ltp-cpuhotplug-tests
* ltp-crypto-tests
* ltp-cve-tests
* ltp-dio-tests
* ltp-fcntl-locktests-tests
* ltp-filecaps-tests
* ltp-fs-tests
* ltp-fs_bind-tests
* ltp-fs_perms_simple-tests
* ltp-fsx-tests
* ltp-hugetlb-tests
* ltp-io-tests
* ltp-ipc-tests
* ltp-math-tests
* ltp-mm-tests
* ltp-nptl-tests
* ltp-open-posix-tests
* ltp-pty-tests
* ltp-sched-tests
* ltp-securebits-tests
* ltp-syscalls-tests
* ltp-tracing-tests
* network-basic-tests
* packetdrill
* perf
* rcutorture
* ssuite
* v4l2-compliance

--
Linaro LKFT
https://lkft.linaro.org

Hi!

I don't see this commit in mainline or next kernels. What is going on here? Is it one of those "only needed in -stable" things? Or do we still need to fix it in the mainline?

Best regards,
Pavel

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Please see the stable list discussion of this commit for other branches, it should answer your question.

thanks,

greg k-h