Messages in this thread | | | From | Colin Ian King <> | Subject | re: scsi: ufs: Optimize host lock on transfer requests send/compl paths (uninitialized pointer error) | Date | Tue, 8 Jun 2021 16:44:22 +0100 |
| |
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@codeaurora.org> 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
| |