linux-kernel - Re: [PATCH] drm: mali-dp: Add check for kzalloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Y5HNv5BEEzB2Gt9W@e110455-lin.cambridge.arm.com>
Date:   Thu, 8 Dec 2022 11:42:55 +0000
From:   Liviu Dudau <liviu.dudau@....com>
To:     Jiasheng Jiang <jiasheng@...as.ac.cn>
Cc:     robin.murphy@....com, brian.starkey@....com, airlied@...il.com,
        daniel@...ll.ch, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] drm: mali-dp: Add check for kzalloc

Hi Jiasheng,

I appreciate the effort you have put into this and I find nothing wrong with the
intention of the patch. However, I don't intend to move base from being the first
member of the malidp_mw_connector_state struct as it has other benefits in the code
and we can use container_of() in implementation of generic APIs. As Robin has rightly
pointed, it is unlikely that a compiler will dereference the pointer before doing
offset_of(), so having mw_state == NULL is safe, even if quirky.

So I'm going to thank you for the patch but I will not merge it.

As a side comment, please use --in-reply-to and link to previous email when re-sending
patches with small spelling fixes as otherwise it gets confusing on which email is the last
one and it also relies on the servers delivering messages in the order you've sent,
not always a strong guarantee.

Best regards,
Liviu

On Thu, Dec 08, 2022 at 11:16:21AM +0800, Jiasheng Jiang wrote:
> As kzalloc may fail and return NULL pointer, the "mw_state" can be NULL.
> If the layout of struct malidp_mw_connector_state ever changes, it
> will cause NULL poineter derefernce of "&mw_state->base".
> Therefore, the "mw_state" should be checked whether it is NULL in order
> to improve the robust.
> 
> Fixes: 8cbc5caf36ef ("drm: mali-dp: Add writeback connector")
> Signed-off-by: Jiasheng Jiang <jiasheng@...as.ac.cn>
> ---
>  drivers/gpu/drm/arm/malidp_mw.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c
> index ef76d0e6ee2f..fe4474c2ddcf 100644
> --- a/drivers/gpu/drm/arm/malidp_mw.c
> +++ b/drivers/gpu/drm/arm/malidp_mw.c
> @@ -72,7 +72,11 @@ static void malidp_mw_connector_reset(struct drm_connector *connector)
>  		__drm_atomic_helper_connector_destroy_state(connector->state);
>  
>  	kfree(connector->state);
> -	__drm_atomic_helper_connector_reset(connector, &mw_state->base);
> +
> +	if (mw_state)
> +		__drm_atomic_helper_connector_reset(connector, &mw_state->base);
> +	else
> +		__drm_atomic_helper_connector_reset(connector, NULL);
>  }
>  
>  static enum drm_connector_status
> -- 
> 2.25.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯