Messages in this thread | | | Subject | Re: [PATCH v5 3/3] drm: protect drm_master pointers in drm_lease.c | From | Desmond Cheong Zhi Xi <> | Date | Wed, 30 Jun 2021 14:37:23 +0800 |
| |
On 30/6/21 8:16 am, Emil Velikov wrote: > Hi Desmond, > > Couple of small suggestions, with those the series is: > Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> > > On Tue, 29 Jun 2021 at 04:38, Desmond Cheong Zhi Xi > <desmondcheongzx@gmail.com> wrote: > >> @@ -128,13 +137,20 @@ bool drm_lease_held(struct drm_file *file_priv, int id) >> struct drm_master *master; >> bool ret; >> >> - if (!file_priv || !file_priv->master || !file_priv->master->lessor) >> + if (!file_priv) >> return true; >> >> - master = file_priv->master; >> + master = drm_file_get_master(file_priv); >> + if (master == NULL) >> + return true; >> + if (!master->lessor) { >> + drm_master_put(&master); >> + return true; > > Let's add a "ret = true; goto unlock;" here, so we can have a single > drm_master_put() in the function. > Nearly all code paths touched by this patch already follow this approach. > >> @@ -154,10 +170,16 @@ uint32_t drm_lease_filter_crtcs(struct drm_file *file_priv, uint32_t crtcs_in) >> int count_in, count_out; >> uint32_t crtcs_out = 0; >> >> - if (!file_priv || !file_priv->master || !file_priv->master->lessor) >> + if (!file_priv) >> return crtcs_in; >> >> - master = file_priv->master; >> + master = drm_file_get_master(file_priv); >> + if (master == NULL) >> + return crtcs_in; >> + if (!master->lessor) { >> + drm_master_put(&master); >> + return crtcs_in; > > Ditto > > Thanks > Emil >
Sounds good to me, I'll revise these functions. Thanks for the review and suggestions, Emil.
Best wishes, Desmond
| |