Bug 189141 - Function bnx2x_init_firmware() does not set error code when the call to kmalloc() fails
Summary: Function bnx2x_init_firmware() does not set error code when the call to kmall...
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Network (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_network@kernel-bugs.osdl.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-25 11:26 UTC by bianpan
Modified: 2017-05-12 00:34 UTC (History)
0 users

See Also:
Kernel Version: linux-4.9-rc6
Subsystem:
Regression: No
Bisected commit-id:


Attachments
The patch fixes the bug (1.31 KB, patch)
2017-05-12 00:34 UTC, bianpan
Details | Diff

Description bianpan 2016-11-25 11:26:53 UTC
Function bnx2x_init_firmware() is defined in file drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c. It calls macro BNX2X_ALLOC_AND_SET() (at lines 13508, 13511, 13514, and 13535) to allocate memory. In macro BNX2X_ALLOC_AND_SET(), kmalloc() is called, and if it returns a NULL pointer, the control flow will jump to the given label, such as "request_firmware_exit", and returns variable rc at last. Because variable rc is checked at line 13499, its value must be 0 when BNX2X_ALLOC_AND_SET() is called. As a result, the return value will be 0 (indicates success) even if memory allocation fails. The inconsistence between return value and execution status may misled the callers of bnx2x_init_firmware(), resulting in unexpected bugs. Though these errors may occur rarely, I think it's better to return correct error code on failures. Codes related to these bugs are summarised as follows.

bnx2x_init_firmware @@ drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
13460 #define BNX2X_ALLOC_AND_SET(arr, lbl, func)             \
13461 do {                                    \
13462     u32 len = be32_to_cpu(fw_hdr->arr.len);             \
13463     bp->arr = kmalloc(len, GFP_KERNEL);             \
13464     if (!bp->arr)                           \
13465         goto lbl;                       \
13466     func(bp->firmware->data + be32_to_cpu(fw_hdr->arr.offset),  \
13467          (u8 *)bp->arr, len);                   \
13468 } while (0)
13469 
13470 static int bnx2x_init_firmware(struct bnx2x *bp)
13471 {
13472     const char *fw_file_name;
13473     struct bnx2x_fw_file_hdr *fw_hdr;
13474     int rc;
          ...
13498     rc = bnx2x_check_firmware(bp);
13499     if (rc) {
13500         BNX2X_ERR("Corrupt firmware file %s\n", fw_file_name);
13501         goto request_firmware_exit;
13502     }
13503 
13504     fw_hdr = (struct bnx2x_fw_file_hdr *)bp->firmware->data;
13505 
13506     /* Initialize the pointers to the init arrays */
13507     /* Blob */
          // Insert "rc = -ENOMEM;" ?
          // Bug (1)
13508     BNX2X_ALLOC_AND_SET(init_data, request_firmware_exit, be32_to_cpu_n);
13509 
13510     /* Opcodes */
          // Bug (2)
13511     BNX2X_ALLOC_AND_SET(init_ops, init_ops_alloc_err, bnx2x_prep_ops);
13512 
13513     /* Offsets */
          // Bug (3)
13514     BNX2X_ALLOC_AND_SET(init_ops_offsets, init_offsets_alloc_err,
13515                 be16_to_cpu_n);

13534     /* IRO */
          // Bug (4)
13535     BNX2X_ALLOC_AND_SET(iro_arr, iro_alloc_err, bnx2x_prep_iro);
13536 
13537     return 0;
13538 
13539 iro_alloc_err:
13540     kfree(bp->init_ops_offsets);
13541 init_offsets_alloc_err:
13542     kfree(bp->init_ops);
13543 init_ops_alloc_err:
13544     kfree(bp->init_data);
13545 request_firmware_exit:
13546     release_firmware(bp->firmware);
13547     bp->firmware = NULL;
13548 
13549     return rc;
13550 }

Thanks very much!
Comment 1 bianpan 2017-05-12 00:34:02 UTC
Created attachment 256463 [details]
The patch fixes the bug

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

Note You need to log in before you can comment on or make changes to this bug.