linux-kernel - Re: [PATCH v2] drm/komeda: Fix an undefined behavior bug in komeda_plane

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <Yad4l3HXBRavsE0y@e110455-lin.cambridge.arm.com>
Date:   Wed, 1 Dec 2021 13:28:55 +0000
From:   Liviu Dudau <liviu.dudau@....com>
To:     Zhou Qingyang <zhou1615@....edu>
Cc:     kjlu@....edu, "James (Qian) Wang" <james.qian.wang@....com>,
        Mihail Atanassov <mihail.atanassov@....com>,
        Brian Starkey <brian.starkey@....com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/komeda: Fix an undefined behavior bug in
 komeda_plane_add()

On Wed, Dec 01, 2021 at 11:37:03AM +0800, Zhou Qingyang wrote:
> In komeda_plane_add(), komeda_get_layer_fourcc_list() is assigned to
> formats and used in drm_universal_plane_init().
> drm_universal_plane_init() passes formats to
> __drm_universal_plane_init(). __drm_universal_plane_init() further
> passes formats to memcpy() as src parameter, which could lead to an
> undefined behavior bug on failure of komeda_get_layer_fourcc_list().
> 
> Fix this bug by adding a check of formats.
> 
> This bug was found by a static analyzer. The analysis employs
> differential checking to identify inconsistent security operations
> (e.g., checks or kfrees) between two code paths and confirms that the
> inconsistent operations are not recovered in the current function or
> the callers, so they constitute bugs.
> 
> Note that, as a bug found by static analysis, it can be a false
> positive or hard to trigger. Multiple researchers have cross-reviewed
> the bug.
> 
> Builds with CONFIG_DRM_KOMEDA=m show no new warnings,
> and our static analyzer no longer warns about this code.
> 
> Fixes: 61f1c4a8ab75 ("drm/komeda: Attach komeda_dev to DRM-KMS")
> Signed-off-by: Zhou Qingyang <zhou1615@....edu>

Thanks for the fix!

Reviewed-by: Liviu Dudau <liviu.dudau@....com>

Best regards,
Liviu

> ---
> Changes in v2
>   -  Use kfree and return instead of using 'goto'
> 
>  drivers/gpu/drm/arm/display/komeda/komeda_plane.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> index d63d83800a8a..aa193c58f4bf 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_plane.c
> @@ -265,6 +265,10 @@ static int komeda_plane_add(struct komeda_kms_dev *kms,
>  
>  	formats = komeda_get_layer_fourcc_list(&mdev->fmt_tbl,
>  					       layer->layer_type, &n_formats);
> +	if (!formats) {
> +		kfree(kplane);
> +		return -ENOMEM;
> +	}
>  
>  	err = drm_universal_plane_init(&kms->base, plane,
>  			get_possible_crtcs(kms, c->pipeline),
> -- 
> 2.25.1
> 

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