| Message ID | 20230215235048.1166484-1-robdclark@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm: Fix potential invalid ptr free | expand |
On 16/02/2023 01:50, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > The error path cleanup expects that chain and syncobj are either NULL or > valid pointers. But post_deps was not allocated with __GFP_ZERO. > > Fixes: ab723b7a992a ("drm/msm: Add syncobj support.") > Signed-off-by: Rob Clark <robdclark@chromium.org> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> This makes me want to rewrite the ab723b7a992a ("drm/msm: Add syncobj support.") in the usual explicit-error-path way. WDYT? > --- > drivers/gpu/drm/msm/msm_gem_submit.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c > index 6503220e5a4b..e4d13540300e 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -640,8 +640,8 @@ static struct msm_submit_post_dep *msm_parse_post_deps(struct drm_device *dev, > int ret = 0; > uint32_t i, j; > > - post_deps = kmalloc_array(nr_syncobjs, sizeof(*post_deps), > - GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY); > + post_deps = kcalloc(nr_syncobjs, sizeof(*post_deps), > + GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY); > if (!post_deps) > return ERR_PTR(-ENOMEM); > > @@ -656,7 +656,6 @@ static struct msm_submit_post_dep *msm_parse_post_deps(struct drm_device *dev, > } > > post_deps[i].point = syncobj_desc.point; > - post_deps[i].chain = NULL; > > if (syncobj_desc.flags) { > ret = -EINVAL;
On 2/16/23 02:50, Rob Clark wrote: > From: Rob Clark <robdclark@chromium.org> > > The error path cleanup expects that chain and syncobj are either NULL or > valid pointers. But post_deps was not allocated with __GFP_ZERO. > > Fixes: ab723b7a992a ("drm/msm: Add syncobj support.") > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > drivers/gpu/drm/msm/msm_gem_submit.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c > index 6503220e5a4b..e4d13540300e 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -640,8 +640,8 @@ static struct msm_submit_post_dep *msm_parse_post_deps(struct drm_device *dev, > int ret = 0; > uint32_t i, j; > > - post_deps = kmalloc_array(nr_syncobjs, sizeof(*post_deps), > - GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY); > + post_deps = kcalloc(nr_syncobjs, sizeof(*post_deps), > + GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY); > if (!post_deps) > return ERR_PTR(-ENOMEM); > > @@ -656,7 +656,6 @@ static struct msm_submit_post_dep *msm_parse_post_deps(struct drm_device *dev, > } > > post_deps[i].point = syncobj_desc.point; > - post_deps[i].chain = NULL; > > if (syncobj_desc.flags) { > ret = -EINVAL; Good catch! Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 6503220e5a4b..e4d13540300e 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -640,8 +640,8 @@ static struct msm_submit_post_dep *msm_parse_post_deps(struct drm_device *dev, int ret = 0; uint32_t i, j; - post_deps = kmalloc_array(nr_syncobjs, sizeof(*post_deps), - GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY); + post_deps = kcalloc(nr_syncobjs, sizeof(*post_deps), + GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY); if (!post_deps) return ERR_PTR(-ENOMEM); @@ -656,7 +656,6 @@ static struct msm_submit_post_dep *msm_parse_post_deps(struct drm_device *dev, } post_deps[i].point = syncobj_desc.point; - post_deps[i].chain = NULL; if (syncobj_desc.flags) { ret = -EINVAL;