target/linux/bcm27xx/patches-6.1/950-1235-drm-vc4-don-t-check-if-plane-state-fb-state-fb.patch

   1 From 146bbf9627f6c37816939de29538ec8ee9a7be1a Mon Sep 17 00:00:00 2001
   2 From: =?UTF-8?q?Ma=C3=ADra=20Canal?= <mcanal@igalia.com>
   3 Date: Fri, 5 Jan 2024 15:07:34 -0300
   4 Subject: [PATCH] drm/vc4: don't check if plane->state->fb == state->fb
   5 MIME-Version: 1.0
   6 Content-Type: text/plain; charset=UTF-8
   7 Content-Transfer-Encoding: 8bit
   8
   9 Currently, when using non-blocking commits, we can see the following
  10 kernel warning:
  11
  12 [  110.908514] ------------[ cut here ]------------
  13 [  110.908529] refcount_t: underflow; use-after-free.
  14 [  110.908620] WARNING: CPU: 0 PID: 1866 at lib/refcount.c:87 refcount_dec_not_one+0xb8/0xc0
  15 [  110.908664] Modules linked in: rfcomm snd_seq_dummy snd_hrtimer snd_seq snd_seq_device cmac algif_hash aes_arm64 aes_generic algif_skcipher af_alg bnep hid_logitech_hidpp vc4 brcmfmac hci_uart btbcm brcmutil bluetooth snd_soc_hdmi_codec cfg80211 cec drm_display_helper drm_dma_helper drm_kms_helper snd_soc_core snd_compress snd_pcm_dmaengine fb_sys_fops sysimgblt syscopyarea sysfillrect raspberrypi_hwmon ecdh_generic ecc rfkill libaes i2c_bcm2835 binfmt_misc joydev snd_bcm2835(C) bcm2835_codec(C) bcm2835_isp(C) v4l2_mem2mem videobuf2_dma_contig snd_pcm bcm2835_v4l2(C) raspberrypi_gpiomem bcm2835_mmal_vchiq(C) videobuf2_v4l2 snd_timer videobuf2_vmalloc videobuf2_memops videobuf2_common snd videodev vc_sm_cma(C) mc hid_logitech_dj uio_pdrv_genirq uio i2c_dev drm fuse dm_mod drm_panel_orientation_quirks backlight ip_tables x_tables ipv6
  16 [  110.909086] CPU: 0 PID: 1866 Comm: kodi.bin Tainted: G         C         6.1.66-v8+ #32
  17 [  110.909104] Hardware name: Raspberry Pi 3 Model B Rev 1.2 (DT)
  18 [  110.909114] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  19 [  110.909132] pc : refcount_dec_not_one+0xb8/0xc0
  20 [  110.909152] lr : refcount_dec_not_one+0xb4/0xc0
  21 [  110.909170] sp : ffffffc00913b9c0
  22 [  110.909177] x29: ffffffc00913b9c0 x28: 000000556969bbb0 x27: 000000556990df60
  23 [  110.909205] x26: 0000000000000002 x25: 0000000000000004 x24: ffffff8004448480
  24 [  110.909230] x23: ffffff800570b500 x22: ffffff802e03a7bc x21: ffffffecfca68c78
  25 [  110.909257] x20: ffffff8002b42000 x19: ffffff802e03a600 x18: 0000000000000000
  26 [  110.909283] x17: 0000000000000011 x16: ffffffffffffffff x15: 0000000000000004
  27 [  110.909308] x14: 0000000000000fff x13: ffffffed577e47e0 x12: 0000000000000003
  28 [  110.909333] x11: 0000000000000000 x10: 0000000000000027 x9 : c912d0d083728c00
  29 [  110.909359] x8 : c912d0d083728c00 x7 : 65646e75203a745f x6 : 746e756f63666572
  30 [  110.909384] x5 : ffffffed579f62ee x4 : ffffffed579eb01e x3 : 0000000000000000
  31 [  110.909409] x2 : 0000000000000000 x1 : ffffffc00913b750 x0 : 0000000000000001
  32 [  110.909434] Call trace:
  33 [  110.909441]  refcount_dec_not_one+0xb8/0xc0
  34 [  110.909461]  vc4_bo_dec_usecnt+0x4c/0x1b0 [vc4]
  35 [  110.909903]  vc4_cleanup_fb+0x44/0x50 [vc4]
  36 [  110.910315]  drm_atomic_helper_cleanup_planes+0x88/0xa4 [drm_kms_helper]
  37 [  110.910669]  vc4_atomic_commit_tail+0x390/0x9dc [vc4]
  38 [  110.911079]  commit_tail+0xb0/0x164 [drm_kms_helper]
  39 [  110.911397]  drm_atomic_helper_commit+0x1d0/0x1f0 [drm_kms_helper]
  40 [  110.911716]  drm_atomic_commit+0xb0/0xdc [drm]
  41 [  110.912569]  drm_mode_atomic_ioctl+0x348/0x4b8 [drm]
  42 [  110.913330]  drm_ioctl_kernel+0xec/0x15c [drm]
  43 [  110.914091]  drm_ioctl+0x24c/0x3b0 [drm]
  44 [  110.914850]  __arm64_sys_ioctl+0x9c/0xd4
  45 [  110.914873]  invoke_syscall+0x4c/0x114
  46 [  110.914897]  el0_svc_common+0xd0/0x118
  47 [  110.914917]  do_el0_svc+0x38/0xd0
  48 [  110.914936]  el0_svc+0x30/0x8c
  49 [  110.914958]  el0t_64_sync_handler+0x84/0xf0
  50 [  110.914979]  el0t_64_sync+0x18c/0x190
  51 [  110.914996] ---[ end trace 0000000000000000 ]---
  52
  53 This happens because, although `prepare_fb` and `cleanup_fb` are
  54 perfectly balanced, we cannot guarantee consistency in the check
  55 plane->state->fb == state->fb. This means that sometimes we can increase
  56 the refcount in `prepare_fb` and don't decrease it in `cleanup_fb`. The
  57 opposite can also be true.
  58
  59 In fact, the struct drm_plane .state shouldn't be accessed directly
  60 but instead, the `drm_atomic_get_new_plane_state()` helper function should
  61 be used. So, we could stick to this check, but using
  62 `drm_atomic_get_new_plane_state()`. But actually, this check is not really
  63 needed. We can increase and decrease the refcount symmetrically without
  64 problems.
  65
  66 This is going to make the code more simple and consistent.
  67
  68 Signed-off-by: Maíra Canal <mcanal@igalia.com>
  69 ---
  70  drivers/gpu/drm/vc4/vc4_plane.c | 5 +----
  71  1 file changed, 1 insertion(+), 4 deletions(-)
  72
  73 --- a/drivers/gpu/drm/vc4/vc4_plane.c
  74 +++ b/drivers/gpu/drm/vc4/vc4_plane.c
  75 @@ -2225,9 +2225,6 @@ static int vc4_prepare_fb(struct drm_pla
  76
  77         drm_gem_plane_helper_prepare_fb(plane, state);
  78
  79 -       if (plane->state->fb == state->fb)
  80 -               return 0;
  81 -
  82         return vc4_bo_inc_usecnt(bo);
  83  }
  84
  85 @@ -2236,7 +2233,7 @@ static void vc4_cleanup_fb(struct drm_pl
  86  {
  87         struct vc4_bo *bo;
  88
  89 -       if (plane->state->fb == state->fb || !state->fb)
  90 +       if (!state->fb)
  91                 return;
  92
  93         bo = to_vc4_bo(&drm_fb_dma_get_gem_obj(state->fb, 0)->base);