Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S932345AbdC2PSz (ORCPT <rfc822;w@1wt.eu>);
        Wed, 29 Mar 2017 11:18:55 -0400
Received: from pegasos-out.vodafone.de ([80.84.1.38]:57541 "EHLO
        pegasos-out.vodafone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S932302AbdC2PSx (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 29 Mar 2017 11:18:53 -0400
X-Spam-Flag: NO
X-Spam-Score: 0.2
Authentication-Results: rohrpostix1.prod.vfnet.de (amavisd-new); dkim=pass
        header.i=@vodafone.de
X-DKIM: OpenDKIM Filter v2.6.8 pegasos-out.vodafone.de 6D582261EC4
X-DKIM: OpenDKIM Filter v2.0.2 smtp-01.vodafone.de BBC38E5676
Subject: Re: [PATCH] Fix IB va_start+ib_bytes range check on 32Bit systems
To: =?UTF-8?Q?Michel_D=c3=a4nzer?= <michel@daenzer.net>,
        Jan Burgmeier <jan.burgmeier@unicon-software.com>
References: <20170329091839.5246-1-jan.burgmeier@unicon-software.com>
 <20170329091839.5246-2-jan.burgmeier@unicon-software.com>
 <6dc2814c-c32b-220a-daa7-da530d13f20d@vodafone.de>
 <26c98d79-025c-bdf6-4bbe-bfd083cb6720@daenzer.net>
Cc: amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, airlied@linux.ie
From: =?UTF-8?Q?Christian_K=c3=b6nig?= <deathsimple@vodafone.de>
Message-ID: <af21de45-36ce-2351-7704-7e26d8212e3f@vodafone.de>
Date: Wed, 29 Mar 2017 17:18:24 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <26c98d79-025c-bdf6-4bbe-bfd083cb6720@daenzer.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 1816
Lines: 45

Am 29.03.2017 um 16:54 schrieb Michel Dänzer:
> On 29/03/17 10:22 PM, Christian König wrote:
>> Am 29.03.2017 um 11:18 schrieb Jan Burgmeier:
>>> Signed-off-by: Jan Burgmeier <jan.burgmeier@unicon-software.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 +++-
>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> index 99424cb8020b..583d22974e14 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>> @@ -908,6 +908,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device
>>> *adev,
>>>                struct amdgpu_bo *aobj = NULL;
>>>                uint64_t offset;
>>>                uint8_t *kptr;
>>> +            uint64_t it_last;
>>>                  m = amdgpu_cs_find_mapping(parser, chunk_ib->va_start,
>>>                               &aobj);
>>> @@ -916,8 +917,9 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device
>>> *adev,
>>>                    return -EINVAL;
>>>                }
>>>    +            it_last = m->it.last;
>>>                if ((chunk_ib->va_start + chunk_ib->ib_bytes) >
>>> -                (m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) {
>>> +                (it_last + 1) * AMDGPU_GPU_PAGE_SIZE) {
>> Nice catch, but just adding a u64 case should do here as well. E.g:
>>
>> if ((chunk_ib->va_start + chunk_ib->ib_bytes) >
>>      (u64)(m->it.last + 1) * AMDGPU_GPU_PAGE_SIZE) {
> That won't work correctly if m->it.last == 0xffffffff ? Or is that not
> possible?
Hui, why? is it.last signed?

And even then m->it.last probably won't ever become 0xffffffff on a 
32bit system.

BTW: We need to fix using the 64bit R/B tree instead of the long sized 
tree for Vega10 here anyway.

Regards,
Christian.