drm/radeon: evergreen_hpd_init()/_fini(): fix HPD IRQ bitset
Subsystems (2)
PATCH 1/1 drm/radeon: evergreen_hpd_init()/_fini(): fix HPD IRQ bitset
The values of all but the RADEON_HPD_NONE members of the radeon_hpd_id enum transform 1:1 into bit positions within the 'enabled' bitset as assembled by evergreen_hpd_init():
enabled |= 1 << radeon_connector->hpd.hpd;
However, if ->hpd.hpd happens to equal RADEON_HPD_NONE == 0xff, UBSAN reports
UBSAN: Undefined behaviour in drivers/gpu/drm/radeon/evergreen.c:1867:16 shift exponent 255 is too large for 32-bit type 'int' [...] Call Trace: [<ffffffff818c4d35>] dump_stack+0xbc/0x117 [<ffffffff818c4c79>] ? _atomic_dec_and_lock+0x169/0x169 [<ffffffff819411bb>] ubsan_epilogue+0xd/0x4e [<ffffffff81941cbc>] __ubsan_handle_shift_out_of_bounds+0x1fb/0x254 [<ffffffffa0ba7f2e>] ? atom_execute_table+0x3e/0x50 [radeon] [<ffffffff81941ac1>] ? __ubsan_handle_load_invalid_value+0x158/0x158 [<ffffffffa0b87700>] ? radeon_get_pll_use_mask+0x130/0x130 [radeon] [<ffffffff81219930>] ? wake_up_klogd_work_func+0x60/0x60 [<ffffffff8121a35e>] ? vprintk_default+0x3e/0x60 [<ffffffffa0c603c4>] evergreen_hpd_init+0x274/0x2d0 [radeon] [<ffffffffa0c603c4>] ? evergreen_hpd_init+0x274/0x2d0 [radeon] [<ffffffffa0bd196e>] radeon_modeset_init+0x8ce/0x18d0 [radeon] [<ffffffffa0b71d86>] radeon_driver_load_kms+0x186/0x350 [radeon] [<ffffffffa03b6b16>] drm_dev_register+0xc6/0x100 [drm] [<ffffffffa03bc8c4>] drm_get_pci_dev+0xe4/0x490 [drm] [<ffffffff814b83f0>] ? kfree+0x220/0x370 [<ffffffffa0b687c2>] radeon_pci_probe+0x112/0x140 [radeon] [...] ===================================================================== radeon 0000:01:00.0: No connectors reported connected with modes
The net effect is that radeon_irq_kms_enable_hpd() enables the HPD interrupts for all HPD pins in the range from 0 to RADEON_MAX_HPD_PINS. The system seems to work without any noticeable glitches though.
All of the above applies analogously to evergreen_hpd_fini().
Silence UBSAN by checking ->hpd.hpd for RADEON_HPD_NONE before oring it into the 'enabled' bitset in evergreen_hpd_init() or the 'disabled' bitset in evergreen_hpd_fini() respectively.
---
Applicable to linux-next-20160318.
drivers/gpu/drm/radeon/evergreen.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/gpu/drm/radeon/evergreen.c +4 -2
@@ -1864,7 +1864,8 @@ void evergreen_hpd_init(struct radeon_device *rdev)break ; }radeon_hpd_set_polarity (rdev, radeon_connector->hpd.hpd); - enabled |=1 << radeon_connector->hpd.hpd; +if (radeon_connector->hpd.hpd !=RADEON_HPD_NONE ) + enabled |=1 << radeon_connector->hpd.hpd; }radeon_irq_kms_enable_hpd (rdev, enabled); }
@@ -1907,7 +1908,8 @@ void evergreen_hpd_fini(struct radeon_device *rdev)default :break ; } - disabled |=1 << radeon_connector->hpd.hpd; +if (radeon_connector->hpd.hpd !=RADEON_HPD_NONE ) + disabled |=1 << radeon_connector->hpd.hpd; }radeon_irq_kms_disable_hpd (rdev, disabled); }
Signature · 1 line
2.7.3
Quoted 34 lines · Nicolai Stange
The values of all but the
RADEON_HPD_NONEmembers of theradeon_hpd_idenum transform 1:1 into bit positions within the 'enabled' bitset as assembled byevergreen_hpd_init():enabled |= 1 << radeon_connector->hpd.hpd;However, if ->hpd.hpd happens to equal
RADEON_HPD_NONE==0xff, UBSAN reportsUBSAN: Undefined behaviour in drivers/gpu/drm/radeon/evergreen.c:1867:16 shift exponent 255 is too large for 32-bit type 'int' [...] Call Trace: [<ffffffff818c4d35>] dump_stack+0xbc/0x117 [<ffffffff818c4c79>] ? _atomic_dec_and_lock+0x169/0x169 [<ffffffff819411bb>] ubsan_epilogue+0xd/0x4e [<ffffffff81941cbc>] __ubsan_handle_shift_out_of_bounds+0x1fb/0x254 [<ffffffffa0ba7f2e>] ? atom_execute_table+0x3e/0x50 [radeon] [<ffffffff81941ac1>] ? __ubsan_handle_load_invalid_value+0x158/0x158 [<ffffffffa0b87700>] ? radeon_get_pll_use_mask+0x130/0x130 [radeon] [<ffffffff81219930>] ? wake_up_klogd_work_func+0x60/0x60 [<ffffffff8121a35e>] ? vprintk_default+0x3e/0x60 [<ffffffffa0c603c4>] evergreen_hpd_init+0x274/0x2d0 [radeon] [<ffffffffa0c603c4>] ? evergreen_hpd_init+0x274/0x2d0 [radeon] [<ffffffffa0bd196e>] radeon_modeset_init+0x8ce/0x18d0 [radeon] [<ffffffffa0b71d86>] radeon_driver_load_kms+0x186/0x350 [radeon] [<ffffffffa03b6b16>] drm_dev_register+0xc6/0x100 [drm] [<ffffffffa03bc8c4>] drm_get_pci_dev+0xe4/0x490 [drm] [<ffffffff814b83f0>] ? kfree+0x220/0x370 [<ffffffffa0b687c2>] radeon_pci_probe+0x112/0x140 [radeon] [...] ===================================================================== radeon 0000:01:00.0: No connectors reported connected with modes
Quoted 2 lines
The net effect is that
radeon_irq_kms_enable_hpd()enables the HPD interrupts for all HPD pins in the range from 0 toRADEON_MAX_HPD_PINS.
And this is obviously wrong: I mixed the shift count 0xff with the final value when reasoning about the implications :(
1 << 0xff == 1 << 0x1f == 2^31 on x86 at least. Thus the net effect is nothing.
I will resend later this day with an updated description.
Quoted 39 lines
The system seems to work without any noticeable glitches though.
All of the above applies analogously to
evergreen_hpd_fini().Silence UBSAN by checking ->hpd.hpd for
RADEON_HPD_NONEbefore oring it into the 'enabled' bitset inevergreen_hpd_init()or the 'disabled' bitset inevergreen_hpd_fini()respectively.Signed-off-by Nicolai Stange <[email protected]>---
Applicable to linux-next-20160318.drivers/gpu/drm/radeon/evergreen.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)drivers/gpu/drm/radeon/evergreen.c +4
-2@@ -1864,7 +1864,8 @@ void evergreen_hpd_init(struct radeon_device *rdev)break ; }radeon_hpd_set_polarity (rdev, radeon_connector->hpd.hpd); - enabled |=1 << radeon_connector->hpd.hpd; +if (radeon_connector->hpd.hpd !=RADEON_HPD_NONE ) + enabled |=1 << radeon_connector->hpd.hpd; }radeon_irq_kms_enable_hpd (rdev, enabled); }@@ -1907,7 +1908,8 @@ void evergreen_hpd_fini(struct radeon_device *rdev)default :break ; } - disabled |=1 << radeon_connector->hpd.hpd; +if (radeon_connector->hpd.hpd !=RADEON_HPD_NONE ) + disabled |=1 << radeon_connector->hpd.hpd; }radeon_irq_kms_disable_hpd (rdev, disabled); }