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!  /
  ---------------
    ¯\_(ツ)_/¯

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ