From: Thomas Zimmermann <tzimmermann@suse.de> To: Kevin Tang <kevin3.tang@gmail.com>, maarten.lankhorst@linux.intel.com, mripard@kernel.org, sean@poorly.run, airlied@linux.ie, daniel@ffwll.ch, robh+dt@kernel.org, mark.rutland@arm.com Cc: devicetree@vger.kernel.org, zhang.lyra@gmail.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, lukas.bulwahn@gmail.com, orsonzhai@gmail.com, zou_wei@huawei.com, pony1.wu@gmail.com, dan.carpenter@oracle.com Subject: Re: [PATCH v1 2/2] drm/sprd: fix potential NULL dereference Date: Tue, 11 Jan 2022 10:31:17 +0100 [thread overview] Message-ID: <0963819e-6739-2ad9-c5d8-dc3529032d22@suse.de> (raw) In-Reply-To: <20211224141213.27612-3-kevin3.tang@gmail.com> [-- Attachment #1.1: Type: text/plain, Size: 3133 bytes --] Hi, on the changes for platform_get_resource(), you can Acked-by: Thomas Zimmermann <tzimmermann@suse.de> but see my comments below. Am 24.12.21 um 15:12 schrieb Kevin Tang: > platform_get_resource() may fail and return NULL, so check it's value > before using it. > > 'drm' could be null in sprd_drm_shutdown, and drm_warn maybe dereference > it, remove this warning log. > > Cc: Orson Zhai <orsonzhai@gmail.com> > Cc: Chunyan Zhang <zhang.lyra@gmail.com> > Signed-off-by: Kevin Tang <kevin.tang@unisoc.com> > --- > drivers/gpu/drm/sprd/sprd_dpu.c | 3 +++ > drivers/gpu/drm/sprd/sprd_drm.c | 8 ++------ > drivers/gpu/drm/sprd/sprd_dsi.c | 3 +++ > 3 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/sprd/sprd_dpu.c b/drivers/gpu/drm/sprd/sprd_dpu.c > index 06a3414ee..69683b7ba 100644 > --- a/drivers/gpu/drm/sprd/sprd_dpu.c > +++ b/drivers/gpu/drm/sprd/sprd_dpu.c > @@ -790,6 +790,9 @@ static int sprd_dpu_context_init(struct sprd_dpu *dpu, > int ret; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -EINVAL; > + You can add an error message if this fails. > ctx->base = devm_ioremap(dev, res->start, resource_size(res)); > if (!ctx->base) { > dev_err(dev, "failed to map dpu registers\n"); > diff --git a/drivers/gpu/drm/sprd/sprd_drm.c b/drivers/gpu/drm/sprd/sprd_drm.c > index a077e2d4d..54030839e 100644 > --- a/drivers/gpu/drm/sprd/sprd_drm.c > +++ b/drivers/gpu/drm/sprd/sprd_drm.c > @@ -154,12 +154,8 @@ static void sprd_drm_shutdown(struct platform_device *pdev) > { > struct drm_device *drm = platform_get_drvdata(pdev); > > - if (!drm) { > - drm_warn(drm, "drm device is not available, no shutdown\n"); > - return; > - } > - > - drm_atomic_helper_shutdown(drm); > + if (drm) > + drm_atomic_helper_shutdown(drm); This change should be in a separate patch. Instead of removing the warning, you should rather use dev_err() or dev_warn() from [1]. Not being able to shut down here looks like a serious driver bug that the user should know about. > } > > static const struct of_device_id drm_match_table[] = { > diff --git a/drivers/gpu/drm/sprd/sprd_dsi.c b/drivers/gpu/drm/sprd/sprd_dsi.c > index 911b3cddc..955c5995a 100644 > --- a/drivers/gpu/drm/sprd/sprd_dsi.c > +++ b/drivers/gpu/drm/sprd/sprd_dsi.c > @@ -907,6 +907,9 @@ static int sprd_dsi_context_init(struct sprd_dsi *dsi, > struct resource *res; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -EINVAL; > + Again, an error message seems appropriate. Best regards Thomas [1] https://elixir.bootlin.com/linux/latest/source/include/linux/dev_printk.h#L145 > ctx->base = devm_ioremap(dev, res->start, resource_size(res)); > if (!ctx->base) { > drm_err(dsi->drm, "failed to map dsi host registers\n"); -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Zimmermann <tzimmermann@suse.de> To: Kevin Tang <kevin3.tang@gmail.com>, maarten.lankhorst@linux.intel.com, mripard@kernel.org, sean@poorly.run, airlied@linux.ie, daniel@ffwll.ch, robh+dt@kernel.org, mark.rutland@arm.com Cc: devicetree@vger.kernel.org, zhang.lyra@gmail.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, orsonzhai@gmail.com, lukas.bulwahn@gmail.com, zou_wei@huawei.com, pony1.wu@gmail.com, dan.carpenter@oracle.com Subject: Re: [PATCH v1 2/2] drm/sprd: fix potential NULL dereference Date: Tue, 11 Jan 2022 10:31:17 +0100 [thread overview] Message-ID: <0963819e-6739-2ad9-c5d8-dc3529032d22@suse.de> (raw) In-Reply-To: <20211224141213.27612-3-kevin3.tang@gmail.com> [-- Attachment #1.1: Type: text/plain, Size: 3133 bytes --] Hi, on the changes for platform_get_resource(), you can Acked-by: Thomas Zimmermann <tzimmermann@suse.de> but see my comments below. Am 24.12.21 um 15:12 schrieb Kevin Tang: > platform_get_resource() may fail and return NULL, so check it's value > before using it. > > 'drm' could be null in sprd_drm_shutdown, and drm_warn maybe dereference > it, remove this warning log. > > Cc: Orson Zhai <orsonzhai@gmail.com> > Cc: Chunyan Zhang <zhang.lyra@gmail.com> > Signed-off-by: Kevin Tang <kevin.tang@unisoc.com> > --- > drivers/gpu/drm/sprd/sprd_dpu.c | 3 +++ > drivers/gpu/drm/sprd/sprd_drm.c | 8 ++------ > drivers/gpu/drm/sprd/sprd_dsi.c | 3 +++ > 3 files changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/sprd/sprd_dpu.c b/drivers/gpu/drm/sprd/sprd_dpu.c > index 06a3414ee..69683b7ba 100644 > --- a/drivers/gpu/drm/sprd/sprd_dpu.c > +++ b/drivers/gpu/drm/sprd/sprd_dpu.c > @@ -790,6 +790,9 @@ static int sprd_dpu_context_init(struct sprd_dpu *dpu, > int ret; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -EINVAL; > + You can add an error message if this fails. > ctx->base = devm_ioremap(dev, res->start, resource_size(res)); > if (!ctx->base) { > dev_err(dev, "failed to map dpu registers\n"); > diff --git a/drivers/gpu/drm/sprd/sprd_drm.c b/drivers/gpu/drm/sprd/sprd_drm.c > index a077e2d4d..54030839e 100644 > --- a/drivers/gpu/drm/sprd/sprd_drm.c > +++ b/drivers/gpu/drm/sprd/sprd_drm.c > @@ -154,12 +154,8 @@ static void sprd_drm_shutdown(struct platform_device *pdev) > { > struct drm_device *drm = platform_get_drvdata(pdev); > > - if (!drm) { > - drm_warn(drm, "drm device is not available, no shutdown\n"); > - return; > - } > - > - drm_atomic_helper_shutdown(drm); > + if (drm) > + drm_atomic_helper_shutdown(drm); This change should be in a separate patch. Instead of removing the warning, you should rather use dev_err() or dev_warn() from [1]. Not being able to shut down here looks like a serious driver bug that the user should know about. > } > > static const struct of_device_id drm_match_table[] = { > diff --git a/drivers/gpu/drm/sprd/sprd_dsi.c b/drivers/gpu/drm/sprd/sprd_dsi.c > index 911b3cddc..955c5995a 100644 > --- a/drivers/gpu/drm/sprd/sprd_dsi.c > +++ b/drivers/gpu/drm/sprd/sprd_dsi.c > @@ -907,6 +907,9 @@ static int sprd_dsi_context_init(struct sprd_dsi *dsi, > struct resource *res; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) > + return -EINVAL; > + Again, an error message seems appropriate. Best regards Thomas [1] https://elixir.bootlin.com/linux/latest/source/include/linux/dev_printk.h#L145 > ctx->base = devm_ioremap(dev, res->start, resource_size(res)); > if (!ctx->base) { > drm_err(dsi->drm, "failed to map dsi host registers\n"); -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --]
next prev parent reply other threads:[~2022-01-11 9:31 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-12-24 14:12 [PATCH v1 0/2] sprd drm cover letter Kevin Tang 2021-12-24 14:12 ` Kevin Tang 2021-12-24 14:12 ` [PATCH v1 1/2] drm/sprd: remove the selected DRM_KMS_CMA_HELPER in kconfig Kevin Tang 2021-12-24 14:12 ` Kevin Tang 2022-01-11 9:11 ` Javier Martinez Canillas 2022-01-11 9:11 ` Javier Martinez Canillas 2022-01-11 9:20 ` Thomas Zimmermann 2022-01-11 9:20 ` Thomas Zimmermann 2022-01-11 9:24 ` Lukas Bulwahn 2022-01-11 9:24 ` Lukas Bulwahn 2021-12-24 14:12 ` [PATCH v1 2/2] drm/sprd: fix potential NULL dereference Kevin Tang 2021-12-24 14:12 ` Kevin Tang 2022-01-11 9:18 ` Javier Martinez Canillas 2022-01-11 9:18 ` Javier Martinez Canillas 2022-01-11 9:31 ` Thomas Zimmermann [this message] 2022-01-11 9:31 ` Thomas Zimmermann
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=0963819e-6739-2ad9-c5d8-dc3529032d22@suse.de \ --to=tzimmermann@suse.de \ --cc=airlied@linux.ie \ --cc=dan.carpenter@oracle.com \ --cc=daniel@ffwll.ch \ --cc=devicetree@vger.kernel.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=kevin3.tang@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=lukas.bulwahn@gmail.com \ --cc=maarten.lankhorst@linux.intel.com \ --cc=mark.rutland@arm.com \ --cc=mripard@kernel.org \ --cc=orsonzhai@gmail.com \ --cc=pony1.wu@gmail.com \ --cc=robh+dt@kernel.org \ --cc=sean@poorly.run \ --cc=zhang.lyra@gmail.com \ --cc=zou_wei@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.