| Message ID | 20241125205314.1725887-1-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/dp_mst: Fix MST sideband message body length check | expand |
Reviewed-by: Lyude Paul <lyude@redhat.com> On Mon, 2024-11-25 at 22:53 +0200, Imre Deak wrote: > Fix the MST sideband message body length check, which must be at least 1 > byte accounting for the message body CRC (aka message data CRC) at the > end of the message. > > This fixes a case where an MST branch device returns a header with a > correct header CRC (indicating a correctly received body length), with > the body length being incorrectly set to 0. This will later lead to a > memory corruption in drm_dp_sideband_append_payload() and the following > errors in dmesg: > > UBSAN: array-index-out-of-bounds in drivers/gpu/drm/display/drm_dp_mst_topology.c:786:25 > index -1 is out of range for type 'u8 [48]' > Call Trace: > drm_dp_sideband_append_payload+0x33d/0x350 [drm_display_helper] > drm_dp_get_one_sb_msg+0x3ce/0x5f0 [drm_display_helper] > drm_dp_mst_hpd_irq_handle_event+0xc8/0x1580 [drm_display_helper] > > memcpy: detected field-spanning write (size 18446744073709551615) of single field "&msg->msg[msg->curlen]" at drivers/gpu/drm/display/drm_dp_mst_topology.c:791 (size 256) > Call Trace: > drm_dp_sideband_append_payload+0x324/0x350 [drm_display_helper] > drm_dp_get_one_sb_msg+0x3ce/0x5f0 [drm_display_helper] > drm_dp_mst_hpd_irq_handle_event+0xc8/0x1580 [drm_display_helper] > > Cc: <stable@vger.kernel.org> > Cc: Lyude Paul <lyude@redhat.com> > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c > index ac90118b9e7a8..e6ee180815b20 100644 > --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c > @@ -320,6 +320,9 @@ static bool drm_dp_decode_sideband_msg_hdr(const struct drm_dp_mst_topology_mgr > hdr->broadcast = (buf[idx] >> 7) & 0x1; > hdr->path_msg = (buf[idx] >> 6) & 0x1; > hdr->msg_len = buf[idx] & 0x3f; > + if (hdr->msg_len < 1) /* min space for body CRC */ > + return false; > + > idx++; > hdr->somt = (buf[idx] >> 7) & 0x1; > hdr->eomt = (buf[idx] >> 6) & 0x1;
On Mon, Nov 25, 2024 at 10:04:46PM +0000, Patchwork wrote: > == Series Details == > > Series: drm/dp_mst: Fix MST sideband message body length check > URL : https://patchwork.freedesktop.org/series/141772/ > State : failure Thanks for the review, patch is pushed to drm-misc-fixes. The failure below is unrelated, since the KBL machine doesn't have an MST sink connected. > == Summary == > > CI Bug Log - changes from CI_DRM_15743 -> Patchwork_141772v1 > ==================================================== > > Summary > ------- > > **FAILURE** > > Serious unknown changes coming with Patchwork_141772v1 absolutely need to be > verified manually. > > If you think the reported changes have nothing to do with the changes > introduced in Patchwork_141772v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them > to document this new failure mode, which will reduce false positives in CI. > > External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/index.html > > Participating hosts (45 -> 44) > ------------------------------ > > Missing (1): fi-snb-2520m > > Possible new issues > ------------------- > > Here are the unknown changes that may have been introduced in Patchwork_141772v1: > > ### IGT changes ### > > #### Possible regressions #### > > * igt@i915_pm_rpm@module-reload: > - fi-kbl-7567u: [PASS][1] -> [DMESG-WARN][2] > [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15743/fi-kbl-7567u/igt@i915_pm_rpm@module-reload.html > [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/fi-kbl-7567u/igt@i915_pm_rpm@module-reload.html > > > Known issues > ------------ > > Here are the changes found in Patchwork_141772v1 that come from known issues: > > ### IGT changes ### > > #### Issues hit #### > > * igt@i915_pm_rpm@module-reload: > - bat-rpls-4: [PASS][3] -> [FAIL][4] ([i915#12903]) > [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15743/bat-rpls-4/igt@i915_pm_rpm@module-reload.html > [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/bat-rpls-4/igt@i915_pm_rpm@module-reload.html > > * igt@i915_selftest@live: > - bat-mtlp-8: [PASS][5] -> [ABORT][6] ([i915#12061]) +1 other test abort > [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15743/bat-mtlp-8/igt@i915_selftest@live.html > [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/bat-mtlp-8/igt@i915_selftest@live.html > > > #### Possible fixes #### > > * igt@dmabuf@all-tests@dma_fence_chain: > - fi-bsw-nick: [INCOMPLETE][7] ([i915#12904]) -> [PASS][8] +1 other test pass > [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15743/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_chain.html > [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_chain.html > > * igt@i915_selftest@live: > - bat-twl-2: [ABORT][9] ([i915#12919]) -> [PASS][10] > [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15743/bat-twl-2/igt@i915_selftest@live.html > [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/bat-twl-2/igt@i915_selftest@live.html > > * igt@i915_selftest@live@gt_pm: > - bat-twl-2: [ABORT][11] -> [PASS][12] > [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15743/bat-twl-2/igt@i915_selftest@live@gt_pm.html > [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/bat-twl-2/igt@i915_selftest@live@gt_pm.html > > * igt@i915_selftest@live@workarounds: > - bat-arlh-2: [ABORT][13] ([i915#12061]) -> [PASS][14] +1 other test pass > [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15743/bat-arlh-2/igt@i915_selftest@live@workarounds.html > [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/bat-arlh-2/igt@i915_selftest@live@workarounds.html > - {bat-arls-6}: [ABORT][15] ([i915#12061]) -> [PASS][16] +1 other test pass > [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15743/bat-arls-6/igt@i915_selftest@live@workarounds.html > [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/bat-arls-6/igt@i915_selftest@live@workarounds.html > > * igt@kms_flip@basic-flip-vs-modeset@a-dp1: > - bat-apl-1: [DMESG-WARN][17] -> [PASS][18] +1 other test pass > [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15743/bat-apl-1/igt@kms_flip@basic-flip-vs-modeset@a-dp1.html > [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/bat-apl-1/igt@kms_flip@basic-flip-vs-modeset@a-dp1.html > > > {name}: This element is suppressed. This means it is ignored when computing > the status of the difference (SUCCESS, WARNING, or FAILURE). > > [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061 > [i915#12903]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12903 > [i915#12904]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12904 > [i915#12919]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12919 > > > Build changes > ------------- > > * Linux: CI_DRM_15743 -> Patchwork_141772v1 > > CI-20190529: 20190529 > CI_DRM_15743: 6510562075d8541c218e72188f9ef339f8ba371f @ git://anongit.freedesktop.org/gfx-ci/linux > IGT_8124: 8124 > Patchwork_141772v1: 6510562075d8541c218e72188f9ef339f8ba371f @ git://anongit.freedesktop.org/gfx-ci/linux > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_141772v1/index.html
diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c index ac90118b9e7a8..e6ee180815b20 100644 --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c @@ -320,6 +320,9 @@ static bool drm_dp_decode_sideband_msg_hdr(const struct drm_dp_mst_topology_mgr hdr->broadcast = (buf[idx] >> 7) & 0x1; hdr->path_msg = (buf[idx] >> 6) & 0x1; hdr->msg_len = buf[idx] & 0x3f; + if (hdr->msg_len < 1) /* min space for body CRC */ + return false; + idx++; hdr->somt = (buf[idx] >> 7) & 0x1; hdr->eomt = (buf[idx] >> 6) & 0x1;
Fix the MST sideband message body length check, which must be at least 1 byte accounting for the message body CRC (aka message data CRC) at the end of the message. This fixes a case where an MST branch device returns a header with a correct header CRC (indicating a correctly received body length), with the body length being incorrectly set to 0. This will later lead to a memory corruption in drm_dp_sideband_append_payload() and the following errors in dmesg: UBSAN: array-index-out-of-bounds in drivers/gpu/drm/display/drm_dp_mst_topology.c:786:25 index -1 is out of range for type 'u8 [48]' Call Trace: drm_dp_sideband_append_payload+0x33d/0x350 [drm_display_helper] drm_dp_get_one_sb_msg+0x3ce/0x5f0 [drm_display_helper] drm_dp_mst_hpd_irq_handle_event+0xc8/0x1580 [drm_display_helper] memcpy: detected field-spanning write (size 18446744073709551615) of single field "&msg->msg[msg->curlen]" at drivers/gpu/drm/display/drm_dp_mst_topology.c:791 (size 256) Call Trace: drm_dp_sideband_append_payload+0x324/0x350 [drm_display_helper] drm_dp_get_one_sb_msg+0x3ce/0x5f0 [drm_display_helper] drm_dp_mst_hpd_irq_handle_event+0xc8/0x1580 [drm_display_helper] Cc: <stable@vger.kernel.org> Cc: Lyude Paul <lyude@redhat.com> Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 +++ 1 file changed, 3 insertions(+)