Re: [PATCH] staging: media: atomisp: Fix premature setting of HMM_BO_DEVICE_INITED flag

[Hans de Goede <hansg@kernel.org>]

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@gmail.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.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
> 
>