linux-kernel - Re: [PATCH v2] drm/msm: Fix objtool warning in submit_lock

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: <00f16170-93ca-4dac-a01f-4c5e0c60ff4c@paulmck-laptop>
Date: Thu, 14 Aug 2025 16:48:54 -0700
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Sasha Levin <sashal@...nel.org>
Cc: robin.clark@....qualcomm.com, lumag@...nel.org, abhinav.kumar@...ux.dev,
	jessica.zhang@....qualcomm.com, sean@...rly.run,
	marijn.suijten@...ainline.org, airlied@...il.com, simona@...ll.ch,
	antomani103@...il.com, linux-arm-msm@...r.kernel.org,
	dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/msm: Fix objtool warning in submit_lock_objects()

On Thu, Aug 07, 2025 at 09:10:58AM -0400, Sasha Levin wrote:
> Split the vmbind case into a separate helper function
> submit_lock_objects_vmbind() to fix objtool warning:
> 
>   drivers/gpu/drm/msm/msm.o: warning: objtool: submit_lock_objects+0x451:
>   sibling call from callable instruction with modified stack frame
> 
> The drm_exec_until_all_locked() macro uses computed gotos internally
> for its retry loop. Having return statements inside this macro, or
> immediately after it in certain code paths, confuses objtool's static
> analysis of stack frames, causing it to incorrectly flag tail call
> optimizations.
> 
> Fixes: 92395af63a99 ("drm/msm: Add VM_BIND submitqueue")
> Signed-off-by: Sasha Levin <sashal@...nel.org>

Tested-by: Paul E. McKenney <paulmck@...nel.org>

> ---
> 
> Changes since v1:
>  - Extract helper submit_lock_objects_vmbind() instead of refactoring
>    single loop
> 
>  drivers/gpu/drm/msm/msm_gem_submit.c | 49 +++++++++++++++-------------
>  1 file changed, 27 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 5f8e939a5906..1ce90e351b7a 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -271,32 +271,37 @@ static int submit_lookup_cmds(struct msm_gem_submit *submit,
>  	return ret;
>  }
>  
> -/* This is where we make sure all the bo's are reserved and pin'd: */
> -static int submit_lock_objects(struct msm_gem_submit *submit)
> +static int submit_lock_objects_vmbind(struct msm_gem_submit *submit)
>  {
> -	unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
> +	unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT | DRM_EXEC_IGNORE_DUPLICATES;
>  	struct drm_exec *exec = &submit->exec;
> -	int ret;
> +	int ret = 0;
>  
> -	if (msm_context_is_vmbind(submit->queue->ctx)) {
> -		flags |= DRM_EXEC_IGNORE_DUPLICATES;
> +	drm_exec_init(&submit->exec, flags, submit->nr_bos);
>  
> -		drm_exec_init(&submit->exec, flags, submit->nr_bos);
> +	drm_exec_until_all_locked (&submit->exec) {
> +		ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> +		drm_exec_retry_on_contention(exec);
> +		if (ret)
> +			break;
>  
> -		drm_exec_until_all_locked (&submit->exec) {
> -			ret = drm_gpuvm_prepare_vm(submit->vm, exec, 1);
> -			drm_exec_retry_on_contention(exec);
> -			if (ret)
> -				return ret;
> +		ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
> +		drm_exec_retry_on_contention(exec);
> +		if (ret)
> +			break;
> +	}
>  
> -			ret = drm_gpuvm_prepare_objects(submit->vm, exec, 1);
> -			drm_exec_retry_on_contention(exec);
> -			if (ret)
> -				return ret;
> -		}
> +	return ret;
> +}
>  
> -		return 0;
> -	}
> +/* This is where we make sure all the bo's are reserved and pin'd: */
> +static int submit_lock_objects(struct msm_gem_submit *submit)
> +{
> +	unsigned flags = DRM_EXEC_INTERRUPTIBLE_WAIT;
> +	int ret = 0;
> +
> +	if (msm_context_is_vmbind(submit->queue->ctx))
> +		return submit_lock_objects_vmbind(submit);
>  
>  	drm_exec_init(&submit->exec, flags, submit->nr_bos);
>  
> @@ -305,17 +310,17 @@ static int submit_lock_objects(struct msm_gem_submit *submit)
>  					drm_gpuvm_resv_obj(submit->vm));
>  		drm_exec_retry_on_contention(&submit->exec);
>  		if (ret)
> -			return ret;
> +			break;
>  		for (unsigned i = 0; i < submit->nr_bos; i++) {
>  			struct drm_gem_object *obj = submit->bos[i].obj;
>  			ret = drm_exec_prepare_obj(&submit->exec, obj, 1);
>  			drm_exec_retry_on_contention(&submit->exec);
>  			if (ret)
> -				return ret;
> +				break;
>  		}
>  	}
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int submit_fence_sync(struct msm_gem_submit *submit)
> -- 
> 2.39.5
>