drm/crtc: fix connector reference counting mismatch in drm_crtc_helper_set_config

Since commit 0955c1250e96 ("drm/crtc: take references to connectors used
in a modeset. (v2)"), the reference counts of all connectors in the
drm_mode_set given to drm_crtc_helper_set_config are incremented, and then
the reference counts of all connectors are decremented on success, but in a
temporary copy of the connector structure. This leads to the following
error after the first modeset on imx-drm:

    Unable to handle kernel NULL pointer dereference at virtual address 00000004
    pgd = ad8c4000
    [00000004] *pgd=3d9c5831, *pte=00000000, *ppte=00000000
    Internal error: Oops: 817 [#1] PREEMPT SMP ARM
    Modules linked in:
    CPU: 1 PID: 190 Comm: kmsfb-manage Not tainted 4.7.0-rc1+ #657
    Hardware name: Freescale i.MX6 Quad/DualLit: [<80506098>]    lr : [<80252e94>]    psr: 200c0013
    sp : adca7ca8  ip : adca7b90  fp : adca7cd4
    r10: 00000000  r9 : 00000100  r8 : 00000200
    r7 : af3c9800  r6 : aded7848  r5 : aded7800  r4 : 00000000
    r3 : af3ca058  r2 : 00000200  r1 : af3ca058  r0 : 00000000
    Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
    Control: 10c5387d  Table: 3d8c404a  DAC: 00000051
    Process kmsfb-manage (pid: 190, stack limit = 0xadca6210)
    Stack: (0xadca7ca8 to 0xadca8000)
    7ca0:                   805190e0 aded7800 aded7820 80501a88 8155a290 af3c9c6c
    7cc0: adca7ddc 0000000f adca7cec adca7cd8 80519104 80506044 805190e0 aded7800
    7ce0: adca7d04 adca7cf0 80501ac0 805190ec aded7820 aded7814 adca7d24 adca7d08
    7d00: 804fdb80 80501a94 aded7800 af3ca010 aded7afc af3c9c60 adca7d94 adca7d28
    7d20: 804e3518 804fdb20 00000000 af3c9b1c adca7d50 81506f44 00000000 8093c500
    7d40: af3c9c6c ae4f2ca8 ae4f2c18 00000000 00000000 ae637f00 00000000 aded7800
    7d60: 00000001 af3c9800 af23c300 ae77fcc0 ae4f2c18 00000001 af3c9800 8155a290
    7d80: af1af700 adca6000 adca7db4 adca7d98 804fea6c 804e2de4 adca7e50 adb3d940
    7da0: 00000001 af3c9800 adca7e24 adca7db8 8050440c 804fea0c ae77fcc0 00000003
    7dc0: adca7e24 adb3d940 af1af700 ae77fcc0 ae77fccc ae4f2c18 8083d44c ae77fcc0
    7de0: ae4002 80d03040 adca7e64 adca7e40 adca7e50 80503f08
    7e40: 7ebd5630 adca7e50 00000068 c06864a2 7ebd5be8 00000000 00000001 00000018
    7e60: 00000026 00000000 00000000 00000000 00000001 000115bc 05010500 05a0059f
    7e80: 03200000 03360321 00000337 0000003c 00000000 00000040 30383231 30303878
    7ea0: 00000000 00000000 00000000 00000000 00000000 00000000 80173058 80172e30
    7ec0: 80d77d32 00004000 adf7d900 00000003 00000000 7ebd5630 af342bb0 adfe3b80
    7ee0: 80272f50 00000003 adca6000 00000000 adca7f7c adca7f00 802725ec 804f52cc
    7f00: 802809cc 80178450 00000000 00000000 80280880 80145904 adb3d8c0 adf7d990
    7f20: ffffffff 00000003 00004000 01614c10 c06864a2 00000003 adca6000 00000000
    7f40: adca7f6c adca7f50 80280b04 8028088c 000115bc adfe3b81 7ebd5630 adfe3b80
    7f60: c06864a2 00000003 adca6000 00000000 adca7fa4 adca7f80 80272f50 80272548
    7f80: 000115bc 00017050 00000001 01614c10 00000036 801089e4 00000000 adca7fa8
    7fa0: 80108840 80272f18 00017050 00000001 00000003 c06864a2 7ebd5630 000115bc
    7fc0: 00017050 00000001 01614c10 00000036 00000003 00000000 00000026 00000018
    7fe0: 00016f38 7ebd562c 0000b5e9 76ef31e6 400c0030 00000003 ff5f37db bfe7dd4d
    Backtrace:
    [<80506038>] (drm_connector_cleanup) from [<80519104>] (dw_hdmi_connector_destroy+0x24/0x28)
     r10:0000000f r9:adca7ddc r8:af3c9c6c r7:8155a290 r6:80501a88 r5:aded7820
     r4:aded7800 r3:805190e0
    [<805190e0>] (dw_hdmi_connector_destroy) from [<80501ac0>] (drm_connector_free+0x38/0x3c)
     r4:aded7800 nreference) from [<804e3518>] (drm_crtc_helper_set_config+0x740/0xbf4)
     r6:af3c9c60 r5:aded7afc r4:af3ca010 r3:aded7800
    [<804e2dd8>] (drm_crtc_helper_set_config) from [<804fea6c>] (drm_mode_set_config_internal+0x6c/0xf4)
     r10:adca6000 r9:af1af700 r8:8155a290 r7:af3c9800 r6:00000001 r5:ae4f2c18
     r4:ae77fcc0
    [<804fea00>] (drm_mode_set_config_internal) from [<8050440c>] (drm_mode_setcrtc+0x504/0x57c)
     r7:af3c9800 r6:00000001 r5:adb3d940 r4:adca7e50
    [<80503f08>] (drm_mode_setcrtc) from [<804f5404>] (drm_ioctl+0x144/0x4dc)
     r10:ada2e000 r9:000000a2 r8:af3c9800 r7:8155a290 r6:809320b4 r5:00000051
     r4:adca7e50
    [<804f52c0>] (drm_ioctl) from [<802725ec>] (do_vfs_ioctl+0xb0/0x9d0)
     r10:00000000 r9:adca6000 r8:00000003 r7:80272f50 r6:adfe3b80 r5:af342bb0
     r4:7ebd5630
    [<8027253c>] (do_vfs_ioctl) from [<80272f50>] (SyS_ioctl+0x44/0x6c)
     r10:00000000 r9:adca6000 r8:00000003 r7:c06864a2 r6:adfe3b80 r5:7ebd5630
     r4:adfe3b81
    [<80272f0c>] (SyS_ioctl) from [<80108840>] (ret_fast_syscall+0x0/0x1c)
     r8:801089e4 r7:00000036 r6:01614c10 r5:00000001 r4:00017050 r3:000115bc
    Code: 0a00000c e5932004 e1a01003 e1a0a004 (e5842004)
    ---[ end trace 9a7257572ccacb16 ]---

Only the reference count of connectors that weren't previously bound to
an encoder should be incremented after a call to drm_crtc_helper_set_config.
And only the reference count of connectors that were previously bound to
an encoder and are unbound afterwards should ever be decremented.
The reference counts of the temporary copies in the save_connectors
should not be touched at all.

This patch fixes the above error by only incrementing the reference count
of those connectors in the set that are initially not bound to any encoder,
and also by restoring the reference count of only those connectors in the
set in the failure case.

"Note that this can only be hit when fbdev emulation is disabled, since
then the refcount drops from 1 to 0 and we call the connector destroy
functions on the backup copy, which eventually results in tears. With
fbdev emulation the refcount only goes down from 2 to 1 ever. And since we
unconditionally increment the refcount on the real object, the refcount of
that will slowly increase. The backup connector's refcount doesn't matter,
since we kfree() that either way in the end of
drm_crtc_helper_set_config()."

Fixes: 0955c1250e96 ("drm/crtc: take references to connectors used in a modeset. (v2)")
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Dave Airlie <airlied@redhat.com>

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index a6e42433ef0e0d5cde2d87739fb778e59336b705..1c4d6746f90ca620f174baa5695fd37e8624a603 100644 (file)
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -631,8 +631,12 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
                mode_changed = true;
        }
 
-       /* take a reference on all connectors in set */
+       /* take a reference on all unbound connectors in set, reuse the
+        * already taken reference for bound connectors
+        */
        for (ro = 0; ro < set->num_connectors; ro++) {
+               if (set->connectors[ro]->encoder)
+                       continue;
                drm_connector_reference(set->connectors[ro]);
        }
 
@@ -754,12 +758,6 @@ int drm_crtc_helper_set_config(struct drm_mode_set *set)
                }
        }
 
-       /* after fail drop reference on all connectors in save set */
-       count = 0;
-       drm_for_each_connector(connector, dev) {
-               drm_connector_unreference(&save_connectors[count++]);
-       }
-
        kfree(save_connectors);
        kfree(save_encoders);
        return 0;
@@ -776,8 +774,12 @@ fail:
                *connector = save_connectors[count++];
        }
 
-       /* after fail drop reference on all connectors in set */
+       /* after fail drop reference on all unbound connectors in set, let
+        * bound connectors keep their reference
+        */
        for (ro = 0; ro < set->num_connectors; ro++) {
+               if (set->connectors[ro]->encoder)
+                       continue;
                drm_connector_unreference(set->connectors[ro]);
        }