| Message ID | 20240710134130.17292-5-tursulin@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | v3d: Perfmon cleanup | expand |
On 7/10/24 10:41, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > > If userspace provides an unknown or invalid handle anywhere in the handle > array the rest of the driver will not handle that well. > > Fix it by checking handle was looked up successfuly or otherwise fail the I believe you mean "Fix it by checking if the handle..." Also s/successfuly/successfully > extension by jumping into the existing unwind. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> > Fixes: 9ba0ff3e083f ("drm/v3d: Create a CPU job extension for the timestamp query job") > Cc: Maíra Canal <mcanal@igalia.com> > Cc: Iago Toral Quiroga <itoral@igalia.com> > Cc: <stable@vger.kernel.org> # v6.8+ > --- > drivers/gpu/drm/v3d/v3d_submit.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c > index ca1b1ad0a75c..3313423080e7 100644 > --- a/drivers/gpu/drm/v3d/v3d_submit.c > +++ b/drivers/gpu/drm/v3d/v3d_submit.c > @@ -497,6 +497,10 @@ v3d_get_cpu_timestamp_query_params(struct drm_file *file_priv, > } > > job->timestamp_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync); > + if (!job->timestamp_query.queries[i].syncobj) { > + err = -ENOENT; I'm not sure if err should be -ENOENT or -EINVAL, but based on other drivers, I believe it should be -EINVAL. Best Regards, - Maíra > + goto error; > + } > } > job->timestamp_query.count = timestamp.count; > > @@ -550,6 +554,10 @@ v3d_get_cpu_reset_timestamp_params(struct drm_file *file_priv, > } > > job->timestamp_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync); > + if (!job->timestamp_query.queries[i].syncobj) { > + err = -ENOENT; > + goto error; > + } > } > job->timestamp_query.count = reset.count; > > @@ -613,6 +621,10 @@ v3d_get_cpu_copy_query_results_params(struct drm_file *file_priv, > } > > job->timestamp_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync); > + if (!job->timestamp_query.queries[i].syncobj) { > + err = -ENOENT; > + goto error; > + } > } > job->timestamp_query.count = copy.count; >
On 10/07/2024 18:06, Maíra Canal wrote: > On 7/10/24 10:41, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> >> >> If userspace provides an unknown or invalid handle anywhere in the handle >> array the rest of the driver will not handle that well. >> >> Fix it by checking handle was looked up successfuly or otherwise fail the > > I believe you mean "Fix it by checking if the handle..." > > Also s/successfuly/successfully Oops, thank you! > >> extension by jumping into the existing unwind. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> >> Fixes: 9ba0ff3e083f ("drm/v3d: Create a CPU job extension for the >> timestamp query job") >> Cc: Maíra Canal <mcanal@igalia.com> >> Cc: Iago Toral Quiroga <itoral@igalia.com> >> Cc: <stable@vger.kernel.org> # v6.8+ >> --- >> drivers/gpu/drm/v3d/v3d_submit.c | 12 ++++++++++++ >> 1 file changed, 12 insertions(+) >> >> diff --git a/drivers/gpu/drm/v3d/v3d_submit.c >> b/drivers/gpu/drm/v3d/v3d_submit.c >> index ca1b1ad0a75c..3313423080e7 100644 >> --- a/drivers/gpu/drm/v3d/v3d_submit.c >> +++ b/drivers/gpu/drm/v3d/v3d_submit.c >> @@ -497,6 +497,10 @@ v3d_get_cpu_timestamp_query_params(struct >> drm_file *file_priv, >> } >> job->timestamp_query.queries[i].syncobj = >> drm_syncobj_find(file_priv, sync); >> + if (!job->timestamp_query.queries[i].syncobj) { >> + err = -ENOENT; > > I'm not sure if err should be -ENOENT or -EINVAL, but based on other > drivers, I believe it should be -EINVAL. After a quick grep I am inclined to think ENOENT is correct. DRM core uses that, and drivers seem generally confused (split between ENOENT and EINVAL). With one even going for ENODEV! Regards, Tvrtko >> + goto error; >> + } >> } >> job->timestamp_query.count = timestamp.count; >> @@ -550,6 +554,10 @@ v3d_get_cpu_reset_timestamp_params(struct >> drm_file *file_priv, >> } >> job->timestamp_query.queries[i].syncobj = >> drm_syncobj_find(file_priv, sync); >> + if (!job->timestamp_query.queries[i].syncobj) { >> + err = -ENOENT; >> + goto error; >> + } >> } >> job->timestamp_query.count = reset.count; >> @@ -613,6 +621,10 @@ v3d_get_cpu_copy_query_results_params(struct >> drm_file *file_priv, >> } >> job->timestamp_query.queries[i].syncobj = >> drm_syncobj_find(file_priv, sync); >> + if (!job->timestamp_query.queries[i].syncobj) { >> + err = -ENOENT; >> + goto error; >> + } >> } >> job->timestamp_query.count = copy.count;
diff --git a/drivers/gpu/drm/v3d/v3d_submit.c b/drivers/gpu/drm/v3d/v3d_submit.c index ca1b1ad0a75c..3313423080e7 100644 --- a/drivers/gpu/drm/v3d/v3d_submit.c +++ b/drivers/gpu/drm/v3d/v3d_submit.c @@ -497,6 +497,10 @@ v3d_get_cpu_timestamp_query_params(struct drm_file *file_priv, } job->timestamp_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync); + if (!job->timestamp_query.queries[i].syncobj) { + err = -ENOENT; + goto error; + } } job->timestamp_query.count = timestamp.count; @@ -550,6 +554,10 @@ v3d_get_cpu_reset_timestamp_params(struct drm_file *file_priv, } job->timestamp_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync); + if (!job->timestamp_query.queries[i].syncobj) { + err = -ENOENT; + goto error; + } } job->timestamp_query.count = reset.count; @@ -613,6 +621,10 @@ v3d_get_cpu_copy_query_results_params(struct drm_file *file_priv, } job->timestamp_query.queries[i].syncobj = drm_syncobj_find(file_priv, sync); + if (!job->timestamp_query.queries[i].syncobj) { + err = -ENOENT; + goto error; + } } job->timestamp_query.count = copy.count;