| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 8 Sep 2016 09:14:23 +0200
From: Christian König <christian.koenig@....com>
To: Mark Fortescue <mark@...rning-instruments.co.uk>,
David Airlie <airlied@...ux.ie>
CC: Alex Deucher <alexander.deucher@....com>,
Christian König <christian.koenig@....com>,
<dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 001/001] drivers/gpu/radeon: NULL pointer deference
workaround
Am 07.09.2016 um 19:38 schrieb Mark Fortescue:
>
> On an LV-683 (AMD Dual-core G-T56N) Mini-ITX board, I get a Kernel
> Oops because Connector 0 (LCD Panel interface) does not have DDC.
I'm not an expert on this, but that is really odd cause even LCD Panels
should have a DDC interface.
>
> Ubuntu 16.04 LTS Kernel (4.4 series):
>
> ...
> [ 8.262990] [drm] ib test on ring 5 succeeded
> [ 8.288897] [drm] Radeon Display Connectors
> [ 8.293175] [drm] Connector 0:
> [ 8.296252] [drm] DP-1
Especially since the BIOS claims that this is a displayport connector
and there is no physical way to have a DP without an DDC as far as I know.
Please open a bug report on FDO and attach you BIOS image.
Alex can probably take a look when he's back from vacation.
Regards,
Christian.
> [ 8.298791] [drm] Encoders:
> [ 8.301770] [drm] DFP1: INTERNAL_UNIPHY
> [ 8.305973] [drm] Connector 1:
> [ 8.309043] [drm] DP-2
> [ 8.311598] [drm] HPD2
> [ 8.314169] [drm] DDC: 0x6440 0x6440 0x6444 0x6444 0x6448 0x6448
> 0x644c 0x644c
> [ 8.321609] [drm] Encoders:
> [ 8.324589] [drm] DFP2: INTERNAL_UNIPHY
> [ 8.328793] [drm] Connector 2:
> [ 8.331856] [drm] VGA-1
> [ 8.342947] [drm] DDC: 0x64d8 0x64d8 0x64dc 0x64dc 0x64e0 0x64e0
> 0x64e4 0x64e4
> [ 8.350341] [drm] Encoders:
> [ 8.353310] [drm] CRT1: INTERNAL_KLDSCP_DAC1
> [ 8.358195] BUG: unable to handle kernel NULL pointer dereference at
> 0000000000000409
> [ 8.409733] [<ffffffffc02024da>] radeon_dp_getsinktype+0x1a/0x30 [radeon]
> [ 8.416805] PGD 0
> [ 8.418841] Oops: 0000 [#1] SMP
> ...
>
> This patch prevents Kernel failures due to a connector not having a
> DDC interface by changing the code so that ddc_bus is always checked
> before use.
> The problem was first identified using the uBuntu MATE 14.04 LTS (3.16
> series kernels) but not dealt with at that time. On attempting to
> install uBuntu MATE 16.04 LTS (4.4 series kernels), it became clear
> that using various workarounds to allow the issue to be ignored were
> not viable so more effort was put in to sorting the issue resulting in
> this patch. See https://bugs.launchpad.net/bugs/1587885 for more details.
>
> Signed-off-by: Mark Fortescue <mark@...rning-instruments.co.uk>
> Tested-by: Mark Fortescue <mark@...rning-instruments.co.uk>
>
> ---
>
> Looks like Thunderbird may have made a mess of the patch when pasting
> the contents into the mail message - my alternate mail client (pine)
> also has strict line length handling and trashes non-MIME encoded
> patches.
>
> This may not be the correct approach to solving the issue but it is
> clean in that it ensures that ddc_bus is never used when NULL
> regardless of how the code ended up at the point of use.
>
> If it helps with back porting, I have patches for the uBuntu 14.04 LTS
> [3.13 series], uBuntu MATE 14.04 LTS [3.16 series] and uBuntu 16.04
> LTS [4.4 series] kernels.
>
> Test Hardware:
> Commell LV-683 Mini-ITX with onboard AMD Dual-core G-T56N
> 4G Ram, 2x1TB Disk, HANNS-G HC194D 1280x1024 LCD (VGA).
> 4.8.0-rc5 with patch boots without error.
>
> drivers/gpu/drm/radeon/atombios_dp.c | 60 ++++++++++++-------
> drivers/gpu/drm/radeon/radeon_connectors.c | 46 +++++++-------
> drivers/gpu/drm/radeon/radeon_dp_mst.c | 9 ++
> drivers/gpu/drm/radeon/radeon_i2c.c | 3
> 4 files changed, 73 insertions(+), 45 deletions(-)
>
> Patch:
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c
> b/drivers/gpu/drm/radeon/atombios_dp.c
> index cead089a..98b3c0e 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -232,6 +232,9 @@ void radeon_dp_aux_init(struct radeon_connector
> *radeon_connector)
> struct radeon_device *rdev = dev->dev_private;
> int ret;
>
> + if (!radeon_connector->ddc_bus)
> + return;
> +
> radeon_connector->ddc_bus->rec.hpd = radeon_connector->hpd.hpd;
> radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev;
> if (ASIC_IS_DCE5(rdev)) {
> @@ -364,6 +367,9 @@ u8 radeon_dp_getsinktype(struct radeon_connector
> *radeon_connector)
> struct drm_device *dev = radeon_connector->base.dev;
> struct radeon_device *rdev = dev->dev_private;
>
> + if (!radeon_connector->ddc_bus)
> + return 0;
> +
> return radeon_dp_encoder_service(rdev,
> ATOM_DP_ACTION_GET_SINK_TYPE, 0,
> radeon_connector->ddc_bus->rec.i2c_id, 0);
> }
> @@ -376,6 +382,9 @@ static void radeon_dp_probe_oui(struct
> radeon_connector *radeon_connector)
> if (!(dig_connector->dpcd[DP_DOWN_STREAM_PORT_COUNT] &
> DP_OUI_SUPPORT))
> return;
>
> + if (!radeon_connector->ddc_bus)
> + return;
> +
> if (drm_dp_dpcd_read(&radeon_connector->ddc_bus->aux,
> DP_SINK_OUI, buf, 3) == 3)
> DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
> buf[0], buf[1], buf[2]);
> @@ -391,6 +400,9 @@ bool radeon_dp_getdpcd(struct radeon_connector
> *radeon_connector)
> u8 msg[DP_DPCD_SIZE];
> int ret, i;
>
> + if (!radeon_connector->ddc_bus)
> + return false;
> +
> for (i = 0; i < 7; i++) {
> ret = drm_dp_dpcd_read(&radeon_connector->ddc_bus->aux,
> DP_DPCD_REV, msg,
> DP_DPCD_SIZE);
> @@ -428,24 +440,26 @@ int radeon_dp_get_panel_mode(struct drm_encoder
> *encoder,
>
> dig_connector = radeon_connector->con_priv;
>
> - if (dp_bridge != ENCODER_OBJECT_ID_NONE) {
> - /* DP bridge chips */
> - if (drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux,
> - DP_EDP_CONFIGURATION_CAP, &tmp) == 1) {
> - if (tmp & 1)
> - panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
> - else if ((dp_bridge == ENCODER_OBJECT_ID_NUTMEG) ||
> - (dp_bridge == ENCODER_OBJECT_ID_TRAVIS))
> - panel_mode = DP_PANEL_MODE_INTERNAL_DP1_MODE;
> - else
> - panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
> - }
> - } else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> - /* eDP */
> - if (drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux,
> - DP_EDP_CONFIGURATION_CAP, &tmp) == 1) {
> - if (tmp & 1)
> - panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
> + if (radeon_connector->ddc_bus) {
> + if (dp_bridge != ENCODER_OBJECT_ID_NONE) {
> + /* DP bridge chips */
> + if (drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux,
> + DP_EDP_CONFIGURATION_CAP, &tmp) == 1) {
> + if (tmp & 1)
> + panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
> + else if ((dp_bridge == ENCODER_OBJECT_ID_NUTMEG) ||
> + (dp_bridge == ENCODER_OBJECT_ID_TRAVIS))
> + panel_mode = DP_PANEL_MODE_INTERNAL_DP1_MODE;
> + else
> + panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE;
> + }
> + } else if (connector->connector_type ==
> DRM_MODE_CONNECTOR_eDP) {
> + /* eDP */
> + if (drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux,
> + DP_EDP_CONFIGURATION_CAP, &tmp) == 1) {
> + if (tmp & 1)
> + panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE;
> + }
> }
> }
>
> @@ -511,6 +525,9 @@ bool radeon_dp_needs_link_train(struct
> radeon_connector *radeon_connector)
> u8 link_status[DP_LINK_STATUS_SIZE];
> struct radeon_connector_atom_dig *dig = radeon_connector->con_priv;
>
> + if (!radeon_connector->ddc_bus)
> + return false;
> +
> if
> (drm_dp_dpcd_read_link_status(&radeon_connector->ddc_bus->aux,
> link_status)
> <= 0)
> return false;
> @@ -531,7 +548,7 @@ void radeon_dp_set_rx_power_state(struct
> drm_connector *connector,
> dig_connector = radeon_connector->con_priv;
>
> /* power up/down the sink */
> - if (dig_connector->dpcd[0] >= 0x11) {
> + if (radeon_connector->ddc_bus && dig_connector->dpcd[0] >= 0x11) {
> drm_dp_dpcd_writeb(&radeon_connector->ddc_bus->aux,
> DP_SET_POWER, power_state);
> usleep_range(1000, 2000);
> @@ -834,7 +851,8 @@ void radeon_dp_link_train(struct drm_encoder
> *encoder,
> else
> dp_info.enc_id |= ATOM_DP_CONFIG_LINK_A;
>
> - if (drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux,
> DP_MAX_LANE_COUNT, &tmp)
> + if (radeon_connector->ddc_bus &&
> + drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux,
> DP_MAX_LANE_COUNT, &tmp)
> == 1) {
> if (ASIC_IS_DCE5(rdev) && (tmp & DP_TPS3_SUPPORTED))
> dp_info.tp3_supported = true;
> @@ -850,7 +868,7 @@ void radeon_dp_link_train(struct drm_encoder
> *encoder,
> dp_info.connector = connector;
> dp_info.dp_lane_count = dig_connector->dp_lane_count;
> dp_info.dp_clock = dig_connector->dp_clock;
> - dp_info.aux = &radeon_connector->ddc_bus->aux;
> + dp_info.aux = radeon_connector->ddc_bus ?
> &radeon_connector->ddc_bus->aux : 0;
>
> if (radeon_dp_link_train_init(&dp_info))
> goto done;
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
> b/drivers/gpu/drm/radeon/radeon_connectors.c
> index b79f3b0..cec30c9 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -328,31 +328,32 @@ static void radeon_connector_get_edid(struct
> drm_connector *connector)
> if (radeon_connector->router.ddc_valid)
> radeon_router_select_ddc_port(radeon_connector);
>
> - if ((radeon_connector_encoder_get_dp_bridge_encoder_id(connector) !=
> - ENCODER_OBJECT_ID_NONE) &&
> - radeon_connector->ddc_bus->has_aux) {
> - radeon_connector->edid = drm_get_edid(connector,
> - &radeon_connector->ddc_bus->aux.ddc);
> - } else if ((connector->connector_type ==
> DRM_MODE_CONNECTOR_DisplayPort) ||
> - (connector->connector_type == DRM_MODE_CONNECTOR_eDP)) {
> - struct radeon_connector_atom_dig *dig =
> radeon_connector->con_priv;
> -
> - if ((dig->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT ||
> - dig->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) &&
> - radeon_connector->ddc_bus->has_aux)
> - radeon_connector->edid =
> drm_get_edid(&radeon_connector->base,
> + if (radeon_connector->ddc_bus) {
> + if
> ((radeon_connector_encoder_get_dp_bridge_encoder_id(connector) !=
> + ENCODER_OBJECT_ID_NONE) &&
> + radeon_connector->ddc_bus->has_aux) {
> + radeon_connector->edid = drm_get_edid(connector,
> &radeon_connector->ddc_bus->aux.ddc);
> - else if (radeon_connector->ddc_bus)
> + } else if ((connector->connector_type ==
> DRM_MODE_CONNECTOR_DisplayPort) ||
> + (connector->connector_type == DRM_MODE_CONNECTOR_eDP)) {
> + struct radeon_connector_atom_dig *dig =
> radeon_connector->con_priv;
> +
> + if ((dig->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT ||
> + dig->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) &&
> + radeon_connector->ddc_bus->has_aux)
> + radeon_connector->edid =
> drm_get_edid(&radeon_connector->base,
> + &radeon_connector->ddc_bus->aux.ddc);
> + else
> + radeon_connector->edid =
> drm_get_edid(&radeon_connector->base,
> + &radeon_connector->ddc_bus->adapter);
> + } else if (vga_switcheroo_handler_flags() &
> VGA_SWITCHEROO_CAN_SWITCH_DDC &&
> + connector->connector_type == DRM_MODE_CONNECTOR_LVDS) {
> + radeon_connector->edid =
> drm_get_edid_switcheroo(&radeon_connector->base,
> + &radeon_connector->ddc_bus->adapter);
> + } else {
> radeon_connector->edid =
> drm_get_edid(&radeon_connector->base,
> &radeon_connector->ddc_bus->adapter);
> - } else if (vga_switcheroo_handler_flags() &
> VGA_SWITCHEROO_CAN_SWITCH_DDC &&
> - connector->connector_type == DRM_MODE_CONNECTOR_LVDS &&
> - radeon_connector->ddc_bus) {
> - radeon_connector->edid =
> drm_get_edid_switcheroo(&radeon_connector->base,
> - &radeon_connector->ddc_bus->adapter);
> - } else if (radeon_connector->ddc_bus) {
> - radeon_connector->edid = drm_get_edid(&radeon_connector->base,
> - &radeon_connector->ddc_bus->adapter);
> + }
> }
>
> if (!radeon_connector->edid) {
> @@ -1312,6 +1313,7 @@ radeon_dvi_detect(struct drm_connector
> *connector, bool force)
> continue;
> list_radeon_connector =
> to_radeon_connector(list_connector);
> if (list_radeon_connector->shared_ddc &&
> + radeon_connector->ddc_bus &&
> (list_radeon_connector->ddc_bus->rec.i2c_id ==
> radeon_connector->ddc_bus->rec.i2c_id)) {
> /* cases where both connectors are digital */
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index de504ea..89e91f3 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -661,7 +661,7 @@ radeon_dp_mst_init(struct radeon_connector
> *radeon_connector)
> {
> struct drm_device *dev = radeon_connector->base.dev;
>
> - if (!radeon_connector->ddc_bus->has_aux)
> + if (!radeon_connector->ddc_bus ||
> !radeon_connector->ddc_bus->has_aux)
> return 0;
>
> radeon_connector->mst_mgr.cbs = &mst_cbs;
> @@ -688,6 +688,9 @@ radeon_dp_mst_probe(struct radeon_connector
> *radeon_connector)
> if (dig_connector->dpcd[DP_DPCD_REV] < 0x12)
> return 0;
>
> + if (!radeon_connector->ddc_bus ||
> !radeon_connector->ddc_bus->has_aux)
> + return 0;
> +
> ret = drm_dp_dpcd_read(&radeon_connector->ddc_bus->aux,
> DP_MSTM_CAP, msg,
> 1);
> if (ret) {
> @@ -711,7 +714,9 @@ radeon_dp_mst_check_status(struct radeon_connector
> *radeon_connector)
> struct radeon_connector_atom_dig *dig_connector =
> radeon_connector->con_priv;
> int retry;
>
> - if (dig_connector->is_mst) {
> + if (dig_connector->is_mst &&
> + radeon_connector->ddc_bus &&
> + radeon_connector->ddc_bus->has_aux) {
> u8 esi[16] = { 0 };
> int dret;
> int ret = 0;
> diff --git a/drivers/gpu/drm/radeon/radeon_i2c.c
> b/drivers/gpu/drm/radeon/radeon_i2c.c
> index 9590bcd..c18f7ba 100644
> --- a/drivers/gpu/drm/radeon/radeon_i2c.c
> +++ b/drivers/gpu/drm/radeon/radeon_i2c.c
> @@ -63,6 +63,9 @@ bool radeon_ddc_probe(struct radeon_connector
> *radeon_connector, bool use_aux)
> if (radeon_connector->router.ddc_valid)
> radeon_router_select_ddc_port(radeon_connector);
>
> + if (!radeon_connector->ddc_bus)
> + return false;
> +
> if (use_aux) {
> ret = i2c_transfer(&radeon_connector->ddc_bus->aux.ddc,
> msgs, 2);
> } else {
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Powered by blists - more mailing lists