| Message ID | 20230724024229.1118444-1-guchun.chen@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/ttm: check null pointer before accessing when swapping | expand |
On Sun, Jul 23, 2023 at 10:43 PM Guchun Chen <guchun.chen@amd.com> wrote: > > Add a check to avoid null pointer dereference as below: > > [ 90.002283] general protection fault, probably for non-canonical > address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI > [ 90.002292] KASAN: null-ptr-deref in range > [0x0000000000000000-0x0000000000000007] > [ 90.002346] ? exc_general_protection+0x159/0x240 > [ 90.002352] ? asm_exc_general_protection+0x26/0x30 > [ 90.002357] ? ttm_bo_evict_swapout_allowable+0x322/0x5e0 [ttm] > [ 90.002365] ? ttm_bo_evict_swapout_allowable+0x42e/0x5e0 [ttm] > [ 90.002373] ttm_bo_swapout+0x134/0x7f0 [ttm] > [ 90.002383] ? __pfx_ttm_bo_swapout+0x10/0x10 [ttm] > [ 90.002391] ? lock_acquire+0x44d/0x4f0 > [ 90.002398] ? ttm_device_swapout+0xa5/0x260 [ttm] > [ 90.002412] ? lock_acquired+0x355/0xa00 > [ 90.002416] ? do_raw_spin_trylock+0xb6/0x190 > [ 90.002421] ? __pfx_lock_acquired+0x10/0x10 > [ 90.002426] ? ttm_global_swapout+0x25/0x210 [ttm] > [ 90.002442] ttm_device_swapout+0x198/0x260 [ttm] > [ 90.002456] ? __pfx_ttm_device_swapout+0x10/0x10 [ttm] > [ 90.002472] ttm_global_swapout+0x75/0x210 [ttm] > [ 90.002486] ttm_tt_populate+0x187/0x3f0 [ttm] > [ 90.002501] ttm_bo_handle_move_mem+0x437/0x590 [ttm] > [ 90.002517] ttm_bo_validate+0x275/0x430 [ttm] > [ 90.002530] ? __pfx_ttm_bo_validate+0x10/0x10 [ttm] > [ 90.002544] ? kasan_save_stack+0x33/0x60 > [ 90.002550] ? kasan_set_track+0x25/0x30 > [ 90.002554] ? __kasan_kmalloc+0x8f/0xa0 > [ 90.002558] ? amdgpu_gtt_mgr_new+0x81/0x420 [amdgpu] > [ 90.003023] ? ttm_resource_alloc+0xf6/0x220 [ttm] > [ 90.003038] amdgpu_bo_pin_restricted+0x2dd/0x8b0 [amdgpu] > [ 90.003210] ? __x64_sys_ioctl+0x131/0x1a0 > [ 90.003210] ? do_syscall_64+0x60/0x90 > > Fixes: a2848d08742c ("drm/ttm: never consider pinned BOs for eviction&swap") > Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com> > Signed-off-by: Guchun Chen <guchun.chen@amd.com> Reviewed-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/ttm/ttm_bo.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 7139a522b2f3..54e3083076b7 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -519,7 +519,8 @@ static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo, > > if (bo->pin_count) { > *locked = false; > - *busy = false; > + if (busy) > + *busy = false; > return false; > } > > -- > 2.25.1 >
Am 24.07.23 um 15:36 schrieb Alex Deucher: > On Sun, Jul 23, 2023 at 10:43 PM Guchun Chen <guchun.chen@amd.com> wrote: >> Add a check to avoid null pointer dereference as below: >> >> [ 90.002283] general protection fault, probably for non-canonical >> address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI >> [ 90.002292] KASAN: null-ptr-deref in range >> [0x0000000000000000-0x0000000000000007] >> [ 90.002346] ? exc_general_protection+0x159/0x240 >> [ 90.002352] ? asm_exc_general_protection+0x26/0x30 >> [ 90.002357] ? ttm_bo_evict_swapout_allowable+0x322/0x5e0 [ttm] >> [ 90.002365] ? ttm_bo_evict_swapout_allowable+0x42e/0x5e0 [ttm] >> [ 90.002373] ttm_bo_swapout+0x134/0x7f0 [ttm] >> [ 90.002383] ? __pfx_ttm_bo_swapout+0x10/0x10 [ttm] >> [ 90.002391] ? lock_acquire+0x44d/0x4f0 >> [ 90.002398] ? ttm_device_swapout+0xa5/0x260 [ttm] >> [ 90.002412] ? lock_acquired+0x355/0xa00 >> [ 90.002416] ? do_raw_spin_trylock+0xb6/0x190 >> [ 90.002421] ? __pfx_lock_acquired+0x10/0x10 >> [ 90.002426] ? ttm_global_swapout+0x25/0x210 [ttm] >> [ 90.002442] ttm_device_swapout+0x198/0x260 [ttm] >> [ 90.002456] ? __pfx_ttm_device_swapout+0x10/0x10 [ttm] >> [ 90.002472] ttm_global_swapout+0x75/0x210 [ttm] >> [ 90.002486] ttm_tt_populate+0x187/0x3f0 [ttm] >> [ 90.002501] ttm_bo_handle_move_mem+0x437/0x590 [ttm] >> [ 90.002517] ttm_bo_validate+0x275/0x430 [ttm] >> [ 90.002530] ? __pfx_ttm_bo_validate+0x10/0x10 [ttm] >> [ 90.002544] ? kasan_save_stack+0x33/0x60 >> [ 90.002550] ? kasan_set_track+0x25/0x30 >> [ 90.002554] ? __kasan_kmalloc+0x8f/0xa0 >> [ 90.002558] ? amdgpu_gtt_mgr_new+0x81/0x420 [amdgpu] >> [ 90.003023] ? ttm_resource_alloc+0xf6/0x220 [ttm] >> [ 90.003038] amdgpu_bo_pin_restricted+0x2dd/0x8b0 [amdgpu] >> [ 90.003210] ? __x64_sys_ioctl+0x131/0x1a0 >> [ 90.003210] ? do_syscall_64+0x60/0x90 >> >> Fixes: a2848d08742c ("drm/ttm: never consider pinned BOs for eviction&swap") >> Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com> >> Signed-off-by: Guchun Chen <guchun.chen@amd.com> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> Reviewed-by: Christian König <christian.koenig@amd.com> Has this already been pushed to drm-misc-next? Thanks, Christian. > >> --- >> drivers/gpu/drm/ttm/ttm_bo.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c >> index 7139a522b2f3..54e3083076b7 100644 >> --- a/drivers/gpu/drm/ttm/ttm_bo.c >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c >> @@ -519,7 +519,8 @@ static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo, >> >> if (bo->pin_count) { >> *locked = false; >> - *busy = false; >> + if (busy) >> + *busy = false; >> return false; >> } >> >> -- >> 2.25.1 >>
[Public] > -----Original Message----- > From: Koenig, Christian <Christian.Koenig@amd.com> > Sent: Thursday, July 27, 2023 3:28 PM > To: Alex Deucher <alexdeucher@gmail.com>; Chen, Guchun > <Guchun.Chen@amd.com> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; airlied@gmail.com; > daniel@ffwll.ch; dri-devel@lists.freedesktop.org; Mikhail Gavrilov > <mikhail.v.gavrilov@gmail.com> > Subject: Re: [PATCH] drm/ttm: check null pointer before accessing when > swapping > > Am 24.07.23 um 15:36 schrieb Alex Deucher: > > On Sun, Jul 23, 2023 at 10:43 PM Guchun Chen <guchun.chen@amd.com> > wrote: > >> Add a check to avoid null pointer dereference as below: > >> > >> [ 90.002283] general protection fault, probably for non-canonical > >> address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI > >> [ 90.002292] KASAN: null-ptr-deref in range > >> [0x0000000000000000-0x0000000000000007] > >> [ 90.002346] ? exc_general_protection+0x159/0x240 > >> [ 90.002352] ? asm_exc_general_protection+0x26/0x30 > >> [ 90.002357] ? ttm_bo_evict_swapout_allowable+0x322/0x5e0 [ttm] > >> [ 90.002365] ? ttm_bo_evict_swapout_allowable+0x42e/0x5e0 [ttm] > >> [ 90.002373] ttm_bo_swapout+0x134/0x7f0 [ttm] > >> [ 90.002383] ? __pfx_ttm_bo_swapout+0x10/0x10 [ttm] > >> [ 90.002391] ? lock_acquire+0x44d/0x4f0 > >> [ 90.002398] ? ttm_device_swapout+0xa5/0x260 [ttm] > >> [ 90.002412] ? lock_acquired+0x355/0xa00 > >> [ 90.002416] ? do_raw_spin_trylock+0xb6/0x190 > >> [ 90.002421] ? __pfx_lock_acquired+0x10/0x10 > >> [ 90.002426] ? ttm_global_swapout+0x25/0x210 [ttm] > >> [ 90.002442] ttm_device_swapout+0x198/0x260 [ttm] > >> [ 90.002456] ? __pfx_ttm_device_swapout+0x10/0x10 [ttm] > >> [ 90.002472] ttm_global_swapout+0x75/0x210 [ttm] > >> [ 90.002486] ttm_tt_populate+0x187/0x3f0 [ttm] > >> [ 90.002501] ttm_bo_handle_move_mem+0x437/0x590 [ttm] > >> [ 90.002517] ttm_bo_validate+0x275/0x430 [ttm] > >> [ 90.002530] ? __pfx_ttm_bo_validate+0x10/0x10 [ttm] > >> [ 90.002544] ? kasan_save_stack+0x33/0x60 > >> [ 90.002550] ? kasan_set_track+0x25/0x30 > >> [ 90.002554] ? __kasan_kmalloc+0x8f/0xa0 > >> [ 90.002558] ? amdgpu_gtt_mgr_new+0x81/0x420 [amdgpu] > >> [ 90.003023] ? ttm_resource_alloc+0xf6/0x220 [ttm] > >> [ 90.003038] amdgpu_bo_pin_restricted+0x2dd/0x8b0 [amdgpu] > >> [ 90.003210] ? __x64_sys_ioctl+0x131/0x1a0 > >> [ 90.003210] ? do_syscall_64+0x60/0x90 > >> > >> Fixes: a2848d08742c ("drm/ttm: never consider pinned BOs for > >> eviction&swap") > >> Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com> > >> Signed-off-by: Guchun Chen <guchun.chen@amd.com> > > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> > > Reviewed-by: Christian König <christian.koenig@amd.com> > > Has this already been pushed to drm-misc-next? > > Thanks, > Christian. Not yet, Christian, as I don't have push permission. I saw you were on vacation, so I would expect to ping you to push after you are back with full recharge. Regards, Guchun > > > >> --- > >> drivers/gpu/drm/ttm/ttm_bo.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c > >> b/drivers/gpu/drm/ttm/ttm_bo.c index 7139a522b2f3..54e3083076b7 > >> 100644 > >> --- a/drivers/gpu/drm/ttm/ttm_bo.c > >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c > >> @@ -519,7 +519,8 @@ static bool > ttm_bo_evict_swapout_allowable(struct > >> ttm_buffer_object *bo, > >> > >> if (bo->pin_count) { > >> *locked = false; > >> - *busy = false; > >> + if (busy) > >> + *busy = false; > >> return false; > >> } > >> > >> -- > >> 2.25.1 > >>
On Thu, Jul 27, 2023 at 12:33 PM Chen, Guchun <Guchun.Chen@amd.com> wrote: > > Reviewed-by: Christian König <christian.koenig@amd.com> > > > > Has this already been pushed to drm-misc-next? > > > > Thanks, > > Christian. > > Not yet, Christian, as I don't have push permission. I saw you were on vacation, so I would expect to ping you to push after you are back with full recharge. I expect to see it in drm-fixes-6.5 cause the problem appeared during the 6.5 release cycle. And yes, I follow all pull requests. This patch was not included in yesterday's pull request :(
Am 27.07.23 um 09:39 schrieb Mikhail Gavrilov: > On Thu, Jul 27, 2023 at 12:33 PM Chen, Guchun <Guchun.Chen@amd.com> wrote: >>> Reviewed-by: Christian König <christian.koenig@amd.com> >>> >>> Has this already been pushed to drm-misc-next? >>> >>> Thanks, >>> Christian. >> Not yet, Christian, as I don't have push permission. I saw you were on vacation, so I would expect to ping you to push after you are back with full recharge. > I expect to see it in drm-fixes-6.5 cause the problem appeared during > the 6.5 release cycle. Good point. The patch introducing the problem had a CC: stable tag as well. So I've added the appropriate tags and pushed it to drm-misc-fixes. Thanks, Christian. > And yes, I follow all pull requests. This patch was not included in > yesterday's pull request :(
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 7139a522b2f3..54e3083076b7 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -519,7 +519,8 @@ static bool ttm_bo_evict_swapout_allowable(struct ttm_buffer_object *bo, if (bo->pin_count) { *locked = false; - *busy = false; + if (busy) + *busy = false; return false; }
Add a check to avoid null pointer dereference as below: [ 90.002283] general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] PREEMPT SMP KASAN NOPTI [ 90.002292] KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007] [ 90.002346] ? exc_general_protection+0x159/0x240 [ 90.002352] ? asm_exc_general_protection+0x26/0x30 [ 90.002357] ? ttm_bo_evict_swapout_allowable+0x322/0x5e0 [ttm] [ 90.002365] ? ttm_bo_evict_swapout_allowable+0x42e/0x5e0 [ttm] [ 90.002373] ttm_bo_swapout+0x134/0x7f0 [ttm] [ 90.002383] ? __pfx_ttm_bo_swapout+0x10/0x10 [ttm] [ 90.002391] ? lock_acquire+0x44d/0x4f0 [ 90.002398] ? ttm_device_swapout+0xa5/0x260 [ttm] [ 90.002412] ? lock_acquired+0x355/0xa00 [ 90.002416] ? do_raw_spin_trylock+0xb6/0x190 [ 90.002421] ? __pfx_lock_acquired+0x10/0x10 [ 90.002426] ? ttm_global_swapout+0x25/0x210 [ttm] [ 90.002442] ttm_device_swapout+0x198/0x260 [ttm] [ 90.002456] ? __pfx_ttm_device_swapout+0x10/0x10 [ttm] [ 90.002472] ttm_global_swapout+0x75/0x210 [ttm] [ 90.002486] ttm_tt_populate+0x187/0x3f0 [ttm] [ 90.002501] ttm_bo_handle_move_mem+0x437/0x590 [ttm] [ 90.002517] ttm_bo_validate+0x275/0x430 [ttm] [ 90.002530] ? __pfx_ttm_bo_validate+0x10/0x10 [ttm] [ 90.002544] ? kasan_save_stack+0x33/0x60 [ 90.002550] ? kasan_set_track+0x25/0x30 [ 90.002554] ? __kasan_kmalloc+0x8f/0xa0 [ 90.002558] ? amdgpu_gtt_mgr_new+0x81/0x420 [amdgpu] [ 90.003023] ? ttm_resource_alloc+0xf6/0x220 [ttm] [ 90.003038] amdgpu_bo_pin_restricted+0x2dd/0x8b0 [amdgpu] [ 90.003210] ? __x64_sys_ioctl+0x131/0x1a0 [ 90.003210] ? do_syscall_64+0x60/0x90 Fixes: a2848d08742c ("drm/ttm: never consider pinned BOs for eviction&swap") Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com> Signed-off-by: Guchun Chen <guchun.chen@amd.com> --- drivers/gpu/drm/ttm/ttm_bo.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)