From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-bn1bon0074.outbound.protection.outlook.com ([157.56.111.74]:37252 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933901AbcCITFu (ORCPT ); Wed, 9 Mar 2016 14:05:50 -0500 Subject: Re: Oops in 3.10.99 -- NULL pointer dereference in radeon_fence_ref To: Luis Henriques , Greg Kroah-Hartman References: <20160307025014.GA9499@mail.codepoet.org> <20160307204654.GB6545@kroah.com> <56DDED67.2030801@amd.com> <20160307225851.GB25867@kroah.com> <20160309135612.GA20283@charon.olymp> CC: =?UTF-8?Q?Christian_K=c3=b6nig?= , , linux-kernel , , Sasha Levin , Jiri Slaby , Kamal Mostafa From: =?UTF-8?Q?Nicolai_H=c3=a4hnle?= Message-ID: <56E04FFA.7070906@amd.com> Date: Wed, 9 Mar 2016 11:31:54 -0500 MIME-Version: 1.0 In-Reply-To: <20160309135612.GA20283@charon.olymp> Content-Type: multipart/mixed; boundary="------------090506060305070802030409" Sender: stable-owner@vger.kernel.org List-ID: --------------090506060305070802030409 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 8bit On 09.03.2016 08:56, Luis Henriques wrote: > On Mon, Mar 07, 2016 at 02:58:51PM -0800, Greg Kroah-Hartman wrote: >> On Mon, Mar 07, 2016 at 10:06:47PM +0100, Christian K�nig wrote: >>> Am 07.03.2016 um 21:46 schrieb Greg Kroah-Hartman: >>>> On Sun, Mar 06, 2016 at 07:50:14PM -0700, Erik Andersen wrote: >>>>> The following patch to radeon_sa_bo_new that >>>>> went into 3.10.99 >>>>> >>>>> commit 8d5e1e5af0c667545c202e8f4051f77aa3bf31b7 >>>>> Author: Nicolai Hähnle >>>>> Date: Fri Feb 5 14:35:53 2016 -0500 >>>>> drm/radeon: hold reference to fences in radeon_sa_bo_new >>>>> commit f6ff4f67cdf8455d0a4226eeeaf5af17c37d05eb upstream. >>>>> >>>>> is triggering an Oops for me right when xscreensaver >>>>> first began doing 3D stuff. After reverting this >>>>> patch, xscreensaver has been happily running 3D stuff. >>>>> >>>>> Mar 6 18:00:43 sage kernel: BUG: unable to handle kernel NULL pointer dereference at 0000000000000008 >>>>> Mar 6 18:00:43 sage kernel: IP: [] radeon_fence_ref+0xd/0x50 [radeon] >>>>> Mar 6 18:00:43 sage kernel: PGD 799e1d067 PUD 819186067 PMD 0 >>>>> Mar 6 18:00:43 sage kernel: Oops: 0002 [#1] SMP >>>>> >>>>> Mar 6 18:00:43 sage kernel: Stack: >>>>> Mar 6 18:00:43 sage kernel: ffffffffa01607ec ffff88108a4e8000 ffff88108a4e8000 ffff880888fbc000 >>>>> Mar 6 18:00:43 sage kernel: ffff880ecbf11c78 0000fe2001000006 0000000000000000 0020000000000100 >>>>> Mar 6 18:00:43 sage kernel: 00000000000d1200 ffff880ecbf11c14 0000000000000000 0000000000000000 >>>>> Mar 6 18:00:43 sage kernel: Call Trace: >>>>> Mar 6 18:00:43 sage kernel: [] ? radeon_sa_bo_new+0x2ac/0x4f0 [radeon] >>>>> Mar 6 18:00:43 sage kernel: [] ? ttm_eu_list_ref_sub+0x3d/0x60 [ttm] >>>>> Mar 6 18:00:43 sage kernel: [] radeon_ib_get+0x39/0x110 [radeon] >>>>> Mar 6 18:00:43 sage kernel: [] radeon_cs_ioctl+0x69a/0xa70 [radeon] >>>>> Mar 6 18:00:43 sage kernel: [] drm_ioctl+0x512/0x650 [drm] >>>>> Mar 6 18:00:43 sage kernel: [] ? do_futex+0x111/0xc30 >>>>> Mar 6 18:00:43 sage kernel: [] do_vfs_ioctl+0x305/0x520 >>>>> Mar 6 18:00:43 sage kernel: [] ? vtime_account_user+0x69/0x80 >>>>> Mar 6 18:00:43 sage kernel: [] SyS_ioctl+0x81/0xa0 >>>>> Mar 6 18:00:43 sage kernel: [] tracesys+0xe1/0xe6 >>>>> >>>>> $ lspci | grep VGA >>>>> 03:00.0 VGA compatible controller: Advanced Micro Devices, Inc. >>>>> [AMD/ATI] Redwood XT [Radeon HD 5670/5690/5730] >>>> Next time, please cc: the people responsible for that patch as well... >>>> >>>> I can revert it, but maybe something else is going on here? Do you have >>>> this same problem on 3.14, and 4.5-rc7? >>> >>> Hi Greg, >>> >>> yes that's an already known issue. Feel free to revert that one for now. >>> >>> I got it on my TODO list to provide a fixed patch for older kernel, but that >>> can take a while. >>> >>> For the background Nicolais patch is correct, but assumes that >>> radeon_fence_unref() can safely take NULL as the fence which is not the case >>> for older kernels. Actually, the call to radeon_fence_ref() is the culprit. >> >> Ok, thanks, now reverted. >> > > And looks like a few more kernels may be affected as well. I'll > revert it from 3.16 kernel, and I'm adding Kamal, Sasha and Jiri to > the CC list. Kernels that contain commit 954605ca "drm/radeon: use common fence implementation for fences, v4" are safe, older kernels require a NULL-pointer check around the call to radeon_fence_ref. This means kernels 3.17 and older are affected and need the additional NULL pointer check that I've sent out already on a different thread (I'm attaching it again, hoping that Erik gets a chance to test it). It would be nice to get a confirmation that this really does fix the observed bug, then I can prepare a fixed version of the patch for 3.17 and older (i.e. squash the original bad commit with the attached patch). Cheers, Nicolai > > Cheers, > -- > Lu�s > >> greg k-h >> -- >> To unsubscribe from this list: send the line "unsubscribe stable" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html --------------090506060305070802030409 Content-Type: text/x-patch; name="0001-drm-radeon-guard-call-to-radeon_fence_ref-against-NU.patch" Content-Transfer-Encoding: 8bit Content-Disposition: attachment; filename*0="0001-drm-radeon-guard-call-to-radeon_fence_ref-against-NU.pa"; filename*1="tch" >>From 85d028178d9772f2a07e4ed156820d95c4e0ad18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolai=20H=C3=A4hnle?= Date: Mon, 7 Mar 2016 23:41:52 -0300 Subject: [PATCH] drm/radeon: guard call to radeon_fence_ref against NULL pointers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Candidate fix for a kernel oops that was introduced by the backport of commit 954605ca3 "drm/radeon: hold reference to fences in radeon_sa_bo_new" to kernels where radeon does not use the common fence implementation for fences. Reported-by: Lutz Euler Signed-off-by: Nicolai Hähnle --- drivers/gpu/drm/radeon/radeon_sa.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c index 197b157..7d11901 100644 --- a/drivers/gpu/drm/radeon/radeon_sa.c +++ b/drivers/gpu/drm/radeon/radeon_sa.c @@ -349,8 +349,10 @@ int radeon_sa_bo_new(struct radeon_device *rdev, /* see if we can skip over some allocations */ } while (radeon_sa_bo_next_hole(sa_manager, fences, tries)); - for (i = 0; i < RADEON_NUM_RINGS; ++i) - radeon_fence_ref(fences[i]); + for (i = 0; i < RADEON_NUM_RINGS; ++i) { + if (fences[i]) + radeon_fence_ref(fences[i]); + } spin_unlock(&sa_manager->wq.lock); r = radeon_fence_wait_any(rdev, fences, false); -- 2.5.0 --------------090506060305070802030409--