| Message ID | 20240806143855.29789-2-pstanner@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/sched: memset() 'job' in drm_sched_job_init() | expand |
*PING* On Tue, 2024-08-06 at 16:38 +0200, Philipp Stanner wrote: > drm_sched_job_init() has no control over how users allocate struct > drm_sched_job. Unfortunately, the function can also not set some > struct > members such as job->sched. > > This could theoretically lead to UB by users dereferencing the > struct's > pointer members too early. > > It is easier to debug such issues if these pointers are initialized > to > NULL, so dereferencing them causes a NULL pointer exception. > Accordingly, drm_sched_entity_init() does precisely that and > initializes > its struct with memset(). > > Initialize parameter "job" to 0 in drm_sched_job_init(). > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> > --- > Hi all, > I did some experiments with the scheduler recently and am trying to > make > the documentation and bits of the code more bullet proof. > > I tested the performance of v6.11-rc2 with and without this memset() > by > creating 1e6 jobs and found no performance regression. > > Cheers, > P. > --- > drivers/gpu/drm/scheduler/sched_main.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/scheduler/sched_main.c > b/drivers/gpu/drm/scheduler/sched_main.c > index 76969f9c59c2..1498ee3cbf39 100644 > --- a/drivers/gpu/drm/scheduler/sched_main.c > +++ b/drivers/gpu/drm/scheduler/sched_main.c > @@ -936,6 +936,14 @@ int drm_sched_job_init(struct drm_sched_job > *job, > return -EINVAL; > } > > + /* > + * We don't know for sure how the user has allocated. Thus, > zero the > + * struct so that unallowed (i.e., too early) usage of > pointers that > + * this function does not set is guaranteed to lead to a > NULL pointer > + * exception instead of UB. > + */ > + memset(job, 0, sizeof(*job)); > + > job->entity = entity; > job->credits = credits; > job->s_fence = drm_sched_fence_alloc(entity, owner);
diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index 76969f9c59c2..1498ee3cbf39 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -936,6 +936,14 @@ int drm_sched_job_init(struct drm_sched_job *job, return -EINVAL; } + /* + * We don't know for sure how the user has allocated. Thus, zero the + * struct so that unallowed (i.e., too early) usage of pointers that + * this function does not set is guaranteed to lead to a NULL pointer + * exception instead of UB. + */ + memset(job, 0, sizeof(*job)); + job->entity = entity; job->credits = credits; job->s_fence = drm_sched_fence_alloc(entity, owner);
drm_sched_job_init() has no control over how users allocate struct drm_sched_job. Unfortunately, the function can also not set some struct members such as job->sched. This could theoretically lead to UB by users dereferencing the struct's pointer members too early. It is easier to debug such issues if these pointers are initialized to NULL, so dereferencing them causes a NULL pointer exception. Accordingly, drm_sched_entity_init() does precisely that and initializes its struct with memset(). Initialize parameter "job" to 0 in drm_sched_job_init(). Signed-off-by: Philipp Stanner <pstanner@redhat.com> --- Hi all, I did some experiments with the scheduler recently and am trying to make the documentation and bits of the code more bullet proof. I tested the performance of v6.11-rc2 with and without this memset() by creating 1e6 jobs and found no performance regression. Cheers, P. --- drivers/gpu/drm/scheduler/sched_main.c | 8 ++++++++ 1 file changed, 8 insertions(+)