linux-kernel - RE: [PATCH V1] scsi: ufs-bsg: complete ufs-bsg job only if no error

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

Message-ID: <SN6PR04MB4925EDC53B49B2E5DDD9368DFCE10@SN6PR04MB4925.namprd04.prod.outlook.com>
Date:   Sun, 23 Jun 2019 07:45:48 +0000
From:   Avri Altman <Avri.Altman@....com>
To:     "Bean Huo (beanhuo)" <beanhuo@...ron.com>,
        Alim Akhtar <alim.akhtar@...sung.com>,
        Pedro Sousa <pedrom.sousa@...opsys.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>
CC:     "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        Evan Green <evgreen@...omium.org>,
        Stanley Chu <stanley.chu@...iatek.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>
Subject: RE: [PATCH V1] scsi: ufs-bsg: complete ufs-bsg job only if no error

Hi Bean,
Thank you for fixing this.

> From: Bean Huo <beanhuo@...ron.com>
> 
> In the case of UPIU/DME request execution failed in UFS device,
> ufs_bsg_request() will complete this failed bsg job by calling
> bsg_job_done(). Meanwhile, it returns this error status to blk-mq
> layer, then trigger blk-mq complete this request again, this will
> cause below panic.
> 
> [   68.673050] Call trace:
> [   68.675491]  __ll_sc___cmpxchg_case_acq_32+0x4/0x20
> [   68.680369]  complete+0x28/0x70
> [   68.683510]  blk_end_sync_rq+0x24/0x30
> [   68.687255]  blk_mq_end_request+0xb8/0x118
> [   68.691350]  bsg_job_put+0x4c/0x58
> [   68.694747]  bsg_complete+0x20/0x30
> [   68.698231]  blk_done_softirq+0xb4/0xe8
> [   68.702066]  __do_softirq+0x154/0x3f0
> [   68.705726]  run_ksoftirqd+0x4c/0x68
> [   68.709298]  smpboot_thread_fn+0x22c/0x268
> [   68.713394]  kthread+0x130/0x138
> [   68.716619]  ret_from_fork+0x10/0x1c
> [   68.720193] Code: f84107fe d65f03c0 d503201f f9800011 (885ffc10)
> [   68.726298] ---[ end trace d92825bff6326e66 ]---
> [   68.730913] Kernel panic - not syncing: Fatal exception in interrupt
Please add a 'fixes:' tag

 
> This patch is to fix this issue. The solution is we complete
> the ufs-bsg job only if no error happened.
> 
> Signed-off-by: Bean Huo <beanhuo@...ron.com>
> ---
>  drivers/scsi/ufs/ufs_bsg.c | 7 ++++---
>  drivers/scsi/ufs/ufshcd.c  | 2 +-
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/ufs/ufs_bsg.c b/drivers/scsi/ufs/ufs_bsg.c
> index 869e71f..d5516dc 100644
> --- a/drivers/scsi/ufs/ufs_bsg.c
> +++ b/drivers/scsi/ufs/ufs_bsg.c
> @@ -122,7 +122,7 @@ static int ufs_bsg_request(struct bsg_job *job)
>  		memcpy(&uc, &bsg_request->upiu_req.uc, UIC_CMD_SIZE);
>  		ret = ufshcd_send_uic_cmd(hba, &uc);
>  		if (ret)
> -			dev_dbg(hba->dev,
> +			dev_err(hba->dev,
>  				"send uic cmd: error code %d\n", ret);
Please send a different patch fixing typos (also in ufshcd_uic_change_pwr_mode)
> 
>  		memcpy(&bsg_reply->upiu_rsp.uc, &uc, UIC_CMD_SIZE);
> @@ -143,13 +143,14 @@ static int ufs_bsg_request(struct bsg_job *job)
>  			sg_copy_from_buffer(job->request_payload.sg_list,
>  					    job->request_payload.sg_cnt,
>  					    desc_buff, desc_len);
> -
Keep this one line space please

>  	kfree(desc_buff);
> 
>  out:
>  	bsg_reply->result = ret;
>  	job->reply_len = sizeof(struct ufs_bsg_reply);
> -	bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
> +	/* complete the job here only if no error */
> +	if (ret == 0)
> +		bsg_job_done(job, ret, bsg_reply->reply_payload_rcv_len);
> 
>  	return ret;
>  }
> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> index 04d3686..4718041 100644
> --- a/drivers/scsi/ufs/ufshcd.c
> +++ b/drivers/scsi/ufs/ufshcd.c
> @@ -3776,7 +3776,7 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba *hba,
> struct uic_command *cmd)
>  }
> 
>  /**
> - * ufshcd_uic_change_pwr_mode - Perform the UIC power mode chage
> + * ufshcd_uic_change_pwr_mode - Perform the UIC power mode change
Please send a different patch fixing typos

Thanks,
Avri

>   *				using DME_SET primitives.
>   * @hba: per adapter instance
>   * @mode: powr mode value
> --
> 2.7.4