Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753322AbcCVVFj (ORCPT ); Tue, 22 Mar 2016 17:05:39 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:33192 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751964AbcCVVFg (ORCPT ); Tue, 22 Mar 2016 17:05:36 -0400 From: Nicolai Stange To: David Airlie , Alex Deucher , =?UTF-8?q?Christian=20K=C3=B6nig?= Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Nicolai Stange Subject: [PATCH v3] drm/radeon: don't include RADEON_HPD_NONE in HPD IRQ enable bitsets Date: Tue, 22 Mar 2016 22:05:27 +0100 Message-Id: <1458680727-19795-1-git-send-email-nicstange@gmail.com> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6526 Lines: 171 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: [] dump_stack+0xbc/0x117 [] ? _atomic_dec_and_lock+0x169/0x169 [] ubsan_epilogue+0xd/0x4e [] __ubsan_handle_shift_out_of_bounds+0x1fb/0x254 [] ? atom_execute_table+0x3e/0x50 [radeon] [] ? __ubsan_handle_load_invalid_value+0x158/0x158 [] ? radeon_get_pll_use_mask+0x130/0x130 [radeon] [] ? wake_up_klogd_work_func+0x60/0x60 [] ? vprintk_default+0x3e/0x60 [] evergreen_hpd_init+0x274/0x2d0 [radeon] [] ? evergreen_hpd_init+0x274/0x2d0 [radeon] [] radeon_modeset_init+0x8ce/0x18d0 [radeon] [] radeon_driver_load_kms+0x186/0x350 [radeon] [] drm_dev_register+0xc6/0x100 [drm] [] drm_get_pci_dev+0xe4/0x490 [drm] [] ? kfree+0x220/0x370 [] radeon_pci_probe+0x112/0x140 [radeon] [...] ===================================================================== radeon 0000:01:00.0: No connectors reported connected with modes At least on x86, there should be no user-visible impact as there 1 << 0xff == 1 << (0xff & 31) == 1 << 31 holds and 31 > RADEON_MAX_HPD_PINS. Thus, this patch is a cosmetic one. All of the above applies analogously to evergreen_hpd_fini(), r100_hpd_init(), r100_hpd_fini(), r600_hpd_init(), r600_hpd_fini(), rs600_hpd_init() and rs600_hpd_fini() Silence UBSAN by checking ->hpd.hpd for RADEON_HPD_NONE before oring it into the 'enabled' bitset in the *_init()- or the 'disabled' bitset in the *_fini()-functions respectively. Signed-off-by: Nicolai Stange --- Applicable to linux-next-20160322. WARNING: Apart from compilation, the changes to r100.c, r600.c and rs600.c are untested! v2 thread can be found here: http://lkml.kernel.org/g/1458562256-2882-1-git-send-email-nicstange@gmail.com Changes to v2: - Fix the very same issue not only for evergreen, but for r100, r600 and rs600 also. - Change the commit's subject and description body accordingly. Changes to v1: - Turn commit message's impact part into a non-impact part. drivers/gpu/drm/radeon/evergreen.c | 6 ++++-- drivers/gpu/drm/radeon/r100.c | 6 ++++-- drivers/gpu/drm/radeon/r600.c | 6 ++++-- drivers/gpu/drm/radeon/rs600.c | 6 ++++-- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 76c4bdf..6360717 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -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); } diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c index 6e478a2..55687f3 100644 --- a/drivers/gpu/drm/radeon/r100.c +++ b/drivers/gpu/drm/radeon/r100.c @@ -592,7 +592,8 @@ void r100_hpd_init(struct radeon_device *rdev) list_for_each_entry(connector, &dev->mode_config.connector_list, head) { struct radeon_connector *radeon_connector = to_radeon_connector(connector); - enable |= 1 << radeon_connector->hpd.hpd; + if (radeon_connector->hpd.hpd != RADEON_HPD_NONE) + enable |= 1 << radeon_connector->hpd.hpd; radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); } radeon_irq_kms_enable_hpd(rdev, enable); @@ -614,7 +615,8 @@ void r100_hpd_fini(struct radeon_device *rdev) list_for_each_entry(connector, &dev->mode_config.connector_list, head) { struct radeon_connector *radeon_connector = to_radeon_connector(connector); - disable |= 1 << radeon_connector->hpd.hpd; + if (radeon_connector->hpd.hpd != RADEON_HPD_NONE) + disable |= 1 << radeon_connector->hpd.hpd; } radeon_irq_kms_disable_hpd(rdev, disable); } diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index f86ab69..f4012ae 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -1002,7 +1002,8 @@ void r600_hpd_init(struct radeon_device *rdev) break; } } - enable |= 1 << radeon_connector->hpd.hpd; + if (radeon_connector->hpd.hpd != RADEON_HPD_NONE) + enable |= 1 << radeon_connector->hpd.hpd; radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); } radeon_irq_kms_enable_hpd(rdev, enable); @@ -1055,7 +1056,8 @@ void r600_hpd_fini(struct radeon_device *rdev) break; } } - disable |= 1 << radeon_connector->hpd.hpd; + if (radeon_connector->hpd.hpd != RADEON_HPD_NONE) + disable |= 1 << radeon_connector->hpd.hpd; } radeon_irq_kms_disable_hpd(rdev, disable); } diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c index 6244f4e..4b35213 100644 --- a/drivers/gpu/drm/radeon/rs600.c +++ b/drivers/gpu/drm/radeon/rs600.c @@ -413,7 +413,8 @@ void rs600_hpd_init(struct radeon_device *rdev) default: break; } - enable |= 1 << radeon_connector->hpd.hpd; + if (radeon_connector->hpd.hpd != RADEON_HPD_NONE) + enable |= 1 << radeon_connector->hpd.hpd; radeon_hpd_set_polarity(rdev, radeon_connector->hpd.hpd); } radeon_irq_kms_enable_hpd(rdev, enable); @@ -439,7 +440,8 @@ void rs600_hpd_fini(struct radeon_device *rdev) default: break; } - disable |= 1 << radeon_connector->hpd.hpd; + if (radeon_connector->hpd.hpd != RADEON_HPD_NONE) + disable |= 1 << radeon_connector->hpd.hpd; } radeon_irq_kms_disable_hpd(rdev, disable); } -- 2.7.4