[PATCH] drm/ast: Fix potential NULL-pointer read during atomic mode setting

Thomas Zimmermann tzimmermann at suse.de
Thu Nov 14 11:45:32 UTC 2019


When enabling the CRTC after waking up from a power-saving mode, the
primary plane's framebuffer might be NULL, which leads to a stack trace
as shown below.

  [  632.624608] BUG: kernel NULL pointer dereference, address: 0000000000000048
  [  632.624631] #PF: supervisor read access in kernel mode
  [  632.624639] #PF: error_code(0x0000) - not-present page
  [  632.624647] PGD 0 P4D 0
  [  632.624654] Oops: 0000 [#1] SMP PTI
  [  632.624662] CPU: 0 PID: 2082 Comm: gnome-shell Tainted: G            E     5.4.0-rc7-1-default+ #114
  [  632.624673] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN FIRE X2270 M2, BIOS 2.05    07/01/2010
  [  632.624689] RIP: 0010:ast_crtc_helper_atomic_enable+0x7d/0x680 [ast]
  [  632.624698] Code: 48 8b 80 e0 02 00 00 4c 8b 60 10 31 c0 f3 48 ab 48 8b 83 78 04 00 00 4c 89 ef 48 8d 70 18 e8 9a e9 55 ce 48 8b 83 78 04 00 00 <49> 8b 7c 24 48 4c 89 ea 4c 8d 44 24 28 48 8d 4c 24 20 48 8d 70 18
  [  632.624718] RSP: 0018:ffffbe9ec123fa40 EFLAGS: 00010246
  [  632.624726] RAX: ffff95a13cfd3400 RBX: ffff95a13cf32000 RCX: 0000000000000000
  [  632.624735] RDX: 0000000000000000 RSI: ffff95a13cfd34e8 RDI: ffffbe9ec123fb40
  [  632.624744] RBP: ffffbe9ec123fb80 R08: 0000000000000000 R09: 0000000000000003
  [  632.624753] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
  [  632.624762] R13: ffffbe9ec123fa70 R14: ffff95a13beb7000 R15: ffff95a13cf32800
  [  632.624772] FS:  00007f6d2763e140(0000) GS:ffff95a134000000(0000) knlGS:0000000000000000
  [  632.624782] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  [  632.624790] CR2: 0000000000000048 CR3: 00000001192f8004 CR4: 00000000000206f0
  [  632.624800] Call Trace:
  [  632.624811]  ? __lock_acquire+0x409/0x7c0
  [  632.624830]  drm_atomic_helper_commit_modeset_enables+0x1af/0x200
  [  632.624840]  drm_atomic_helper_commit_tail+0x32/0x70
  [  632.624849]  commit_tail+0xc7/0x110
  [  632.624857]  drm_atomic_helper_commit+0x121/0x130
  [  632.624867]  drm_atomic_connector_commit_dpms+0xd7/0x100
  [  632.624878]  set_property_atomic+0xaf/0x110
  [  632.624890]  drm_mode_obj_set_property_ioctl+0xbb/0x190
  [  632.624899]  ? drm_mode_obj_find_prop_id+0x40/0x40
  [  632.624909]  drm_ioctl_kernel+0x86/0xd0
  [  632.624918]  drm_ioctl+0x1e4/0x36b
  [  632.624925]  ? drm_mode_obj_find_prop_id+0x40/0x40
  [  632.624939]  do_vfs_ioctl+0x4bd/0x6e0
  [  632.624949]  ksys_ioctl+0x5e/0x90
  [  632.624957]  __x64_sys_ioctl+0x16/0x20
  [  632.624966]  do_syscall_64+0x5a/0x220
  [  632.624976]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
  [  632.624984] RIP: 0033:0x7f6d2b0de387
  [  632.624991] Code: 00 00 90 48 8b 05 f9 9a 0c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d c9 9a 0c 00 f7 d8 64 89 01 48
  [  632.625011] RSP: 002b:00007fffb49def38 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
  [  632.625021] RAX: ffffffffffffffda RBX: 00007fffb49def70 RCX: 00007f6d2b0de387
  [  632.625030] RDX: 00007fffb49def70 RSI: 00000000c01864ba RDI: 0000000000000009
  [  632.625040] RBP: 00000000c01864ba R08: 0000000000000000 R09: 00000000c0c0c0c0
  [  632.625049] R10: 0000000000000030 R11: 0000000000000246 R12: 000055bc367eb920
  [  632.625058] R13: 0000000000000009 R14: 0000000000000002 R15: 0000000000000000
  [  632.625071] Modules linked in: ebtable_filter(E) ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E) ip_tables(E) x_tables(E) af_packet(E) scsi_transport_iscsi(E) dmi_sysfs(E) msr(E) xfs(E) intel_powerclamp(E) coretemp(E) k)
  [  632.625185] CR2: 0000000000000048

The STR is

  * start gdm and wait for it to switch off the display
  * wake up the display by pressing a key

The fix avoids dereferencing the pointer and postpones modesetting until
there's a framebuffer bound to the CRTC.

Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
Fixes: b48e1b6ffd28 ("drm/ast: Add CRTC helpers for atomic modesetting")
Cc: Gerd Hoffmann <kraxel at redhat.com>
Cc: Dave Airlie <airlied at redhat.com>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: "Y.C. Chen" <yc_chen at aspeedtech.com>
Cc: Sam Ravnborg <sam at ravnborg.org>
---
 drivers/gpu/drm/ast/ast_mode.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 4725ec911a66..b173a8e5c00b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -774,7 +774,8 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
 					       &adjusted_mode, &vbios_mode);
 		if (!succ)
 			return -EINVAL;
-	}
+	} else
+		state->mode_changed = true;
 
 	return 0;
 }
@@ -824,6 +825,9 @@ ast_crtc_helper_atomic_enable(struct drm_crtc *crtc,
 	struct ast_vbios_mode_info vbios_mode;
 	bool succ;
 
+	if (!fb)
+		return;
+
 	memset(&adjusted_mode, 0, sizeof(adjusted_mode));
 	drm_mode_copy(&adjusted_mode, &crtc->state->adjusted_mode);
 
@@ -832,6 +836,13 @@ ast_crtc_helper_atomic_enable(struct drm_crtc *crtc,
 	if (WARN_ON_ONCE(!succ))
 		return;
 
+	/*
+	 * TODO: Some of the registers' values depend on the framebuffer
+	 *       configuration. To resolve this, fully separate the mode
+	 *	 setting from the framebuffer update. The vbios mode info
+	 *       should be gathered by atomic_check, the framebuffer regs
+	 *       should be set in primary plane's update function.
+	 */
 	ast_set_vbios_mode_reg(crtc, &adjusted_mode, &vbios_mode);
 	ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06);
 	ast_set_std_reg(crtc, &adjusted_mode, &vbios_mode);
-- 
2.23.0



More information about the dri-devel mailing list