* [PATCH] drm/ttm: add a WARN_ON in ttm_set_driver_manager when array bounds (v2) @ 2021-09-10 10:09 Guchun Chen 2021-09-10 18:24 ` Robin Murphy 0 siblings, 1 reply; 4+ messages in thread From: Guchun Chen @ 2021-09-10 10:09 UTC (permalink / raw) To: amd-gfx, dri-devel, christian.koenig, xinhui.pan, alexander.deucher Cc: Guchun Chen, Leslie Shi Vendor will define their own memory types on top of TTM_PL_PRIV, but call ttm_set_driver_manager directly without checking mem_type value when setting up memory manager. So add such check to aware the case when array bounds. v2: lower check level to WARN_ON Signed-off-by: Leslie Shi <Yuliang.Shi@amd.com> Signed-off-by: Guchun Chen <guchun.chen@amd.com> --- include/drm/ttm/ttm_device.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h index 07d722950d5b..aa79953c807c 100644 --- a/include/drm/ttm/ttm_device.h +++ b/include/drm/ttm/ttm_device.h @@ -291,6 +291,7 @@ ttm_manager_type(struct ttm_device *bdev, int mem_type) static inline void ttm_set_driver_manager(struct ttm_device *bdev, int type, struct ttm_resource_manager *manager) { + WARN_ON(type >= TTM_NUM_MEM_TYPES); bdev->man_drv[type] = manager; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/ttm: add a WARN_ON in ttm_set_driver_manager when array bounds (v2) 2021-09-10 10:09 [PATCH] drm/ttm: add a WARN_ON in ttm_set_driver_manager when array bounds (v2) Guchun Chen @ 2021-09-10 18:24 ` Robin Murphy 2021-09-13 2:35 ` Chen, Guchun 0 siblings, 1 reply; 4+ messages in thread From: Robin Murphy @ 2021-09-10 18:24 UTC (permalink / raw) To: Guchun Chen, amd-gfx, dri-devel, christian.koenig, xinhui.pan, alexander.deucher Cc: Leslie Shi On 2021-09-10 11:09, Guchun Chen wrote: > Vendor will define their own memory types on top of TTM_PL_PRIV, > but call ttm_set_driver_manager directly without checking mem_type > value when setting up memory manager. So add such check to aware > the case when array bounds. > > v2: lower check level to WARN_ON > > Signed-off-by: Leslie Shi <Yuliang.Shi@amd.com> > Signed-off-by: Guchun Chen <guchun.chen@amd.com> > --- > include/drm/ttm/ttm_device.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h > index 07d722950d5b..aa79953c807c 100644 > --- a/include/drm/ttm/ttm_device.h > +++ b/include/drm/ttm/ttm_device.h > @@ -291,6 +291,7 @@ ttm_manager_type(struct ttm_device *bdev, int mem_type) > static inline void ttm_set_driver_manager(struct ttm_device *bdev, int type, > struct ttm_resource_manager *manager) > { > + WARN_ON(type >= TTM_NUM_MEM_TYPES); Nit: I know nothing about this code, but from the context alone it would seem sensible to do if (WARN_ON(type >= TTM_NUM_MEM_TYPES)) return; to avoid making the subsequent assignment when we *know* it's invalid and likely to corrupt memory. Robin. > bdev->man_drv[type] = manager; > } > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] drm/ttm: add a WARN_ON in ttm_set_driver_manager when array bounds (v2) 2021-09-10 18:24 ` Robin Murphy @ 2021-09-13 2:35 ` Chen, Guchun 2021-09-13 6:23 ` Christian König 0 siblings, 1 reply; 4+ messages in thread From: Chen, Guchun @ 2021-09-13 2:35 UTC (permalink / raw) To: Robin Murphy, amd-gfx, dri-devel, Koenig, Christian, Pan, Xinhui, Deucher, Alexander Cc: Shi, Leslie [Public] Thanks for your suggestion, Robin. Do you agree with this as well, Christian and Xinhui? Regards, Guchun -----Original Message----- From: Robin Murphy <robin.murphy@arm.com> Sent: Saturday, September 11, 2021 2:25 AM To: Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com> Cc: Shi, Leslie <Yuliang.Shi@amd.com> Subject: Re: [PATCH] drm/ttm: add a WARN_ON in ttm_set_driver_manager when array bounds (v2) On 2021-09-10 11:09, Guchun Chen wrote: > Vendor will define their own memory types on top of TTM_PL_PRIV, but > call ttm_set_driver_manager directly without checking mem_type value > when setting up memory manager. So add such check to aware the case > when array bounds. > > v2: lower check level to WARN_ON > > Signed-off-by: Leslie Shi <Yuliang.Shi@amd.com> > Signed-off-by: Guchun Chen <guchun.chen@amd.com> > --- > include/drm/ttm/ttm_device.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/drm/ttm/ttm_device.h > b/include/drm/ttm/ttm_device.h index 07d722950d5b..aa79953c807c 100644 > --- a/include/drm/ttm/ttm_device.h > +++ b/include/drm/ttm/ttm_device.h > @@ -291,6 +291,7 @@ ttm_manager_type(struct ttm_device *bdev, int mem_type) > static inline void ttm_set_driver_manager(struct ttm_device *bdev, int type, > struct ttm_resource_manager *manager) > { > + WARN_ON(type >= TTM_NUM_MEM_TYPES); Nit: I know nothing about this code, but from the context alone it would seem sensible to do if (WARN_ON(type >= TTM_NUM_MEM_TYPES)) return; to avoid making the subsequent assignment when we *know* it's invalid and likely to corrupt memory. Robin. > bdev->man_drv[type] = manager; > } > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/ttm: add a WARN_ON in ttm_set_driver_manager when array bounds (v2) 2021-09-13 2:35 ` Chen, Guchun @ 2021-09-13 6:23 ` Christian König 0 siblings, 0 replies; 4+ messages in thread From: Christian König @ 2021-09-13 6:23 UTC (permalink / raw) To: Chen, Guchun, Robin Murphy, amd-gfx, dri-devel, Pan, Xinhui, Deucher, Alexander Cc: Shi, Leslie Well it will crash later on when accessing the invalid offset, so not much gained. But either way works for me. Christian. Am 13.09.21 um 04:35 schrieb Chen, Guchun: > [Public] > > Thanks for your suggestion, Robin. Do you agree with this as well, Christian and Xinhui? > > Regards, > Guchun > > -----Original Message----- > From: Robin Murphy <robin.murphy@arm.com> > Sent: Saturday, September 11, 2021 2:25 AM > To: Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com> > Cc: Shi, Leslie <Yuliang.Shi@amd.com> > Subject: Re: [PATCH] drm/ttm: add a WARN_ON in ttm_set_driver_manager when array bounds (v2) > > On 2021-09-10 11:09, Guchun Chen wrote: >> Vendor will define their own memory types on top of TTM_PL_PRIV, but >> call ttm_set_driver_manager directly without checking mem_type value >> when setting up memory manager. So add such check to aware the case >> when array bounds. >> >> v2: lower check level to WARN_ON >> >> Signed-off-by: Leslie Shi <Yuliang.Shi@amd.com> >> Signed-off-by: Guchun Chen <guchun.chen@amd.com> >> --- >> include/drm/ttm/ttm_device.h | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/include/drm/ttm/ttm_device.h >> b/include/drm/ttm/ttm_device.h index 07d722950d5b..aa79953c807c 100644 >> --- a/include/drm/ttm/ttm_device.h >> +++ b/include/drm/ttm/ttm_device.h >> @@ -291,6 +291,7 @@ ttm_manager_type(struct ttm_device *bdev, int mem_type) >> static inline void ttm_set_driver_manager(struct ttm_device *bdev, int type, >> struct ttm_resource_manager *manager) >> { >> + WARN_ON(type >= TTM_NUM_MEM_TYPES); > Nit: I know nothing about this code, but from the context alone it would seem sensible to do > > if (WARN_ON(type >= TTM_NUM_MEM_TYPES)) > return; > > to avoid making the subsequent assignment when we *know* it's invalid and likely to corrupt memory. > > Robin. > >> bdev->man_drv[type] = manager; >> } >> >> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-13 6:23 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-10 10:09 [PATCH] drm/ttm: add a WARN_ON in ttm_set_driver_manager when array bounds (v2) Guchun Chen 2021-09-10 18:24 ` Robin Murphy 2021-09-13 2:35 ` Chen, Guchun 2021-09-13 6:23 ` Christian König
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.