New: Function beiscsi_create_eqs() may return improper value when the call to pci_alloc_consistent() fails, which may result in use-after-free

https://bugzilla.kernel.org/show_bug.cgi?id=188951

Bug ID: 188951
Summary: Function beiscsi_create_eqs() may return improper
value when the call to pci_alloc_consistent() fails, which may result in use-after-free
Product: SCSI Drivers
Version: 2.5
Kernel Version: linux-4.9-rc6
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Other
Assignee: [email protected]
Reporter: [email protected]
Regression: No

Function pci_alloc_consistent() returns a NULL pointer if there is no enough memory. In function beiscsi_create_eqs() defined in file drivers/scsi/be2iscsi/be_main.c, function pci_alloc_consistent() is called and its return value is checked against NULL (at line 3052). If the return value is NULL, the control flow will jump to label "create_cq_error", frees allocated memory and returns variable ret. Because after the first execution of the loop the value of ret must be 0 (see the check statement of ret at line 3067), the return value will be 0 (indicates success) if pci_alloc_consistent() fails during the second or after repeats of the loop body. In this case, the freed memory may be used or freed again in the callers of beiscsi_create_eqs(). I think it is better to assign "-ENOMEM" when the call pci_alloc_consistent() fails. Codes and comments related to this bug are summarised as follows.

beiscsi_create_eqs @@ drivers/scsi/be2iscsi/be_main.c 3028 static int beiscsi_create_eqs(struct beiscsi_hba *phba,
3029 struct hwi_context_memory *phwi_context)
3030 {

3031     int ret = -ENOMEM, eq_for_mcc;
         ...
3045     for (i = 0; i < (phba->num_cpus + eq_for_mcc); i++) {
3046         eq = &phwi_context->be_eq[i].q;
3047         mem = &eq->dma_mem;
3048         phwi_context->be_eq[i].phba = phba;
3049         eq_vaddress = pci_alloc_consistent(phba->pcidev,
3050                            num_eq_pages * PAGE_SIZE,
3051                            &paddr);
3052         if (!eq_vaddress)
                 // ret may takes value 0. Add "ret = -ENOMEM" here?

3053 goto create_eq_error;

3065 ret = beiscsi_cmd_eq_create(&phba->ctrl, eq,
3066 phwi_context->cur_eqd);
3067 if (ret) {
3068 beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
3069 "BM_%d : beiscsi_cmd_eq_create"
3070 "Failed for EQ\n");
3071 goto create_eq_error;
3072 }
3073
3074 beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
3075 "BM_%d : eqid = %d\n",
3076 phwi_context->be_eq[i].q.id);
3077 }
3078 return 0;
3079
3080 create_eq_error:
3081 for (i = 0; i < (phba->num_cpus + eq_for_mcc); i++) {

3082         eq = &phwi_context->be_eq[i].q;
3083         mem = &eq->dma_mem;
3084         if (mem->va)

3085 pci_free_consistent(phba->pcidev, num_eq_pages
3086 * PAGE_SIZE,
3087 mem->va, mem->dma);
3088 }
3089 return ret;
3090 }

Thanks very much!

You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.kernel.org/show_bug.cgi?id=188951

--- Comment #1 from bianpan ([email protected]) --- Created attachment 256441
--> https://bugzilla.kernel.org/attachment.cgi?id=256441&action=edit
The patch fixes the bug

The patch has been merged into the latest version of the Linux kernel. So I will close the bug.

You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.kernel.org/show_bug.cgi?id=188951

bianpan ([email protected]) changed:

What |Removed |Added
----------------------------------------------------------------------------

             Status|NEW                         |RESOLVED
         Resolution|---                         |CODE_FIX

You are receiving this mail because:
You are watching the assignee of the bug.