On Mon, 2024-10-21 at 15:05 +0200, Christian König wrote: > Am 21.10.24 um 12:50 schrieb Philipp Stanner: > > 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@xxxxxxxxxx> > > --- > > No changes in v2. > > > > +CC Christian and Tvrtko in this thread. > > Would be cool if someone can do a review. > > --- > > 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 dab8cca79eb7..2e0e5a9577d1 100644 > > --- a/drivers/gpu/drm/scheduler/sched_main.c > > +++ b/drivers/gpu/drm/scheduler/sched_main.c > > @@ -796,6 +796,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)); > > + > > Maybe just implicitly set the sched pointer to NULL here? > > On the other hand compilers these days are really good at optimizing > that away anyway, so feel free to add Reviewed-by: Christian König > <christian.koenig@xxxxxxx> to the series as is as well. (I had performance-tested it with several million jobs and couldn't detect a performance regression that was measurable) Applied #1 to drm-misc-next, thanks. Regarding patch #2, I just noticed that it violates the docstring style. I therefore hereby reject my own patch and will resubmit it in a cleaner form ^^' P. > > Regards, > Christian. > > > job->entity = entity; > > job->credits = credits; > > job->s_fence = drm_sched_fence_alloc(entity, owner); >
