On Wed, 2024-09-11 at 17:30 -0700, Bart Van Assche wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> In ufshcd_uic_cmd_compl(), there is code that dereferences 'cmd'
> with
> and without checking the 'cmd' pointer. This confuses static source
> code
> analyzers like Coverity and sparse. Since none of the code in
> ufshcd_uic_cmd_compl() can do anything useful if 'cmd' is NULL, move
> the
> 'cmd' test near the start of this function.
>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> drivers/ufs/core/ufshcd.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 134cba0ff512..bd4ce3395972 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -5462,10 +5462,13 @@ static irqreturn_t
> ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
>
> spin_lock(hba->host->host_lock);
> cmd = hba->active_uic_cmd;
> + if (!cmd)
> + goto unlock;
> +
>
Hi Bart,
Could add a warning line in this case?
WARN_ON(!cmd);
I'm worried that if this situation occurs, we won't have
enough information to debug.
Thanks
Peter
> if (ufshcd_is_auto_hibern8_error(hba, intr_status))
> hba->errors |= (UFSHCD_UIC_HIBERN8_MASK & intr_status);
>
> - if (intr_status & UIC_COMMAND_COMPL && cmd) {
> + if (intr_status & UIC_COMMAND_COMPL) {
> cmd->argument2 |= ufshcd_get_uic_cmd_result(hba);
> cmd->argument3 = ufshcd_get_dme_attr_val(hba);
> if (!hba->uic_async_done)
> @@ -5482,7 +5485,10 @@ static irqreturn_t ufshcd_uic_cmd_compl(struct
> ufs_hba *hba, u32 intr_status)
>
> if (retval == IRQ_HANDLED)
> ufshcd_add_uic_command_trace(hba, cmd, UFS_CMD_COMP);
> +
> +unlock:
> spin_unlock(hba->host->host_lock);
> +
> return retval;
> }
>
