linux-kernel - Re: [PATCH] staging: media: atomisp: Fix premature setting of HMM_BO_DEVICE

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <fe47fcf0-9d37-451d-8985-9d10a7491eeb@kernel.org>
Date: Sun, 6 Jul 2025 18:22:59 +0200
From: Hans de Goede <hansg@...nel.org>
To: Abdelrahman Fekry <abdelrahmanfekry375@...il.com>, andy@...nel.org,
 mchehab@...nel.org, sakari.ailus@...ux.intel.com, gregkh@...uxfoundation.org
Cc: linux-kernel-mentees@...ts.linux.dev, linux-kernel@...r.kernel.org,
 linux-media@...r.kernel.org, linux-staging@...ts.linux.dev,
 skhan@...uxfoundation.com, dan.carpenter@...aro.org
Subject: Re: [PATCH] staging: media: atomisp: Fix premature setting of
 HMM_BO_DEVICE_INITED flag

Hi,

On 28-Jun-25 7:25 AM, Abdelrahman Fekry wrote:
> The HMM_BO_DEVICE_INITED flag was being set in hmm_bo_device_init()
> before key initialization steps like kmem_cache_create(),
> kmem_cache_alloc(), and __bo_init().
> 
> This means that if any of these steps fail, the flag remains set,
> misleading other parts of the driver (e.g. hmm_bo_alloc())
> into thinking the device is initialized. This could lead
> to undefined behavior or invalid memory use.
> 
> Additionally, since __bo_init() is called from inside
> hmm_bo_device_init() after the flag was already set, its internal
> check for HMM_BO_DEVICE_INITED is redundant.
> 
> - Move the flag assignment to the end after all allocations succeed.
> - Remove redundant check of the flag inside __bo_init()
> 
> Signed-off-by: Abdelrahman Fekry <abdelrahmanfekry375@...il.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@...nel.org>

I have adjusted the commit message with a link to the backtrace
you generated through fault-injection and I've merged this in my
media-atomisp branch:

https://git.kernel.org/pub/scm/linux/kernel/git/hansg/linux.git/log/?h=media-atomisp

This patch will be included in my next pull-request
to Mauro (to media subsystem maintainer)

Regards,

Hans




> ---
>  drivers/staging/media/atomisp/pci/hmm/hmm_bo.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
> index 224ca8d42721..5d0cd5260d3a 100644
> --- a/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
> +++ b/drivers/staging/media/atomisp/pci/hmm/hmm_bo.c
> @@ -37,8 +37,6 @@ static int __bo_init(struct hmm_bo_device *bdev, struct hmm_buffer_object *bo,
>  		     unsigned int pgnr)
>  {
>  	check_bodev_null_return(bdev, -EINVAL);
> -	var_equal_return(hmm_bo_device_inited(bdev), 0, -EINVAL,
> -			 "hmm_bo_device not inited yet.\n");
>  	/* prevent zero size buffer object */
>  	if (pgnr == 0) {
>  		dev_err(atomisp_dev, "0 size buffer is not allowed.\n");
> @@ -341,7 +339,6 @@ int hmm_bo_device_init(struct hmm_bo_device *bdev,
>  	spin_lock_init(&bdev->list_lock);
>  	mutex_init(&bdev->rbtree_mutex);
> 
> -	bdev->flag = HMM_BO_DEVICE_INITED;
> 
>  	INIT_LIST_HEAD(&bdev->entire_bo_list);
>  	bdev->allocated_rbtree = RB_ROOT;
> @@ -376,6 +373,8 @@ int hmm_bo_device_init(struct hmm_bo_device *bdev,
> 
>  	__bo_insert_to_free_rbtree(&bdev->free_rbtree, bo);
> 
> +	bdev->flag = HMM_BO_DEVICE_INITED;
> +
>  	return 0;
>  }
> 
> --
> 2.25.1
> 
>