From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: "Christian König" <christian.koenig@amd.com>,
dri-devel@lists.freedesktop.org
Cc: Philip Yang <Philip.Yang@amd.com>,
Daniel Vetter <daniel.vetter@ffwll.ch>,
Felix Kuehling <Felix.Kuehling@amd.com>,
Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>,
Qiang Yu <qiang.yu@amd.com>, Huang Rui <ray.huang@amd.com>,
Matthew Auld <matthew.auld@intel.com>,
Alex Deucher <alexander.deucher@amd.com>,
intel-gfx@lists.freedesktop.org,
Nirmoy Das <nirmoy.das@intel.com>
Subject: Re: [Intel-gfx] [PATCH v2 1/7] drm/ttm: Fix a NULL pointer dereference
Date: Tue, 7 Mar 2023 18:46:54 +0100 [thread overview]
Message-ID: <efc256f9-d0ea-fbae-c1c8-5dce778cf37b@linux.intel.com> (raw)
In-Reply-To: <9383eed5-d3ad-d61f-9fce-6a543890ee83@amd.com>
On 3/7/23 17:55, Christian König wrote:
> Am 07.03.23 um 15:46 schrieb Thomas Hellström:
>> The LRU mechanism may look up a resource in the process of being removed
>> from an object. The locking rules here are a bit unclear but it looks
>> currently like res->bo assignment is protected by the LRU lock, whereas
>> bo->resource is protected by the object lock, while *clearing* of
>> bo->resource is also protected by the LRU lock. This means that if
>> we check that bo->resource points to the LRU resource under the LRU
>> lock we should be safe.
>> So perform that check before deciding to swap out a bo. That avoids
>> dereferencing a NULL bo->resource in ttm_bo_swapout().
>
> Please make sure that this is pushed to drm-misc-fixes ASAP.
>
> I've getting complains for this from different sides.
>
> Thanks,
> Christian.
Done.
/Thomas
>
>>
>> Fixes: 6a9b02899402 ("drm/ttm: move the LRU into resource handling v4")
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Cc: Huang Rui <ray.huang@amd.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Felix Kuehling <Felix.Kuehling@amd.com>
>> Cc: Philip Yang <Philip.Yang@amd.com>
>> Cc: Qiang Yu <qiang.yu@amd.com>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: Nirmoy Das <nirmoy.das@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
>> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
>> Cc: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
>> ---
>> drivers/gpu/drm/ttm/ttm_device.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_device.c
>> b/drivers/gpu/drm/ttm/ttm_device.c
>> index c7a1862f322a..ae2f19dc9f81 100644
>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>> @@ -158,7 +158,7 @@ int ttm_device_swapout(struct ttm_device *bdev,
>> struct ttm_operation_ctx *ctx,
>> struct ttm_buffer_object *bo = res->bo;
>> uint32_t num_pages;
>> - if (!bo)
>> + if (!bo || bo->resource != res)
>> continue;
>> num_pages = PFN_UP(bo->base.size);
>
WARNING: multiple messages have this Message-ID (diff)
From: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
To: "Christian König" <christian.koenig@amd.com>,
dri-devel@lists.freedesktop.org
Cc: Philip Yang <Philip.Yang@amd.com>,
Tvrtko Ursulin <tvrtko.ursulin@intel.com>,
Daniel Vetter <daniel.vetter@ffwll.ch>,
Felix Kuehling <Felix.Kuehling@amd.com>,
Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>,
Qiang Yu <qiang.yu@amd.com>, Huang Rui <ray.huang@amd.com>,
Matthew Auld <matthew.auld@intel.com>,
Anshuman Gupta <anshuman.gupta@intel.com>,
Alex Deucher <alexander.deucher@amd.com>,
intel-gfx@lists.freedesktop.org,
Nirmoy Das <nirmoy.das@intel.com>
Subject: Re: [PATCH v2 1/7] drm/ttm: Fix a NULL pointer dereference
Date: Tue, 7 Mar 2023 18:46:54 +0100 [thread overview]
Message-ID: <efc256f9-d0ea-fbae-c1c8-5dce778cf37b@linux.intel.com> (raw)
In-Reply-To: <9383eed5-d3ad-d61f-9fce-6a543890ee83@amd.com>
On 3/7/23 17:55, Christian König wrote:
> Am 07.03.23 um 15:46 schrieb Thomas Hellström:
>> The LRU mechanism may look up a resource in the process of being removed
>> from an object. The locking rules here are a bit unclear but it looks
>> currently like res->bo assignment is protected by the LRU lock, whereas
>> bo->resource is protected by the object lock, while *clearing* of
>> bo->resource is also protected by the LRU lock. This means that if
>> we check that bo->resource points to the LRU resource under the LRU
>> lock we should be safe.
>> So perform that check before deciding to swap out a bo. That avoids
>> dereferencing a NULL bo->resource in ttm_bo_swapout().
>
> Please make sure that this is pushed to drm-misc-fixes ASAP.
>
> I've getting complains for this from different sides.
>
> Thanks,
> Christian.
Done.
/Thomas
>
>>
>> Fixes: 6a9b02899402 ("drm/ttm: move the LRU into resource handling v4")
>> Cc: Christian König <christian.koenig@amd.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Cc: Huang Rui <ray.huang@amd.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Felix Kuehling <Felix.Kuehling@amd.com>
>> Cc: Philip Yang <Philip.Yang@amd.com>
>> Cc: Qiang Yu <qiang.yu@amd.com>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: Nirmoy Das <nirmoy.das@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
>> Cc: Anshuman Gupta <anshuman.gupta@intel.com>
>> Cc: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
>> Cc: dri-devel@lists.freedesktop.org
>> Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
>> ---
>> drivers/gpu/drm/ttm/ttm_device.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/ttm/ttm_device.c
>> b/drivers/gpu/drm/ttm/ttm_device.c
>> index c7a1862f322a..ae2f19dc9f81 100644
>> --- a/drivers/gpu/drm/ttm/ttm_device.c
>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
>> @@ -158,7 +158,7 @@ int ttm_device_swapout(struct ttm_device *bdev,
>> struct ttm_operation_ctx *ctx,
>> struct ttm_buffer_object *bo = res->bo;
>> uint32_t num_pages;
>> - if (!bo)
>> + if (!bo || bo->resource != res)
>> continue;
>> num_pages = PFN_UP(bo->base.size);
>
next prev parent reply other threads:[~2023-03-07 17:47 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-07 14:46 [Intel-gfx] [PATCH v2 0/7] drm/ttm: Small fixes / cleanups in prep for shrinking Thomas Hellström
2023-03-07 14:46 ` Thomas Hellström
2023-03-07 14:46 ` [Intel-gfx] [PATCH v2 1/7] drm/ttm: Fix a NULL pointer dereference Thomas Hellström
2023-03-07 14:46 ` Thomas Hellström
2023-03-07 16:55 ` [Intel-gfx] " Christian König
2023-03-07 16:55 ` Christian König
2023-03-07 17:46 ` Thomas Hellström [this message]
2023-03-07 17:46 ` Thomas Hellström
2023-03-07 14:46 ` [Intel-gfx] [PATCH v2 2/7] drm/ttm/pool: Fix ttm_pool_alloc error path Thomas Hellström
2023-03-07 14:46 ` Thomas Hellström
2023-03-08 8:48 ` [Intel-gfx] " Christian König
2023-03-08 8:48 ` Christian König
2023-03-08 8:58 ` [Intel-gfx] " Thomas Hellström
2023-03-08 8:58 ` Thomas Hellström
2023-03-07 14:46 ` [Intel-gfx] [PATCH v2 3/7] drm/ttm: Use the BIT macro for the TTM_TT_FLAGs Thomas Hellström
2023-03-07 14:46 ` Thomas Hellström
2023-03-08 8:49 ` [Intel-gfx] " Christian König
2023-03-08 8:49 ` Christian König
2023-03-09 7:06 ` [Intel-gfx] " Thomas Hellström
2023-03-09 7:06 ` Thomas Hellström
2023-03-09 8:06 ` [Intel-gfx] " Christian König
2023-03-09 8:06 ` Christian König
2023-03-07 14:46 ` [Intel-gfx] [PATCH v2 4/7] drm/ttm: Unexport ttm_global_swapout() Thomas Hellström
2023-03-07 14:46 ` Thomas Hellström
2023-03-08 8:49 ` [Intel-gfx] " Christian König
2023-03-08 8:49 ` Christian König
2023-03-07 14:46 ` [Intel-gfx] [PATCH v2 5/7] drm/ttm: Don't print error message if eviction was interrupted Thomas Hellström
2023-03-07 14:46 ` Thomas Hellström
2023-03-08 8:50 ` [Intel-gfx] " Christian König
2023-03-08 8:50 ` Christian König
2023-03-07 14:46 ` [Intel-gfx] [PATCH v2 6/7] drm/ttm: Reduce the number of used allocation orders for TTM pages Thomas Hellström
2023-03-07 14:46 ` Thomas Hellström
2023-03-07 19:42 ` [Intel-gfx] " kernel test robot
2023-03-07 19:42 ` kernel test robot
2023-03-08 9:15 ` Christian König
2023-03-08 9:15 ` Christian König
2023-03-08 9:22 ` [Intel-gfx] " Thomas Hellström
2023-03-08 9:22 ` Thomas Hellström
2023-03-07 14:46 ` [Intel-gfx] [PATCH v2 7/7] drm/ttm: Make the call to ttm_tt_populate() interruptible when faulting Thomas Hellström
2023-03-07 14:46 ` Thomas Hellström
2023-03-07 21:16 ` [Intel-gfx] ✓ Fi.CI.BAT: success for drm/ttm: Small fixes / cleanups in prep for shrinking Patchwork
2023-03-09 12:09 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=efc256f9-d0ea-fbae-c1c8-5dce778cf37b@linux.intel.com \
--to=thomas.hellstrom@linux.intel.com \
--cc=Arunpravin.PaneerSelvam@amd.com \
--cc=Felix.Kuehling@amd.com \
--cc=Philip.Yang@amd.com \
--cc=alexander.deucher@amd.com \
--cc=christian.koenig@amd.com \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=matthew.auld@intel.com \
--cc=nirmoy.das@intel.com \
--cc=qiang.yu@amd.com \
--cc=ray.huang@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.