drm_edid-load: Fix a missing-check bug in drivers/gpu/drm/drm_edid_load.c

Submitted by Gen Zhang on May 22, 2019, 12:39 p.m.

Details

Message ID 20190522123920.GB6772@zhanggen-UX430UQ
State New
Headers show
Series "drm_edid-load: Fix a missing-check bug in drivers/gpu/drm/drm_edid_load.c" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Gen Zhang May 22, 2019, 12:39 p.m.
In drm_load_edid_firmware(), fwstr is allocated by kstrdup(). And fwstr
is dereferenced in the following codes. However, memory allocation 
functions such as kstrdup() may fail and returns NULL. Dereferencing 
this null pointer may cause the kernel go wrong. Thus we should check 
this kstrdup() operation.
Further, if kstrdup() returns NULL, we should return ERR_PTR(-ENOMEM) to
the caller site.

Signed-off-by: Gen Zhang <blackgod016574@gmail.com>

---
---

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index a491509..a0e107a 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -290,6 +290,8 @@  struct edid *drm_load_edid_firmware(struct drm_connector *connector)
 	 * the last one found one as a fallback.
 	 */
 	fwstr = kstrdup(edid_firmware, GFP_KERNEL);
+	if (!fwstr)
+		return ERR_PTR(-ENOMEM);
 	edidstr = fwstr;
 
 	while ((edidname = strsep(&edidstr, ","))) {

Comments

On Wed, 22 May 2019, Gen Zhang <blackgod016574@gmail.com> wrote:
> In drm_load_edid_firmware(), fwstr is allocated by kstrdup(). And fwstr
> is dereferenced in the following codes. However, memory allocation 
> functions such as kstrdup() may fail and returns NULL. Dereferencing 
> this null pointer may cause the kernel go wrong. Thus we should check 
> this kstrdup() operation.
> Further, if kstrdup() returns NULL, we should return ERR_PTR(-ENOMEM) to
> the caller site.

strsep() handles the NULL pointer just fine, so there won't be a NULL
dereference. However this patch seems like the right thing to do anyway.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
>
> ---
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index a491509..a0e107a 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -290,6 +290,8 @@ struct edid *drm_load_edid_firmware(struct drm_connector *connector)
>  	 * the last one found one as a fallback.
>  	 */
>  	fwstr = kstrdup(edid_firmware, GFP_KERNEL);
> +	if (!fwstr)
> +		return ERR_PTR(-ENOMEM);
>  	edidstr = fwstr;
>  
>  	while ((edidname = strsep(&edidstr, ","))) {
> ---
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, 24 May 2019, Gen Zhang <blackgod016574@gmail.com> wrote:
> In drm_load_edid_firmware(), fwstr is allocated by kstrdup(). And fwstr
> is dereferenced in the following codes. However, memory allocation 
> functions such as kstrdup() may fail and returns NULL. Dereferencing 
> this null pointer may cause the kernel go wrong. Thus we should check 
> this kstrdup() operation.
> Further, if kstrdup() returns NULL, we should return ERR_PTR(-ENOMEM) to
> the caller site.
>
> Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>

Pushed to drm-misc-next, thanks for the patch.

BR,
Jani.

> ---
> diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
> index a491509..a0e107a 100644
> --- a/drivers/gpu/drm/drm_edid_load.c
> +++ b/drivers/gpu/drm/drm_edid_load.c
> @@ -290,6 +290,8 @@ struct edid *drm_load_edid_firmware(struct drm_connector *connector)
>  	 * the last one found one as a fallback.
>  	 */
>  	fwstr = kstrdup(edid_firmware, GFP_KERNEL);
> +	if (!fwstr)
> +		return ERR_PTR(-ENOMEM);
>  	edidstr = fwstr;
>  
>  	while ((edidname = strsep(&edidstr, ","))) {
> ---