From fd66c7799866440da13c85c77be2876a487896c1 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sat, 22 Jul 2023 13:45:56 +0200 Subject: [PATCH] drop some drm patches that were not needed. --- ...se-after-free-in-nonblocking-commits.patch | 91 +++++++++++++++++++ queue-4.14/series | 1 + ...se-after-free-in-nonblocking-commits.patch | 91 +++++++++++++++++++ queue-4.19/series | 1 + ...se-after-free-in-nonblocking-commits.patch | 91 +++++++++++++++++++ queue-5.10/series | 1 + ...se-after-free-in-nonblocking-commits.patch | 91 +++++++++++++++++++ ...onsider-pinned-bos-for-eviction-swap.patch | 41 --------- queue-5.15/series | 2 +- ...se-after-free-in-nonblocking-commits.patch | 91 +++++++++++++++++++ queue-5.4/series | 1 + ...se-after-free-in-nonblocking-commits.patch | 91 +++++++++++++++++++ ...onsider-pinned-bos-for-eviction-swap.patch | 41 --------- queue-6.1/series | 2 +- ...se-after-free-in-nonblocking-commits.patch | 91 +++++++++++++++++++ ...onsider-pinned-bos-for-eviction-swap.patch | 41 --------- queue-6.4/series | 3 +- queue-6.4/x86-fineibt-poison-endbr-at-0.patch | 89 ------------------ 18 files changed, 644 insertions(+), 216 deletions(-) create mode 100644 queue-4.14/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch create mode 100644 queue-4.19/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch create mode 100644 queue-5.10/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch create mode 100644 queue-5.15/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch delete mode 100644 queue-5.15/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch create mode 100644 queue-5.4/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch create mode 100644 queue-6.1/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch delete mode 100644 queue-6.1/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch create mode 100644 queue-6.4/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch delete mode 100644 queue-6.4/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch delete mode 100644 queue-6.4/x86-fineibt-poison-endbr-at-0.patch diff --git a/queue-4.14/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch b/queue-4.14/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch new file mode 100644 index 00000000000..ea9567dfdb7 --- /dev/null +++ b/queue-4.14/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch @@ -0,0 +1,91 @@ +From 4e076c73e4f6e90816b30fcd4a0d7ab365087255 Mon Sep 17 00:00:00 2001 +From: Daniel Vetter +Date: Fri, 21 Jul 2023 15:58:38 +0200 +Subject: drm/atomic: Fix potential use-after-free in nonblocking commits + +From: Daniel Vetter + +commit 4e076c73e4f6e90816b30fcd4a0d7ab365087255 upstream. + +This requires a bit of background. Properly done a modeset driver's +unload/remove sequence should be + + drm_dev_unplug(); + drm_atomic_helper_shutdown(); + drm_dev_put(); + +The trouble is that the drm_dev_unplugged() checks are by design racy, +they do not synchronize against all outstanding ioctl. This is because +those ioctl could block forever (both for modeset and for driver +specific ioctls), leading to deadlocks in hotunplug. Instead the code +sections that touch the hardware need to be annotated with +drm_dev_enter/exit, to avoid accessing hardware resources after the +unload/remove has finished. + +To avoid use-after-free issues all the involved userspace visible +objects are supposed to hold a reference on the underlying drm_device, +like drm_file does. + +The issue now is that we missed one, the atomic modeset ioctl can be run +in a nonblocking fashion, and in that case it cannot rely on the implied +drm_device reference provided by the ioctl calling context. This can +result in a use-after-free if an nonblocking atomic commit is carefully +raced against a driver unload. + +Fix this by unconditionally grabbing a drm_device reference for any +drm_atomic_state structures. Strictly speaking this isn't required for +blocking commits and TEST_ONLY calls, but it's the simpler approach. + +Thanks to shanzhulig for the initial idea of grabbing an unconditional +reference, I just added comments, a condensed commit message and fixed a +minor potential issue in where exactly we drop the final reference. + +Reported-by: shanzhulig +Suggested-by: shanzhulig +Reviewed-by: Maxime Ripard +Cc: Maarten Lankhorst +Cc: Thomas Zimmermann +Cc: David Airlie +Cc: stable@kernel.org +Signed-off-by: Daniel Vetter +Signed-off-by: Daniel Vetter +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman +--- + drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- + 1 file changed, 10 insertions(+), 1 deletion(-) + +--- a/drivers/gpu/drm/drm_atomic.c ++++ b/drivers/gpu/drm/drm_atomic.c +@@ -87,6 +87,12 @@ drm_atomic_state_init(struct drm_device + if (!state->planes) + goto fail; + ++ /* ++ * Because drm_atomic_state can be committed asynchronously we need our ++ * own reference and cannot rely on the on implied by drm_file in the ++ * ioctl call. ++ */ ++ drm_dev_get(dev); + state->dev = dev; + + DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state); +@@ -246,7 +252,8 @@ EXPORT_SYMBOL(drm_atomic_state_clear); + void __drm_atomic_state_free(struct kref *ref) + { + struct drm_atomic_state *state = container_of(ref, typeof(*state), ref); +- struct drm_mode_config *config = &state->dev->mode_config; ++ struct drm_device *dev = state->dev; ++ struct drm_mode_config *config = &dev->mode_config; + + drm_atomic_state_clear(state); + +@@ -258,6 +265,8 @@ void __drm_atomic_state_free(struct kref + drm_atomic_state_default_release(state); + kfree(state); + } ++ ++ drm_dev_put(dev); + } + EXPORT_SYMBOL(__drm_atomic_state_free); + diff --git a/queue-4.14/series b/queue-4.14/series index 9652e39578c..00294c5ffaa 100644 --- a/queue-4.14/series +++ b/queue-4.14/series @@ -121,4 +121,5 @@ scsi-qla2xxx-wait-for-io-return-on-terminate-rport.patch scsi-qla2xxx-fix-potential-null-pointer-dereference.patch scsi-qla2xxx-check-valid-rport-returned-by-fc_bsg_to_rport.patch scsi-qla2xxx-pointer-may-be-dereferenced.patch +drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch serial-atmel-don-t-enable-irqs-prematurely.patch diff --git a/queue-4.19/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch b/queue-4.19/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch new file mode 100644 index 00000000000..f1faf8d442d --- /dev/null +++ b/queue-4.19/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch @@ -0,0 +1,91 @@ +From 4e076c73e4f6e90816b30fcd4a0d7ab365087255 Mon Sep 17 00:00:00 2001 +From: Daniel Vetter +Date: Fri, 21 Jul 2023 15:58:38 +0200 +Subject: drm/atomic: Fix potential use-after-free in nonblocking commits + +From: Daniel Vetter + +commit 4e076c73e4f6e90816b30fcd4a0d7ab365087255 upstream. + +This requires a bit of background. Properly done a modeset driver's +unload/remove sequence should be + + drm_dev_unplug(); + drm_atomic_helper_shutdown(); + drm_dev_put(); + +The trouble is that the drm_dev_unplugged() checks are by design racy, +they do not synchronize against all outstanding ioctl. This is because +those ioctl could block forever (both for modeset and for driver +specific ioctls), leading to deadlocks in hotunplug. Instead the code +sections that touch the hardware need to be annotated with +drm_dev_enter/exit, to avoid accessing hardware resources after the +unload/remove has finished. + +To avoid use-after-free issues all the involved userspace visible +objects are supposed to hold a reference on the underlying drm_device, +like drm_file does. + +The issue now is that we missed one, the atomic modeset ioctl can be run +in a nonblocking fashion, and in that case it cannot rely on the implied +drm_device reference provided by the ioctl calling context. This can +result in a use-after-free if an nonblocking atomic commit is carefully +raced against a driver unload. + +Fix this by unconditionally grabbing a drm_device reference for any +drm_atomic_state structures. Strictly speaking this isn't required for +blocking commits and TEST_ONLY calls, but it's the simpler approach. + +Thanks to shanzhulig for the initial idea of grabbing an unconditional +reference, I just added comments, a condensed commit message and fixed a +minor potential issue in where exactly we drop the final reference. + +Reported-by: shanzhulig +Suggested-by: shanzhulig +Reviewed-by: Maxime Ripard +Cc: Maarten Lankhorst +Cc: Thomas Zimmermann +Cc: David Airlie +Cc: stable@kernel.org +Signed-off-by: Daniel Vetter +Signed-off-by: Daniel Vetter +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman +--- + drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- + 1 file changed, 10 insertions(+), 1 deletion(-) + +--- a/drivers/gpu/drm/drm_atomic.c ++++ b/drivers/gpu/drm/drm_atomic.c +@@ -91,6 +91,12 @@ drm_atomic_state_init(struct drm_device + if (!state->planes) + goto fail; + ++ /* ++ * Because drm_atomic_state can be committed asynchronously we need our ++ * own reference and cannot rely on the on implied by drm_file in the ++ * ioctl call. ++ */ ++ drm_dev_get(dev); + state->dev = dev; + + DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state); +@@ -250,7 +256,8 @@ EXPORT_SYMBOL(drm_atomic_state_clear); + void __drm_atomic_state_free(struct kref *ref) + { + struct drm_atomic_state *state = container_of(ref, typeof(*state), ref); +- struct drm_mode_config *config = &state->dev->mode_config; ++ struct drm_device *dev = state->dev; ++ struct drm_mode_config *config = &dev->mode_config; + + drm_atomic_state_clear(state); + +@@ -262,6 +269,8 @@ void __drm_atomic_state_free(struct kref + drm_atomic_state_default_release(state); + kfree(state); + } ++ ++ drm_dev_put(dev); + } + EXPORT_SYMBOL(__drm_atomic_state_free); + diff --git a/queue-4.19/series b/queue-4.19/series index bb891c30b45..0cac5f95551 100644 --- a/queue-4.19/series +++ b/queue-4.19/series @@ -185,3 +185,4 @@ scsi-qla2xxx-wait-for-io-return-on-terminate-rport.patch scsi-qla2xxx-fix-potential-null-pointer-dereference.patch scsi-qla2xxx-check-valid-rport-returned-by-fc_bsg_to_rport.patch scsi-qla2xxx-pointer-may-be-dereferenced.patch +drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch diff --git a/queue-5.10/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch b/queue-5.10/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch new file mode 100644 index 00000000000..deeb6a06767 --- /dev/null +++ b/queue-5.10/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch @@ -0,0 +1,91 @@ +From 4e076c73e4f6e90816b30fcd4a0d7ab365087255 Mon Sep 17 00:00:00 2001 +From: Daniel Vetter +Date: Fri, 21 Jul 2023 15:58:38 +0200 +Subject: drm/atomic: Fix potential use-after-free in nonblocking commits + +From: Daniel Vetter + +commit 4e076c73e4f6e90816b30fcd4a0d7ab365087255 upstream. + +This requires a bit of background. Properly done a modeset driver's +unload/remove sequence should be + + drm_dev_unplug(); + drm_atomic_helper_shutdown(); + drm_dev_put(); + +The trouble is that the drm_dev_unplugged() checks are by design racy, +they do not synchronize against all outstanding ioctl. This is because +those ioctl could block forever (both for modeset and for driver +specific ioctls), leading to deadlocks in hotunplug. Instead the code +sections that touch the hardware need to be annotated with +drm_dev_enter/exit, to avoid accessing hardware resources after the +unload/remove has finished. + +To avoid use-after-free issues all the involved userspace visible +objects are supposed to hold a reference on the underlying drm_device, +like drm_file does. + +The issue now is that we missed one, the atomic modeset ioctl can be run +in a nonblocking fashion, and in that case it cannot rely on the implied +drm_device reference provided by the ioctl calling context. This can +result in a use-after-free if an nonblocking atomic commit is carefully +raced against a driver unload. + +Fix this by unconditionally grabbing a drm_device reference for any +drm_atomic_state structures. Strictly speaking this isn't required for +blocking commits and TEST_ONLY calls, but it's the simpler approach. + +Thanks to shanzhulig for the initial idea of grabbing an unconditional +reference, I just added comments, a condensed commit message and fixed a +minor potential issue in where exactly we drop the final reference. + +Reported-by: shanzhulig +Suggested-by: shanzhulig +Reviewed-by: Maxime Ripard +Cc: Maarten Lankhorst +Cc: Thomas Zimmermann +Cc: David Airlie +Cc: stable@kernel.org +Signed-off-by: Daniel Vetter +Signed-off-by: Daniel Vetter +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman +--- + drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- + 1 file changed, 10 insertions(+), 1 deletion(-) + +--- a/drivers/gpu/drm/drm_atomic.c ++++ b/drivers/gpu/drm/drm_atomic.c +@@ -98,6 +98,12 @@ drm_atomic_state_init(struct drm_device + if (!state->planes) + goto fail; + ++ /* ++ * Because drm_atomic_state can be committed asynchronously we need our ++ * own reference and cannot rely on the on implied by drm_file in the ++ * ioctl call. ++ */ ++ drm_dev_get(dev); + state->dev = dev; + + DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state); +@@ -257,7 +263,8 @@ EXPORT_SYMBOL(drm_atomic_state_clear); + void __drm_atomic_state_free(struct kref *ref) + { + struct drm_atomic_state *state = container_of(ref, typeof(*state), ref); +- struct drm_mode_config *config = &state->dev->mode_config; ++ struct drm_device *dev = state->dev; ++ struct drm_mode_config *config = &dev->mode_config; + + drm_atomic_state_clear(state); + +@@ -269,6 +276,8 @@ void __drm_atomic_state_free(struct kref + drm_atomic_state_default_release(state); + kfree(state); + } ++ ++ drm_dev_put(dev); + } + EXPORT_SYMBOL(__drm_atomic_state_free); + diff --git a/queue-5.10/series b/queue-5.10/series index a7aac9a1e17..499228db222 100644 --- a/queue-5.10/series +++ b/queue-5.10/series @@ -438,3 +438,4 @@ scsi-qla2xxx-pointer-may-be-dereferenced.patch scsi-qla2xxx-remove-unused-nvme_ls_waitq-wait-queue.patch net-sched-sch_qfq-reintroduce-lmax-bound-check-for-mtu.patch rdma-cma-ensure-rdma_addr_cancel-happens-before-issuing-more-requests.patch +drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch diff --git a/queue-5.15/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch b/queue-5.15/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch new file mode 100644 index 00000000000..ad4cad2fe8e --- /dev/null +++ b/queue-5.15/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch @@ -0,0 +1,91 @@ +From 4e076c73e4f6e90816b30fcd4a0d7ab365087255 Mon Sep 17 00:00:00 2001 +From: Daniel Vetter +Date: Fri, 21 Jul 2023 15:58:38 +0200 +Subject: drm/atomic: Fix potential use-after-free in nonblocking commits + +From: Daniel Vetter + +commit 4e076c73e4f6e90816b30fcd4a0d7ab365087255 upstream. + +This requires a bit of background. Properly done a modeset driver's +unload/remove sequence should be + + drm_dev_unplug(); + drm_atomic_helper_shutdown(); + drm_dev_put(); + +The trouble is that the drm_dev_unplugged() checks are by design racy, +they do not synchronize against all outstanding ioctl. This is because +those ioctl could block forever (both for modeset and for driver +specific ioctls), leading to deadlocks in hotunplug. Instead the code +sections that touch the hardware need to be annotated with +drm_dev_enter/exit, to avoid accessing hardware resources after the +unload/remove has finished. + +To avoid use-after-free issues all the involved userspace visible +objects are supposed to hold a reference on the underlying drm_device, +like drm_file does. + +The issue now is that we missed one, the atomic modeset ioctl can be run +in a nonblocking fashion, and in that case it cannot rely on the implied +drm_device reference provided by the ioctl calling context. This can +result in a use-after-free if an nonblocking atomic commit is carefully +raced against a driver unload. + +Fix this by unconditionally grabbing a drm_device reference for any +drm_atomic_state structures. Strictly speaking this isn't required for +blocking commits and TEST_ONLY calls, but it's the simpler approach. + +Thanks to shanzhulig for the initial idea of grabbing an unconditional +reference, I just added comments, a condensed commit message and fixed a +minor potential issue in where exactly we drop the final reference. + +Reported-by: shanzhulig +Suggested-by: shanzhulig +Reviewed-by: Maxime Ripard +Cc: Maarten Lankhorst +Cc: Thomas Zimmermann +Cc: David Airlie +Cc: stable@kernel.org +Signed-off-by: Daniel Vetter +Signed-off-by: Daniel Vetter +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman +--- + drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- + 1 file changed, 10 insertions(+), 1 deletion(-) + +--- a/drivers/gpu/drm/drm_atomic.c ++++ b/drivers/gpu/drm/drm_atomic.c +@@ -138,6 +138,12 @@ drm_atomic_state_init(struct drm_device + if (!state->planes) + goto fail; + ++ /* ++ * Because drm_atomic_state can be committed asynchronously we need our ++ * own reference and cannot rely on the on implied by drm_file in the ++ * ioctl call. ++ */ ++ drm_dev_get(dev); + state->dev = dev; + + DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state); +@@ -297,7 +303,8 @@ EXPORT_SYMBOL(drm_atomic_state_clear); + void __drm_atomic_state_free(struct kref *ref) + { + struct drm_atomic_state *state = container_of(ref, typeof(*state), ref); +- struct drm_mode_config *config = &state->dev->mode_config; ++ struct drm_device *dev = state->dev; ++ struct drm_mode_config *config = &dev->mode_config; + + drm_atomic_state_clear(state); + +@@ -309,6 +316,8 @@ void __drm_atomic_state_free(struct kref + drm_atomic_state_default_release(state); + kfree(state); + } ++ ++ drm_dev_put(dev); + } + EXPORT_SYMBOL(__drm_atomic_state_free); + diff --git a/queue-5.15/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch b/queue-5.15/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch deleted file mode 100644 index bed5ba2b9df..00000000000 --- a/queue-5.15/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch +++ /dev/null @@ -1,41 +0,0 @@ -From a2848d08742c8e8494675892c02c0d22acbe3cf8 Mon Sep 17 00:00:00 2001 -From: =?UTF-8?q?Christian=20K=C3=B6nig?= -Date: Fri, 7 Jul 2023 11:25:00 +0200 -Subject: drm/ttm: never consider pinned BOs for eviction&swap -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -From: Christian König - -commit a2848d08742c8e8494675892c02c0d22acbe3cf8 upstream. - -There is a small window where we have already incremented the pin count -but not yet moved the bo from the lru to the pinned list. - -Signed-off-by: Christian König -Reported-by: Pelloux-Prayer, Pierre-Eric -Tested-by: Pelloux-Prayer, Pierre-Eric -Acked-by: Alex Deucher -Cc: stable@vger.kernel.org -Link: https://patchwork.freedesktop.org/patch/msgid/20230707120826.3701-1-christian.koenig@amd.com -Signed-off-by: Greg Kroah-Hartman ---- - drivers/gpu/drm/ttm/ttm_bo.c | 6 ++++++ - 1 file changed, 6 insertions(+) - ---- a/drivers/gpu/drm/ttm/ttm_bo.c -+++ b/drivers/gpu/drm/ttm/ttm_bo.c -@@ -603,6 +603,12 @@ static bool ttm_bo_evict_swapout_allowab - { - bool ret = false; - -+ if (bo->pin_count) { -+ *locked = false; -+ *busy = false; -+ return false; -+ } -+ - if (bo->base.resv == ctx->resv) { - dma_resv_assert_held(bo->base.resv); - if (ctx->allow_res_evict) diff --git a/queue-5.15/series b/queue-5.15/series index 2b683d42487..220ef9a4cd3 100644 --- a/queue-5.15/series +++ b/queue-5.15/series @@ -490,7 +490,6 @@ drm-amdgpu-fix-clearing-mappings-for-bos-that-are-always-valid-in-vm.patch drm-amd-display-correct-dmub_fw_version-macro.patch drm-amdgpu-avoid-restore-process-run-into-dead-loop.patch drm-ttm-don-t-leak-a-resource-on-swapout-move-error.patch -drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch serial-atmel-don-t-enable-irqs-prematurely.patch tty-serial-samsung_tty-fix-a-memory-leak-in-s3c24xx_serial_getclk-in-case-of-error.patch tty-serial-samsung_tty-fix-a-memory-leak-in-s3c24xx_serial_getclk-when-iterating-clk.patch @@ -530,3 +529,4 @@ scsi-qla2xxx-pointer-may-be-dereferenced.patch scsi-qla2xxx-remove-unused-nvme_ls_waitq-wait-queue.patch mips-kvm-fix-build-error-with-kvm_mips_debug_cop0_counters-enabled.patch net-sched-sch_qfq-reintroduce-lmax-bound-check-for-mtu.patch +drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch diff --git a/queue-5.4/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch b/queue-5.4/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch new file mode 100644 index 00000000000..dafb7a21dd9 --- /dev/null +++ b/queue-5.4/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch @@ -0,0 +1,91 @@ +From 4e076c73e4f6e90816b30fcd4a0d7ab365087255 Mon Sep 17 00:00:00 2001 +From: Daniel Vetter +Date: Fri, 21 Jul 2023 15:58:38 +0200 +Subject: drm/atomic: Fix potential use-after-free in nonblocking commits + +From: Daniel Vetter + +commit 4e076c73e4f6e90816b30fcd4a0d7ab365087255 upstream. + +This requires a bit of background. Properly done a modeset driver's +unload/remove sequence should be + + drm_dev_unplug(); + drm_atomic_helper_shutdown(); + drm_dev_put(); + +The trouble is that the drm_dev_unplugged() checks are by design racy, +they do not synchronize against all outstanding ioctl. This is because +those ioctl could block forever (both for modeset and for driver +specific ioctls), leading to deadlocks in hotunplug. Instead the code +sections that touch the hardware need to be annotated with +drm_dev_enter/exit, to avoid accessing hardware resources after the +unload/remove has finished. + +To avoid use-after-free issues all the involved userspace visible +objects are supposed to hold a reference on the underlying drm_device, +like drm_file does. + +The issue now is that we missed one, the atomic modeset ioctl can be run +in a nonblocking fashion, and in that case it cannot rely on the implied +drm_device reference provided by the ioctl calling context. This can +result in a use-after-free if an nonblocking atomic commit is carefully +raced against a driver unload. + +Fix this by unconditionally grabbing a drm_device reference for any +drm_atomic_state structures. Strictly speaking this isn't required for +blocking commits and TEST_ONLY calls, but it's the simpler approach. + +Thanks to shanzhulig for the initial idea of grabbing an unconditional +reference, I just added comments, a condensed commit message and fixed a +minor potential issue in where exactly we drop the final reference. + +Reported-by: shanzhulig +Suggested-by: shanzhulig +Reviewed-by: Maxime Ripard +Cc: Maarten Lankhorst +Cc: Thomas Zimmermann +Cc: David Airlie +Cc: stable@kernel.org +Signed-off-by: Daniel Vetter +Signed-off-by: Daniel Vetter +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman +--- + drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- + 1 file changed, 10 insertions(+), 1 deletion(-) + +--- a/drivers/gpu/drm/drm_atomic.c ++++ b/drivers/gpu/drm/drm_atomic.c +@@ -97,6 +97,12 @@ drm_atomic_state_init(struct drm_device + if (!state->planes) + goto fail; + ++ /* ++ * Because drm_atomic_state can be committed asynchronously we need our ++ * own reference and cannot rely on the on implied by drm_file in the ++ * ioctl call. ++ */ ++ drm_dev_get(dev); + state->dev = dev; + + DRM_DEBUG_ATOMIC("Allocated atomic state %p\n", state); +@@ -256,7 +262,8 @@ EXPORT_SYMBOL(drm_atomic_state_clear); + void __drm_atomic_state_free(struct kref *ref) + { + struct drm_atomic_state *state = container_of(ref, typeof(*state), ref); +- struct drm_mode_config *config = &state->dev->mode_config; ++ struct drm_device *dev = state->dev; ++ struct drm_mode_config *config = &dev->mode_config; + + drm_atomic_state_clear(state); + +@@ -268,6 +275,8 @@ void __drm_atomic_state_free(struct kref + drm_atomic_state_default_release(state); + kfree(state); + } ++ ++ drm_dev_put(dev); + } + EXPORT_SYMBOL(__drm_atomic_state_free); + diff --git a/queue-5.4/series b/queue-5.4/series index efb7a2b3968..a741f1b10cb 100644 --- a/queue-5.4/series +++ b/queue-5.4/series @@ -275,3 +275,4 @@ scsi-qla2xxx-check-valid-rport-returned-by-fc_bsg_to_rport.patch scsi-qla2xxx-correct-the-index-of-array.patch scsi-qla2xxx-pointer-may-be-dereferenced.patch scsi-qla2xxx-remove-unused-nvme_ls_waitq-wait-queue.patch +drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch diff --git a/queue-6.1/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch b/queue-6.1/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch new file mode 100644 index 00000000000..4a209ba7488 --- /dev/null +++ b/queue-6.1/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch @@ -0,0 +1,91 @@ +From 4e076c73e4f6e90816b30fcd4a0d7ab365087255 Mon Sep 17 00:00:00 2001 +From: Daniel Vetter +Date: Fri, 21 Jul 2023 15:58:38 +0200 +Subject: drm/atomic: Fix potential use-after-free in nonblocking commits + +From: Daniel Vetter + +commit 4e076c73e4f6e90816b30fcd4a0d7ab365087255 upstream. + +This requires a bit of background. Properly done a modeset driver's +unload/remove sequence should be + + drm_dev_unplug(); + drm_atomic_helper_shutdown(); + drm_dev_put(); + +The trouble is that the drm_dev_unplugged() checks are by design racy, +they do not synchronize against all outstanding ioctl. This is because +those ioctl could block forever (both for modeset and for driver +specific ioctls), leading to deadlocks in hotunplug. Instead the code +sections that touch the hardware need to be annotated with +drm_dev_enter/exit, to avoid accessing hardware resources after the +unload/remove has finished. + +To avoid use-after-free issues all the involved userspace visible +objects are supposed to hold a reference on the underlying drm_device, +like drm_file does. + +The issue now is that we missed one, the atomic modeset ioctl can be run +in a nonblocking fashion, and in that case it cannot rely on the implied +drm_device reference provided by the ioctl calling context. This can +result in a use-after-free if an nonblocking atomic commit is carefully +raced against a driver unload. + +Fix this by unconditionally grabbing a drm_device reference for any +drm_atomic_state structures. Strictly speaking this isn't required for +blocking commits and TEST_ONLY calls, but it's the simpler approach. + +Thanks to shanzhulig for the initial idea of grabbing an unconditional +reference, I just added comments, a condensed commit message and fixed a +minor potential issue in where exactly we drop the final reference. + +Reported-by: shanzhulig +Suggested-by: shanzhulig +Reviewed-by: Maxime Ripard +Cc: Maarten Lankhorst +Cc: Thomas Zimmermann +Cc: David Airlie +Cc: stable@kernel.org +Signed-off-by: Daniel Vetter +Signed-off-by: Daniel Vetter +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman +--- + drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- + 1 file changed, 10 insertions(+), 1 deletion(-) + +--- a/drivers/gpu/drm/drm_atomic.c ++++ b/drivers/gpu/drm/drm_atomic.c +@@ -140,6 +140,12 @@ drm_atomic_state_init(struct drm_device + if (!state->planes) + goto fail; + ++ /* ++ * Because drm_atomic_state can be committed asynchronously we need our ++ * own reference and cannot rely on the on implied by drm_file in the ++ * ioctl call. ++ */ ++ drm_dev_get(dev); + state->dev = dev; + + drm_dbg_atomic(dev, "Allocated atomic state %p\n", state); +@@ -299,7 +305,8 @@ EXPORT_SYMBOL(drm_atomic_state_clear); + void __drm_atomic_state_free(struct kref *ref) + { + struct drm_atomic_state *state = container_of(ref, typeof(*state), ref); +- struct drm_mode_config *config = &state->dev->mode_config; ++ struct drm_device *dev = state->dev; ++ struct drm_mode_config *config = &dev->mode_config; + + drm_atomic_state_clear(state); + +@@ -311,6 +318,8 @@ void __drm_atomic_state_free(struct kref + drm_atomic_state_default_release(state); + kfree(state); + } ++ ++ drm_dev_put(dev); + } + EXPORT_SYMBOL(__drm_atomic_state_free); + diff --git a/queue-6.1/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch b/queue-6.1/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch deleted file mode 100644 index aa2590b6ccf..00000000000 --- a/queue-6.1/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch +++ /dev/null @@ -1,41 +0,0 @@ -From a2848d08742c8e8494675892c02c0d22acbe3cf8 Mon Sep 17 00:00:00 2001 -From: =?UTF-8?q?Christian=20K=C3=B6nig?= -Date: Fri, 7 Jul 2023 11:25:00 +0200 -Subject: drm/ttm: never consider pinned BOs for eviction&swap -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -From: Christian König - -commit a2848d08742c8e8494675892c02c0d22acbe3cf8 upstream. - -There is a small window where we have already incremented the pin count -but not yet moved the bo from the lru to the pinned list. - -Signed-off-by: Christian König -Reported-by: Pelloux-Prayer, Pierre-Eric -Tested-by: Pelloux-Prayer, Pierre-Eric -Acked-by: Alex Deucher -Cc: stable@vger.kernel.org -Link: https://patchwork.freedesktop.org/patch/msgid/20230707120826.3701-1-christian.koenig@amd.com -Signed-off-by: Greg Kroah-Hartman ---- - drivers/gpu/drm/ttm/ttm_bo.c | 6 ++++++ - 1 file changed, 6 insertions(+) - ---- a/drivers/gpu/drm/ttm/ttm_bo.c -+++ b/drivers/gpu/drm/ttm/ttm_bo.c -@@ -549,6 +549,12 @@ static bool ttm_bo_evict_swapout_allowab - { - bool ret = false; - -+ if (bo->pin_count) { -+ *locked = false; -+ *busy = false; -+ return false; -+ } -+ - if (bo->base.resv == ctx->resv) { - dma_resv_assert_held(bo->base.resv); - if (ctx->allow_res_evict) diff --git a/queue-6.1/series b/queue-6.1/series index 830336051ba..5887242dd5e 100644 --- a/queue-6.1/series +++ b/queue-6.1/series @@ -153,7 +153,6 @@ drm-amd-display-correct-dmub_fw_version-macro.patch drm-amd-display-add-monitor-specific-edid-quirk.patch drm-amdgpu-avoid-restore-process-run-into-dead-loop.patch drm-ttm-don-t-leak-a-resource-on-swapout-move-error.patch -drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch serial-atmel-don-t-enable-irqs-prematurely.patch tty-serial-samsung_tty-fix-a-memory-leak-in-s3c24xx_serial_getclk-in-case-of-error.patch tty-serial-samsung_tty-fix-a-memory-leak-in-s3c24xx_serial_getclk-when-iterating-clk.patch @@ -221,3 +220,4 @@ mips-kvm-fix-build-error-with-kvm_mips_debug_cop0_counters-enabled.patch revert-drm-amd-disable-psr-su-on-parade-0803-tcon.patch swiotlb-mark-swiotlb_memblock_alloc-as-__init.patch net-sched-sch_qfq-reintroduce-lmax-bound-check-for-mtu.patch +drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch diff --git a/queue-6.4/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch b/queue-6.4/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch new file mode 100644 index 00000000000..4a209ba7488 --- /dev/null +++ b/queue-6.4/drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch @@ -0,0 +1,91 @@ +From 4e076c73e4f6e90816b30fcd4a0d7ab365087255 Mon Sep 17 00:00:00 2001 +From: Daniel Vetter +Date: Fri, 21 Jul 2023 15:58:38 +0200 +Subject: drm/atomic: Fix potential use-after-free in nonblocking commits + +From: Daniel Vetter + +commit 4e076c73e4f6e90816b30fcd4a0d7ab365087255 upstream. + +This requires a bit of background. Properly done a modeset driver's +unload/remove sequence should be + + drm_dev_unplug(); + drm_atomic_helper_shutdown(); + drm_dev_put(); + +The trouble is that the drm_dev_unplugged() checks are by design racy, +they do not synchronize against all outstanding ioctl. This is because +those ioctl could block forever (both for modeset and for driver +specific ioctls), leading to deadlocks in hotunplug. Instead the code +sections that touch the hardware need to be annotated with +drm_dev_enter/exit, to avoid accessing hardware resources after the +unload/remove has finished. + +To avoid use-after-free issues all the involved userspace visible +objects are supposed to hold a reference on the underlying drm_device, +like drm_file does. + +The issue now is that we missed one, the atomic modeset ioctl can be run +in a nonblocking fashion, and in that case it cannot rely on the implied +drm_device reference provided by the ioctl calling context. This can +result in a use-after-free if an nonblocking atomic commit is carefully +raced against a driver unload. + +Fix this by unconditionally grabbing a drm_device reference for any +drm_atomic_state structures. Strictly speaking this isn't required for +blocking commits and TEST_ONLY calls, but it's the simpler approach. + +Thanks to shanzhulig for the initial idea of grabbing an unconditional +reference, I just added comments, a condensed commit message and fixed a +minor potential issue in where exactly we drop the final reference. + +Reported-by: shanzhulig +Suggested-by: shanzhulig +Reviewed-by: Maxime Ripard +Cc: Maarten Lankhorst +Cc: Thomas Zimmermann +Cc: David Airlie +Cc: stable@kernel.org +Signed-off-by: Daniel Vetter +Signed-off-by: Daniel Vetter +Signed-off-by: Linus Torvalds +Signed-off-by: Greg Kroah-Hartman +--- + drivers/gpu/drm/drm_atomic.c | 11 ++++++++++- + 1 file changed, 10 insertions(+), 1 deletion(-) + +--- a/drivers/gpu/drm/drm_atomic.c ++++ b/drivers/gpu/drm/drm_atomic.c +@@ -140,6 +140,12 @@ drm_atomic_state_init(struct drm_device + if (!state->planes) + goto fail; + ++ /* ++ * Because drm_atomic_state can be committed asynchronously we need our ++ * own reference and cannot rely on the on implied by drm_file in the ++ * ioctl call. ++ */ ++ drm_dev_get(dev); + state->dev = dev; + + drm_dbg_atomic(dev, "Allocated atomic state %p\n", state); +@@ -299,7 +305,8 @@ EXPORT_SYMBOL(drm_atomic_state_clear); + void __drm_atomic_state_free(struct kref *ref) + { + struct drm_atomic_state *state = container_of(ref, typeof(*state), ref); +- struct drm_mode_config *config = &state->dev->mode_config; ++ struct drm_device *dev = state->dev; ++ struct drm_mode_config *config = &dev->mode_config; + + drm_atomic_state_clear(state); + +@@ -311,6 +318,8 @@ void __drm_atomic_state_free(struct kref + drm_atomic_state_default_release(state); + kfree(state); + } ++ ++ drm_dev_put(dev); + } + EXPORT_SYMBOL(__drm_atomic_state_free); + diff --git a/queue-6.4/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch b/queue-6.4/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch deleted file mode 100644 index a7f008a426d..00000000000 --- a/queue-6.4/drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch +++ /dev/null @@ -1,41 +0,0 @@ -From a2848d08742c8e8494675892c02c0d22acbe3cf8 Mon Sep 17 00:00:00 2001 -From: =?UTF-8?q?Christian=20K=C3=B6nig?= -Date: Fri, 7 Jul 2023 11:25:00 +0200 -Subject: drm/ttm: never consider pinned BOs for eviction&swap -MIME-Version: 1.0 -Content-Type: text/plain; charset=UTF-8 -Content-Transfer-Encoding: 8bit - -From: Christian König - -commit a2848d08742c8e8494675892c02c0d22acbe3cf8 upstream. - -There is a small window where we have already incremented the pin count -but not yet moved the bo from the lru to the pinned list. - -Signed-off-by: Christian König -Reported-by: Pelloux-Prayer, Pierre-Eric -Tested-by: Pelloux-Prayer, Pierre-Eric -Acked-by: Alex Deucher -Cc: stable@vger.kernel.org -Link: https://patchwork.freedesktop.org/patch/msgid/20230707120826.3701-1-christian.koenig@amd.com -Signed-off-by: Greg Kroah-Hartman ---- - drivers/gpu/drm/ttm/ttm_bo.c | 6 ++++++ - 1 file changed, 6 insertions(+) - ---- a/drivers/gpu/drm/ttm/ttm_bo.c -+++ b/drivers/gpu/drm/ttm/ttm_bo.c -@@ -517,6 +517,12 @@ static bool ttm_bo_evict_swapout_allowab - { - bool ret = false; - -+ if (bo->pin_count) { -+ *locked = false; -+ *busy = false; -+ return false; -+ } -+ - if (bo->base.resv == ctx->resv) { - dma_resv_assert_held(bo->base.resv); - if (ctx->allow_res_evict) diff --git a/queue-6.4/series b/queue-6.4/series index 5973b94fb10..1eb37b42502 100644 --- a/queue-6.4/series +++ b/queue-6.4/series @@ -64,7 +64,6 @@ ntb-ntb_tool-add-check-for-devm_kcalloc.patch ipv6-addrconf-fix-a-potential-refcount-underflow-for.patch hid-hyperv-avoid-struct-memcpy-overrun-warning.patch net-dsa-qca8k-add-check-for-skb_copy.patch -x86-fineibt-poison-endbr-at-0.patch platform-x86-wmi-break-possible-infinite-loop-when-p.patch net-sched-taprio-replace-tc_taprio_qopt_offload-enab.patch igc-rename-qbv_enable-to-taprio_offload_enable.patch @@ -210,7 +209,6 @@ drm-amdgpu-avoid-restore-process-run-into-dead-loop.patch drm-amd-pm-fix-smu-i2c-data-read-risk.patch drm-ttm-don-t-leak-a-resource-on-eviction-error.patch drm-ttm-don-t-leak-a-resource-on-swapout-move-error.patch -drm-ttm-never-consider-pinned-bos-for-eviction-swap.patch serial-atmel-don-t-enable-irqs-prematurely.patch tty-serial-samsung_tty-fix-a-memory-leak-in-s3c24xx_serial_getclk-in-case-of-error.patch tty-serial-samsung_tty-fix-a-memory-leak-in-s3c24xx_serial_getclk-when-iterating-clk.patch @@ -290,3 +288,4 @@ scsi-qla2xxx-fix-end-of-loop-test.patch net-dsa-ocelot-unlock-on-error-in-vsc9959_qos_port_tas_set.patch mips-kvm-fix-build-error-with-kvm_mips_debug_cop0_counters-enabled.patch revert-drm-amd-disable-psr-su-on-parade-0803-tcon.patch +drm-atomic-fix-potential-use-after-free-in-nonblocking-commits.patch diff --git a/queue-6.4/x86-fineibt-poison-endbr-at-0.patch b/queue-6.4/x86-fineibt-poison-endbr-at-0.patch deleted file mode 100644 index 40fbeec819e..00000000000 --- a/queue-6.4/x86-fineibt-poison-endbr-at-0.patch +++ /dev/null @@ -1,89 +0,0 @@ -From 79f7f4bdb8ae801346e94933d8c848c76e4ea88b Mon Sep 17 00:00:00 2001 -From: Sasha Levin -Date: Thu, 15 Jun 2023 21:35:48 +0200 -Subject: x86/fineibt: Poison ENDBR at +0 - -From: Peter Zijlstra - -[ Upstream commit 04505bbbbb15da950ea0239e328a76a3ad2376e0 ] - -Alyssa noticed that when building the kernel with CFI_CLANG+IBT and -booting on IBT enabled hardware to obtain FineIBT, the indirect -functions look like: - - __cfi_foo: - endbr64 - subl $hash, %r10d - jz 1f - ud2 - nop - 1: - foo: - endbr64 - -This is because the compiler generates code for kCFI+IBT. In that case -the caller does the hash check and will jump to +0, so there must be -an ENDBR there. The compiler doesn't know about FineIBT at all; also -it is possible to actually use kCFI+IBT when booting with 'cfi=kcfi' -on IBT enabled hardware. - -Having this second ENDBR however makes it possible to elide the CFI -check. Therefore, we should poison this second ENDBR when switching to -FineIBT mode. - -Fixes: 931ab63664f0 ("x86/ibt: Implement FineIBT") -Reported-by: "Milburn, Alyssa" -Signed-off-by: Peter Zijlstra (Intel) -Reviewed-by: Kees Cook -Reviewed-by: Sami Tolvanen -Link: https://lore.kernel.org/r/20230615193722.194131053@infradead.org -Signed-off-by: Sasha Levin ---- - arch/x86/kernel/alternative.c | 16 ++++++++++++++++ - 1 file changed, 16 insertions(+) - -diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c -index f615e0cb6d932..4e2c70f88e05b 100644 ---- a/arch/x86/kernel/alternative.c -+++ b/arch/x86/kernel/alternative.c -@@ -940,6 +940,17 @@ static int cfi_rewrite_preamble(s32 *start, s32 *end) - return 0; - } - -+static void cfi_rewrite_endbr(s32 *start, s32 *end) -+{ -+ s32 *s; -+ -+ for (s = start; s < end; s++) { -+ void *addr = (void *)s + *s; -+ -+ poison_endbr(addr+16, false); -+ } -+} -+ - /* .retpoline_sites */ - static int cfi_rand_callers(s32 *start, s32 *end) - { -@@ -1034,14 +1045,19 @@ static void __apply_fineibt(s32 *start_retpoline, s32 *end_retpoline, - return; - - case CFI_FINEIBT: -+ /* place the FineIBT preamble at func()-16 */ - ret = cfi_rewrite_preamble(start_cfi, end_cfi); - if (ret) - goto err; - -+ /* rewrite the callers to target func()-16 */ - ret = cfi_rewrite_callers(start_retpoline, end_retpoline); - if (ret) - goto err; - -+ /* now that nobody targets func()+0, remove ENDBR there */ -+ cfi_rewrite_endbr(start_cfi, end_cfi); -+ - if (builtin) - pr_info("Using FineIBT CFI\n"); - return; --- -2.39.2 - -- 2.47.3