Hi Colin, On 2021-06-08 23:44, Colin Ian King wrote:
Hi, static analysis with Coverity on linux-next has found an issue in drivers/scsi/ufs/ufshcd.c introduced by the following commit: commit a45f937110fa6b0c2c06a5d3ef026963a5759050 Author: Can Guo <cang@xxxxxxxxxxxxxx> Date: Mon May 24 01:36:57 2021 -0700 scsi: ufs: Optimize host lock on transfer requests send/compl paths The analysis is as follows: 2948 static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, 2949 enum dev_cmd_type cmd_type, int timeout) 2950 { 2951 struct request_queue *q = hba->cmd_queue; 2952 struct request *req; 1. var_decl: Declaring variable lrbp without initializer. 2953 struct ufshcd_lrb *lrbp; 2954 int err; 2955 int tag; 2956 struct completion wait; 2957 2958 down_read(&hba->clk_scaling_lock); 2959 2960 /* 2961 * Get free slot, sleep if slots are unavailable.2962 * Even though we use wait_event() which sleeps indefinitely, 2963 * the maximum wait time is bounded by SCSI request timeout.2964 */ 2965 req = blk_get_request(q, REQ_OP_DRV_OUT, 0); 2. Condition IS_ERR(req), taking false branch. 2966 if (IS_ERR(req)) { 2967 err = PTR_ERR(req); 2968 goto out_unlock; 2969 } 2970 tag = req->tag; 3. Condition !!__ret_warn_on, taking false branch. 4. Condition !!__ret_warn_on, taking false branch. 2971 WARN_ON_ONCE(!ufshcd_valid_tag(hba, tag)); 2972 /* Set the timeout such that the SCSI error handler is not activated. */ 2973 req->timeout = msecs_to_jiffies(2 * timeout); 2974 blk_mq_start_request(req); 2975 5. Condition !!test_bit(tag, &hba->outstanding_reqs), taking true branch. 2976 if (unlikely(test_bit(tag, &hba->outstanding_reqs))) { 2977 err = -EBUSY; 6. Jumping to label out. 2978 goto out; 2979 } 2980 2981 init_completion(&wait); 2982 lrbp = &hba->lrb[tag]; 2983 WARN_ON(lrbp->cmd); 2984 err = ufshcd_compose_dev_cmd(hba, lrbp, cmd_type, tag); 2985 if (unlikely(err)) 2986 goto out_put_tag; 2987 2988 hba->dev_cmd.complete = &wait; 2989 2990 ufshcd_add_query_upiu_trace(hba, UFS_QUERY_SEND, lrbp->ucd_req_ptr); 2991 /* Make sure descriptors are ready before ringing the doorbell */ 2992 wmb(); 2993 2994 ufshcd_send_command(hba, tag); 2995 err = ufshcd_wait_for_dev_cmd(hba, lrbp, timeout); 2996 out: 7. Condition err, taking true branch. Uninitialized pointer read (UNINIT) 8. uninit_use: Using uninitialized value lrbp. 2997 ufshcd_add_query_upiu_trace(hba, err ? UFS_QUERY_ERR : UFS_QUERY_COMP, 2998 (struct utp_upiu_req *)lrbp->ucd_rsp_ptr); 2999 3000 out_put_tag: 3001 blk_put_request(req); 3002 out_unlock: 3003 up_read(&hba->clk_scaling_lock); 3004 return err; 3005 } Pointer lrbp is being accessed on the error exit path on line 2989 because it is no longer being initialized early, the pointer assignment was moved to a later point (line 2982) by the commit referenced in the top of the email. Colin
I will fix it by changing "goto out;" -> "goto out_put_tag;" on line #2978
in a new patch. Thanks, Can Guo.
