[lvc-project] [PATCH] drm/komeda: fix integer overflow in AFBC framebuffer size check

[Fedor Pchelkin]

On Tue, 03. Feb 16:48, Alexander Konyukhov wrote:
> The AFBC framebuffer size validation calculates the minimum required
> buffer size by adding the AFBC payload size to the framebuffer offset.
> This addition is performed without checking for integer overflow.
> 
> If the addition oveflows, the size check may incorrectly succed and
> allow userspace to provide an undersized drm_gem_object, potentially
> leading to out-of-bounds memory access.
> 
> Add usage of check_add_overflow() to safely compute the minimum
> required size and reject the framebuffer if an overflow is detected.
> This makes the AFBC size validation more robust against malformed.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 65ad2392dd6d ("drm/komeda: Added AFBC support for komeda driver")
> Signed-off-by: Alexander Konyukhov <Alexander.Konyukhov at kaspersky.com>
> ---
>  drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> index 3ca461eb0a24..3cb34d03f7f8 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
> @@ -4,6 +4,8 @@
>   * Author: James.Qian.Wang <james.qian.wang at arm.com>
>   *
>   */
> +#include <linux/overflow.h>
> +
>  #include <drm/drm_device.h>
>  #include <drm/drm_fb_dma_helper.h>
>  #include <drm/drm_gem.h>
> @@ -93,7 +95,9 @@ komeda_fb_afbc_size_check(struct komeda_fb *kfb, struct drm_file *file,
>  	kfb->afbc_size = kfb->offset_payload + n_blocks *
>  			 ALIGN(bpp * AFBC_SUPERBLK_PIXELS / 8,
>  			       AFBC_SUPERBLK_ALIGNMENT);
> -	min_size = kfb->afbc_size + fb->offsets[0];
> +	if (check_add_overflow(kfb->afbc_size, fb->offsets[0], &min_size)) {
> +		goto check_failed;
> +	}

nit: extra braces around single-statement if-block are not needed per
kernel's coding style.

Another option is to cast one of the operands to u64 type and so perform
the addition in u64 and then proceed to the `min_size > obj->size` check
below.  Otherwise with the current patch it's pointless to declare
min_size as u64 - why u64 if its value is only allowed to be in u32 range
with the new check?

I think casting would probably be more appropriate here though it's up
to you to decide, thanks.

--
Fedor

>  	if (min_size > obj->size) {
>  		DRM_DEBUG_KMS("afbc size check failed, obj_size: 0x%zx. min_size 0x%llx.\n",
>  			      obj->size, min_size);
> -- 
> 2.43.0