NetBSD Problem Report #53565

From www@NetBSD.org  Sat Sep  1 17:29:50 2018
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 80D237A18D
	for <gnats-bugs@gnats.NetBSD.org>; Sat,  1 Sep 2018 17:29:50 +0000 (UTC)
Message-Id: <20180901171414.2AEBB7A1F6@mollari.NetBSD.org>
Date: Sat,  1 Sep 2018 17:14:14 +0000 (UTC)
From: tobiasu@tmux.org
Reply-To: tobiasu@tmux.org
To: gnats-bugs@NetBSD.org
Subject: radeon fence_get crash
X-Send-Pr-Version: www-1.0

>Number:         53565
>Category:       kern
>Synopsis:       radeon fence_get crash
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sat Sep 01 17:30:01 +0000 2018
>Closed-Date:    Sun Sep 02 00:10:28 +0000 2018
>Last-Modified:  Sun Sep 02 00:10:28 +0000 2018
>Originator:     Tobias Ulmer
>Release:        NetBSD 8.99.24
>Organization:
>Environment:
NetBSD i7.tmux.org 8.99.24 NetBSD 8.99.24 (GENERIC) #2: Sat Sep  1 16:29:09 UTC 2018  root@i7.tmux.org:/root/GENERIC amd64
>Description:
Open/close some X11 clients, within a couple of seconds/minutes the machine panics:

alltraps() at netbsd:alltraps+0xc8   
radeon_fence_ref() at netbsd:radeon_fence_ref+0x11
radeon_sa_bo_new() at netbsd:radeon_sa_bo_new+0x280
radeon_ib_get() at netbsd:radeon_ib_get+0x32
radeon_cs_ioctl() at netbsd:radeon_cs_ioctl+0x249
drm_ioctl() at netbsd:drm_ioctl+0x234
sys_ioctl() at netbsd:sys_ioctl+0x11c
syscall() at netbsd:syscall+0x173

Gdb confirms the fence argument to radeon_fence_ref is NULL, which makes sense as radeon_sa_bo_new() doesn't always initialize every member of "fences" in radeon_sa_bo_next_hole() depending on conditions.

On Linux, calls to radeon_fence_ref end up in dma_fence_get(), which checks for NULL: https://elixir.bootlin.com/linux/latest/source/include/linux/dma-fence.h#L274 
>How-To-Repeat:

>Fix:
I'm proposing the following trivial fix:

--- a/external/bsd/drm2/linux/linux_fence.c
+++ b/external/bsd/drm2/linux/linux_fence.c
@@ -178,8 +178,8 @@ fence_is_later(struct fence *a, struct fence *b)
 struct fence *
 fence_get(struct fence *fence)
 {
-
-       kref_get(&fence->refcount);
+       if (fence)
+               kref_get(&fence->refcount);
        return fence;
 }


>Release-Note:

>Audit-Trail:
From: "Taylor R Campbell" <riastradh@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/53565 CVS commit: src/sys/external/bsd/drm2/linux
Date: Sat, 1 Sep 2018 22:28:04 +0000

 Module Name:	src
 Committed By:	riastradh
 Date:		Sat Sep  1 22:28:04 UTC 2018

 Modified Files:
 	src/sys/external/bsd/drm2/linux: linux_fence.c

 Log Message:
 Allow fence_get(NULL).

 This is kind of silly: if the caller has exclusive access to the
 fence, they should be able to guarantee it's there, so this could
 serve to suppress legitimate bugs.  But better to preserve Linux
 semantics here than to be picky about API design.

 Bug by me, fix from Tobias Ulmer in PR kern/53565.


 To generate a diff of this commit:
 cvs rdiff -u -r1.12 -r1.13 src/sys/external/bsd/drm2/linux/linux_fence.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: maya@NetBSD.org
State-Changed-When: Sun, 02 Sep 2018 00:10:28 +0000
State-Changed-Why:
Applied, thanks for the patch!


>Unformatted:

Home	PR Database Search