The patch in raw form so it can be used directly. ** Patch added: "Validate ddc_bus before use" https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1587885/+attachment/4674404/+files/validate_ddc_bus_before_use.patch
-- You received this bug notification because you are a member of Kernel Packages, which is subscribed to linux in Ubuntu. https://bugs.launchpad.net/bugs/1587885 Title: NULL pointer dereference in radeon drm code Status in linux package in Ubuntu: Incomplete Bug description: On one of my systems I use, display connector 0 does not have DDC. This results in the DRM code trying to use radeon_connector->ddc_bus when it is not set. This is known to have been an issue since the 3.2 series kernels. Known Problem Kernels: uBuntu 12.04 LTS (linux-3.2.55), uBuntu-MATE 14.04 LTS (linux-3.16.7) and uBuntu-MATE 16.04 LTS (linux-4.4.8) and linux-4.6.0 (git:/ /git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux). Trying to use the uBuntu-MATE 16.04 LTS LiveCD ubuntu-mate-16.04 -desktop-amd64.iso (from uBuntu-MATE website) showed the problem. Using a serial console I extracted the following information when trying to boot from a uBuntu-MATE 16.04 LTS LiveCD USB Stick (it had been upgraded from 4.4.0-21-generic in an attempt to get something usable): <snipped> [ 0.000000] Linux version 4.4.0-22-generic (buildd@lgw01-41) (gcc version 5.3.1 20160413 (Ubuntu 5.3.1-14ubuntu2) ) #40-Ubuntu SMP Thu May 12 22:03:46 UTC 2 016 (Ubuntu 4.4.0-22.40-generic 4.4.8) [ 0.000000] Command line: BOOT_IMAGE=/casper/vmlinuz.efi file=/cdrom/preseed/ubuntu-mate.seed boot=casper locale=en_GB console-setup/layoutcode=gb console=u art,io,0x3f8,115200n8 earlyprintk=ttyS0,115200n8 debug --- [ 0.000000] KERNEL supported cpus: [ 0.000000] Intel GenuineIntel [ 0.000000] AMD AuthenticAMD [ 0.000000] Centaur CentaurHauls <snipped> [ 0.000000] efi: EFI v2.00 by Phoenix Technologies Ltd. [ 0.000000] efi: EFI v2.00 by Phoenix Technologies Ltd. [ 0.000000] efi: [ 0.000000] efi: ACPI=0x973c7000 ACPI=0x973c7000 ACPI 2.0=0x973c7014 ACPI 2.0=0x973c7014 SMBIOS=0x9726b000 SMBIOS=0x9726b000 [ 0.000000] SMBIOS 2.6 present. [ 0.000000] SMBIOS 2.6 present. [ 0.000000] DMI: AMD Corporation EBrazos Platform/Persimmon, BIOS LV-683 Version: 1.1 02/14/2012 [ 0.000000] DMI: AMD Corporation EBrazos Platform/Persimmon, BIOS LV-683 Version: 1.1 02/14/2012 <snipped> [ 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 [ 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.332316] scsi 0:0:0:0: Direct-Access SMI USB DISK 1100 PQ: 0 ANSI: 0 CCS [ 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.366104] IP:[ 8.367176] sd 0:0:0:0: Attached scsi generic sg1 type 0 [ 8.368538] sd 0:0:0:0: [sdb] 7831552 512-byte logical blocks: (4.01 GB/3.73 GiB) [ 8.369421] sd 0:0:0:0: [sdb] Write Protect is off [ 8.369427] sd 0:0:0:0: [sdb] Mode Sense: 43 00 00 00 [ 8.370288] sd 0:0:0:0: [sdb] No Caching mode page found [ 8.370292] sd 0:0:0:0: [sdb] Assuming drive cache: write through [ 8.374944] sdb: sdb1 [ 8.378398] sd 0:0:0:0: [sdb] Attached SCSI removable disk [ 8.409733] [<ffffffffc02024da>] radeon_dp_getsinktype+0x1a/0x30 [radeon] [ 8.416805] PGD 0 [ 8.418841] Oops: 0000 [#1] SMP [ 8.422104] Modules linked in: hid_generic e1000e psmouse amdkfd usbhid uas amd_iommu_v2 ptp radeon(+) pps_core e1000(+) hid i2c_algo_bit ttm drm_kms_helper usb_storage syscopyarea sysfillrect sysimgblt ahci fb_sys_fops libahci drm video fjes [ 8.444029] CPU: 0 PID: 131 Comm: systemd-udevd Not tainted 4.4.0-22-generic #40-Ubuntu [ 8.452036] Hardware name: AMD Corporation EBrazos Platform/Persimmon, BIOS LV-683 Version: 1.1 02/14/2012 [ 8.461708] task: ffff88014780be80 ti: ffff880147888000 task.ti: ffff880147888000 [ 8.469194] RIP: 0010:[<ffffffffc02024da>] [<ffffffffc02024da>] radeon_dp_getsinktype+0x1a/0x30 [radeon] [ 8.478850] RSP: 0018:ffff88014788b850 EFLAGS: 00010246 [ 8.484179] RAX: 0000000000000000 RBX: ffff88003485f000 RCX: 0000000000000000 [ 8.491321] RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff88003485f000 [ 8.498469] RBP: ffff88014788b850 R08: 0000000000000000 R09: ffffffffc004aff6 [ 8.505602] R10: ffffea0000d5e540 R11: 0000000000000000 R12: 0000000000000001 [ 8.506137] e1000 0000:05:0b.0 eth1: (PCI:33MHz:32-bit) 00:03:1d:09:3b:61 [ 8.506142] e1000 0000:05:0b.0 eth1: Intel(R) PRO/1000 Network Connection [ 8.526299] R13: ffff8800351f4080 R14: ffff880035120000 R15: ffff8801478e7000 [ 8.533441] FS: 00007f4ed97678c0(0000) GS:ffff88014ec00000(0000) knlGS:0000000000000000 [ 8.541525] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 8.547263] CR2: 0000000000000409 CR3: 0000000149f2d000 CR4: 00000000000006f0 [ 8.554396] Stack: [ 8.556412] ffff88014788b888 ffffffffc01c27a6 ffff88003485f050 ffff88003485f000 [ 8.563867] 0000000000000003 ffff88003574e000 0000000000000001 ffff88014788b8e8 [ 8.571329] ffffffffc00ccd95 ffff88014788b920 ffffffffc01a9319 ffff8801478e6200 [ 8.578789] Call Trace: [ 8.581289] [<ffffffffc01c27a6>] radeon_dp_detect+0x206/0x2b0 [radeon] [ 8.587918] [<ffffffffc00ccd95>] drm_helper_probe_single_connector_modes_merge_bits+0x235/0x4d0 [drm_kms_helper] [ 8.598217] [<ffffffffc01a9319>] ? atombios_crtc_disable+0x39/0x350 [radeon] [ 8.605356] [<ffffffffc00cd043>] drm_helper_probe_single_connector_modes+0x13/0x20 [drm_kms_helper] [ 8.614508] [<ffffffffc00d990e>] drm_fb_helper_initial_config+0xae/0x420 [drm_kms_helper] [ 8.622816] [<ffffffffc01cd16e>] radeon_fbdev_init+0xee/0x110 [radeon] [ 8.629471] [<ffffffffc01c726f>] radeon_modeset_init+0x43f/0xa10 [radeon] [ 8.636391] [<ffffffffc019f513>] radeon_driver_load_kms+0x123/0x220 [radeon] [ 8.643565] [<ffffffffc0035397>] drm_dev_register+0xa7/0xb0 [drm] [ 8.649765] [<ffffffffc00379cd>] drm_get_pci_dev+0x8d/0x1e0 [drm] [ 8.655985] [<ffffffffc019b3e0>] radeon_pci_probe+0xa0/0xb0 [radeon] [ 8.662438] [<ffffffff81439dc5>] local_pci_probe+0x45/0xa0 [ 8.668006] [<ffffffff8143b203>] pci_device_probe+0x103/0x150 [ 8.673841] [<ffffffff8154c6e2>] driver_probe_device+0x222/0x4a0 [ 8.679939] [<ffffffff8154c9e4>] __driver_attach+0x84/0x90 [ 8.685519] [<ffffffff8154c960>] ? driver_probe_device+0x4a0/0x4a0 [ 8.691787] [<ffffffff8154a30c>] bus_for_each_dev+0x6c/0xc0 [ 8.697445] [<ffffffff8154be9e>] driver_attach+0x1e/0x20 [ 8.702845] [<ffffffff8154b9db>] bus_add_driver+0x1eb/0x280 [ 8.708504] [<ffffffff8154d2f0>] driver_register+0x60/0xe0 [ 8.714077] [<ffffffff814396ec>] __pci_register_driver+0x4c/0x50 [ 8.720191] [<ffffffffc0037c00>] drm_pci_init+0xe0/0x110 [drm] [ 8.726104] [<ffffffffc030d000>] ? 0xffffffffc030d000 [ 8.731288] [<ffffffffc030d09d>] radeon_init+0x9d/0xb2 [radeon] [ 8.737297] [<ffffffff81002123>] do_one_initcall+0xb3/0x200 [ 8.742955] [<ffffffff811cee51>] ? __vunmap+0x91/0xe0 [ 8.748093] [<ffffffff811eaf83>] ? kmem_cache_alloc_trace+0x183/0x1f0 [ 8.754622] [<ffffffff8118c233>] do_init_module+0x5f/0x1cf [ 8.760193] [<ffffffff81109e67>] load_module+0x1667/0x1c00 [ 8.765764] [<ffffffff81106410>] ? __symbol_put+0x60/0x60 [ 8.771252] [<ffffffff81212820>] ? kernel_read+0x50/0x80 [ 8.776651] [<ffffffff8110a644>] SYSC_finit_module+0xb4/0xe0 [ 8.782395] [<ffffffff8110a68e>] SyS_finit_module+0xe/0x10 [ 8.787970] [<ffffffff818252f2>] entry_SYSCALL_64_fastpath+0x16/0x71 [ 8.794404] Code: 41 5d 41 5e 41 5f 5d c3 66 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 55 48 8b 87 a0 03 00 00 45 31 c0 31 d2 be 01 00 00 00 48 89 e5 <0f> b6 8 8 09 04 00 00 48 8b 07 48 8b 78 28 e8 23 f6 ff ff 5d c3 [ 8.814354] RIP [<ffffffffc02024da>] radeon_dp_getsinktype+0x1a/0x30 [radeon] [ 8.821645] RSP <ffff88014788b850> [ 8.825135] CR2: 0000000000000409 [ 8.828551] ---[ end trace 9fce5ace7679b4a0 ]--- Looking at the code, there are a number of places that assume that radeon_connector->ddc_bus is not a NULL pointer. The following workaround fixes the issue by ensuring that ddc_bus is always validated before use: diff -rupd linux-source-4.4.0.orig/drivers/gpu/drm/radeon/atombios_dp.c linux-source-4.4.0/drivers/gpu/drm/radeon/atombios_dp.c --- linux-source-4.4.0.orig/drivers/gpu/drm/radeon/atombios_dp.c 2016-05-12 23:03:23.000000000 +0100 +++ linux-source-4.4.0/drivers/gpu/drm/radeon/atombios_dp.c 2016-06-01 13:16:43.000000000 +0100 @@ -232,6 +232,9 @@ void radeon_dp_aux_init(struct radeon_co 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)) { @@ -352,6 +355,9 @@ u8 radeon_dp_getsinktype(struct radeon_c 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); } @@ -364,6 +370,9 @@ static void radeon_dp_probe_oui(struct r 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]); @@ -379,6 +388,9 @@ bool radeon_dp_getdpcd(struct radeon_con 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); @@ -416,24 +428,26 @@ int radeon_dp_get_panel_mode(struct drm_ 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; + } } } @@ -499,6 +513,9 @@ bool radeon_dp_needs_link_train(struct r 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; @@ -519,7 +536,7 @@ void radeon_dp_set_rx_power_state(struct 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); @@ -822,7 +839,8 @@ void radeon_dp_link_train(struct drm_enc 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; @@ -838,7 +856,7 @@ void radeon_dp_link_train(struct drm_enc 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 -rupd linux-source-4.4.0.orig/drivers/gpu/drm/radeon/radeon_connectors.c linux-source-4.4.0/drivers/gpu/drm/radeon/radeon_connectors.c --- linux-source-4.4.0.orig/drivers/gpu/drm/radeon/radeon_connectors.c 2016-01-10 23:01:32.000000000 +0000 +++ linux-source-4.4.0/drivers/gpu/drm/radeon/radeon_connectors.c 2016-06-01 13:21:24.000000000 +0100 @@ -327,26 +327,27 @@ static void radeon_connector_get_edid(st 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 radeon_connector->edid = drm_get_edid(&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) { @@ -1306,6 +1307,7 @@ radeon_dvi_detect(struct drm_connector * 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 -rupd linux-source-4.4.0.orig/drivers/gpu/drm/radeon/radeon_dp_mst.c linux-source-4.4.0/drivers/gpu/drm/radeon/radeon_dp_mst.c --- linux-source-4.4.0.orig/drivers/gpu/drm/radeon/radeon_dp_mst.c 2016-05-12 23:03:23.000000000 +0100 +++ linux-source-4.4.0/drivers/gpu/drm/radeon/radeon_dp_mst.c 2016-06-01 13:16:43.000000000 +0100 @@ -662,7 +662,7 @@ radeon_dp_mst_init(struct radeon_connect { 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; @@ -689,6 +689,9 @@ radeon_dp_mst_probe(struct radeon_connec if (dig_connector->dpcd[DP_DPCD_REV] < 0x12) return 0; + if (!radeon_connector->ddc_bus) + return 0; + ret = drm_dp_dpcd_read(&radeon_connector->ddc_bus->aux, DP_MSTM_CAP, msg, 1); if (ret) { @@ -718,6 +721,9 @@ radeon_dp_mst_check_status(struct radeon int ret = 0; bool handled; + if (!radeon_connector->ddc_bus) + return -EINVAL; + dret = drm_dp_dpcd_read(&radeon_connector->ddc_bus->aux, DP_SINK_COUNT_ESI, esi, 8); go_again: diff -rupd linux-source-4.4.0.orig/drivers/gpu/drm/radeon/radeon_i2c.c linux-source-4.4.0/drivers/gpu/drm/radeon/radeon_i2c.c --- linux-source-4.4.0.orig/drivers/gpu/drm/radeon/radeon_i2c.c 2016-01-10 23:01:32.000000000 +0000 +++ linux-source-4.4.0/drivers/gpu/drm/radeon/radeon_i2c.c 2016-06-01 13:16:43.000000000 +0100 @@ -63,6 +63,9 @@ bool radeon_ddc_probe(struct radeon_conn 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 { I think the changes to these four files should be pushed back to the main line kernel and it is probably also desirable to back port these changes to earlier kernels. To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1587885/+subscriptions -- Mailing list: https://launchpad.net/~kernel-packages Post to : kernel-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~kernel-packages More help : https://help.launchpad.net/ListHelp