From 41c7b7c0fa2f68afb1154e88597ff6b9b97334cf Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Sun, 6 Nov 2011 09:40:11 -0500 Subject: Staging: bcm: Fix information leak in ioctl: IOCTL_BCM_REGISTER_READ_PRIVATE, IOCTL_BCM_EEPROM_REGISTER_READ This patch fixes an information leak in ioctl IOCTL_BCM_REGISTER_READ_PRIVATE and IOCTL_BCM_EEPROM_REGISTER_READ when determining the number of bytes to copy to user space. Function, usb_control_msg, returns the correct number of bytes from the hardware. Instead of using this value, we were using a value derived from user space. In this case, this value could be more than the hardware allocated. Therefore, this patch copies the proper number of bytes from the hardware, and uses this value as the maximum number of bytes for user space. Signed-off-by: Kevin McKinney Reviewed-by: Dan Carpenter Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 49 ++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 15 deletions(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index 2fa658eb74d..e110d0e6887 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -161,6 +161,7 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) INT Status = STATUS_FAILURE; int timeout = 0; IOCTL_BUFFER IoBuffer; + int bytes; BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, "Parameters Passed to control IOCTL cmd=0x%X arg=0x%lX", cmd, arg); @@ -230,11 +231,14 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) if (!temp_buff) return -ENOMEM; - Status = rdmalt(Adapter, (UINT)sRdmBuffer.Register, + bytes = rdmalt(Adapter, (UINT)sRdmBuffer.Register, (PUINT)temp_buff, Bufflen); - if (Status == STATUS_SUCCESS) { - if (copy_to_user(IoBuffer.OutputBuffer, temp_buff, IoBuffer.OutputLength)) + if (bytes > 0) { + Status = STATUS_SUCCESS; + if (copy_to_user(IoBuffer.OutputBuffer, temp_buff, bytes)) Status = -EFAULT; + } else { + Status = bytes; } kfree(temp_buff); @@ -318,11 +322,15 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) } uiTempVar = sRdmBuffer.Register & EEPROM_REJECT_MASK; - Status = rdmaltWithLock(Adapter, (UINT)sRdmBuffer.Register, (PUINT)temp_buff, IoBuffer.OutputLength); + bytes = rdmaltWithLock(Adapter, (UINT)sRdmBuffer.Register, (PUINT)temp_buff, IoBuffer.OutputLength); - if (Status == STATUS_SUCCESS) - if (copy_to_user(IoBuffer.OutputBuffer, temp_buff, IoBuffer.OutputLength)) + if (bytes > 0) { + Status = STATUS_SUCCESS; + if (copy_to_user(IoBuffer.OutputBuffer, temp_buff, bytes)) Status = -EFAULT; + } else { + Status = bytes; + } kfree(temp_buff); break; @@ -437,12 +445,14 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) } } - Status = rdmaltWithLock(Adapter, (UINT)GPIO_MODE_REGISTER, (PUINT)ucResetValue, sizeof(UINT)); - - if (STATUS_SUCCESS != Status) { + bytes = rdmaltWithLock(Adapter, (UINT)GPIO_MODE_REGISTER, (PUINT)ucResetValue, sizeof(UINT)); + if (bytes < 0) { + Status = bytes; BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, "GPIO_MODE_REGISTER read failed"); break; + } else { + Status = STATUS_SUCCESS; } /* Set the gpio mode register to output */ @@ -519,12 +529,15 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) uiBit = gpio_info.uiGpioNumber; /* Set the gpio output register */ - Status = rdmaltWithLock(Adapter, (UINT)GPIO_PIN_STATE_REGISTER, + bytes = rdmaltWithLock(Adapter, (UINT)GPIO_PIN_STATE_REGISTER, (PUINT)ucRead, sizeof(UINT)); - if (Status != STATUS_SUCCESS) { + if (bytes < 0) { + Status = bytes; BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "RDM Failed\n"); return Status; + } else { + Status = STATUS_SUCCESS; } } break; @@ -590,11 +603,14 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) } if (pgpio_multi_info[WIMAX_IDX].uiGPIOMask) { - Status = rdmaltWithLock(Adapter, (UINT)GPIO_PIN_STATE_REGISTER, (PUINT)ucResetValue, sizeof(UINT)); + bytes = rdmaltWithLock(Adapter, (UINT)GPIO_PIN_STATE_REGISTER, (PUINT)ucResetValue, sizeof(UINT)); - if (Status != STATUS_SUCCESS) { + if (bytes < 0) { + Status = bytes; BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "RDM to GPIO_PIN_STATE_REGISTER Failed."); return Status; + } else { + Status = STATUS_SUCCESS; } pgpio_multi_info[WIMAX_IDX].uiGPIOValue = (*(UINT *)ucResetValue & @@ -629,11 +645,14 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) if (copy_from_user(&gpio_multi_mode, IoBuffer.InputBuffer, IoBuffer.InputLength)) return -EFAULT; - Status = rdmaltWithLock(Adapter, (UINT)GPIO_MODE_REGISTER, (PUINT)ucResetValue, sizeof(UINT)); + bytes = rdmaltWithLock(Adapter, (UINT)GPIO_MODE_REGISTER, (PUINT)ucResetValue, sizeof(UINT)); - if (STATUS_SUCCESS != Status) { + if (bytes < 0) { + Status = bytes; BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Read of GPIO_MODE_REGISTER failed"); return Status; + } else { + Status = STATUS_SUCCESS; } /* Validating the request */ -- cgit v1.2.3 From 51935d2259a476162bbf5c35ff81f3a01057ed6f Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Tue, 8 Nov 2011 22:33:35 -0500 Subject: Staging: bcm: Clean up code in ioctl: IOCTL_BCM_EEPROM_REGISTER_READ This patch verifies two conditions before executing a kmalloc call. First, it checks to see that IoBuffer.OutputLength is not greater than an unsigned short. If so, an invalid value may be returned. The second change is a check to make sure IoBuffer.OutputLength is not equal to zero. Which simply keeps this code inline with the other ioctl, IOCTL_BCM_REGISTER_READ_PRIVATE. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index e110d0e6887..7cffbddd5e9 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -306,7 +306,11 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) if (copy_from_user(&sRdmBuffer, IoBuffer.InputBuffer, IoBuffer.InputLength)) return -EFAULT; - /* FIXME: don't trust user supplied length */ + if (IoBuffer.OutputLength > USHRT_MAX || + IoBuffer.OutputLength == 0) { + return -EINVAL; + } + temp_buff = kmalloc(IoBuffer.OutputLength, GFP_KERNEL); if (!temp_buff) return STATUS_FAILURE; -- cgit v1.2.3 From 77121d52a4de5e06c0b37d7e2fb9f6060904ecca Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Tue, 22 Nov 2011 20:25:55 -0500 Subject: Staging: bcm: Remove unnecessary "do while" statement in, IOCTL_BCM_BUFFER_DOWNLOAD This patch removes a superfluous "do while" statement in IOCTL_BCM_BUFFER_DOWNLOAD. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 79 +++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 41 deletions(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index 7cffbddd5e9..1bf28c95ef8 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -792,59 +792,56 @@ cntrlEnd: case IOCTL_BCM_BUFFER_DOWNLOAD: { FIRMWARE_INFO *psFwInfo = NULL; BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Starting the firmware download PID =0x%x!!!!\n", current->pid); - do { - if (!down_trylock(&Adapter->fw_download_sema)) { - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, - "Invalid way to download buffer. Use Start and then call this!!!\n"); - Status = -EINVAL; - break; - } - - /* Copy Ioctl Buffer structure */ - if (copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER))) - return -EFAULT; + if (!down_trylock(&Adapter->fw_download_sema)) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, - "Length for FW DLD is : %lx\n", IoBuffer.InputLength); + "Invalid way to download buffer. Use Start and then call this!!!\n"); + Status = -EINVAL; + break; + } - if (IoBuffer.InputLength > sizeof(FIRMWARE_INFO)) - return -EINVAL; + /* Copy Ioctl Buffer structure */ + if (copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER))) + return -EFAULT; - psFwInfo = kmalloc(sizeof(*psFwInfo), GFP_KERNEL); - if (!psFwInfo) - return -ENOMEM; + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, + "Length for FW DLD is : %lx\n", IoBuffer.InputLength); - if (copy_from_user(psFwInfo, IoBuffer.InputBuffer, IoBuffer.InputLength)) - return -EFAULT; + if (IoBuffer.InputLength > sizeof(FIRMWARE_INFO)) + return -EINVAL; - if (!psFwInfo->pvMappedFirmwareAddress || - (psFwInfo->u32FirmwareLength == 0)) { + psFwInfo = kmalloc(sizeof(*psFwInfo), GFP_KERNEL); + if (!psFwInfo) + return -ENOMEM; - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Something else is wrong %lu\n", - psFwInfo->u32FirmwareLength); - Status = -EINVAL; - break; - } + if (copy_from_user(psFwInfo, IoBuffer.InputBuffer, IoBuffer.InputLength)) + return -EFAULT; - Status = bcm_ioctl_fw_download(Adapter, psFwInfo); + if (!psFwInfo->pvMappedFirmwareAddress || + (psFwInfo->u32FirmwareLength == 0)) { - if (Status != STATUS_SUCCESS) { - if (psFwInfo->u32StartingAddress == CONFIG_BEGIN_ADDR) - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "IOCTL: Configuration File Upload Failed\n"); - else - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "IOCTL: Firmware File Upload Failed\n"); + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Something else is wrong %lu\n", + psFwInfo->u32FirmwareLength); + Status = -EINVAL; + break; + } - /* up(&Adapter->fw_download_sema); */ + Status = bcm_ioctl_fw_download(Adapter, psFwInfo); - if (Adapter->LEDInfo.led_thread_running & BCM_LED_THREAD_RUNNING_ACTIVELY) { - Adapter->DriverState = DRIVER_INIT; - Adapter->LEDInfo.bLedInitDone = FALSE; - wake_up(&Adapter->LEDInfo.notify_led_event); - } - } - break; + if (Status != STATUS_SUCCESS) { + if (psFwInfo->u32StartingAddress == CONFIG_BEGIN_ADDR) + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "IOCTL: Configuration File Upload Failed\n"); + else + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "IOCTL: Firmware File Upload Failed\n"); + + /* up(&Adapter->fw_download_sema); */ - } while (0); + if (Adapter->LEDInfo.led_thread_running & BCM_LED_THREAD_RUNNING_ACTIVELY) { + Adapter->DriverState = DRIVER_INIT; + Adapter->LEDInfo.bLedInitDone = FALSE; + wake_up(&Adapter->LEDInfo.notify_led_event); + } + } if (Status != STATUS_SUCCESS) up(&Adapter->fw_download_sema); -- cgit v1.2.3 From abe33fc0933f9221193fc047fbf93cf9db23c8f6 Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Tue, 22 Nov 2011 20:25:56 -0500 Subject: Staging: bcm: Fix semaphore locking bug in, IOCTL_BCM_BUFFER_DOWNLOAD In this ioctl, we are testing to see if the lock is held. If it is not held, that means this ioctl used incorrectly. Therefore, we do not want to take the lock ourselves here. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index 1bf28c95ef8..5a3e7c846a3 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -796,6 +796,7 @@ cntrlEnd: if (!down_trylock(&Adapter->fw_download_sema)) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Invalid way to download buffer. Use Start and then call this!!!\n"); + up(&Adapter->fw_download_sema); Status = -EINVAL; break; } -- cgit v1.2.3 From d9f26a6689a3e4ae643fca9c2acda3148b717e63 Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Tue, 22 Nov 2011 20:25:57 -0500 Subject: Staging: bcm: Fix semaphore locking error when downloading firmware. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch releases semaphore locks when an error occurrs while attempting to download firmware for the bcm driver. When downloading firmware for this driver, a process is expected to call the following ioctl's in this order: (1)IOCTL_BCM_BUFFER_DOWNLOAD_START, (2)IOCTL_BCM_BUFFER_DOWNLOAD, and (3) IOCTL_BCM_BUFFER_DOWNLOAD_STOP. Semaphore, “Adapter->fw_download_sema” is expected to be acquired in the first ioctl, IOCTL_BCM_BUFFER_DOWNLOAD_START, and it should block until IOCTL_BCM_BUFFER_DOWNLOAD_STOP is called. In this case, if an error occurred before STOP finished, the semaphore "Adapter->fw_download_sema" was not being released. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index 5a3e7c846a3..d0a0b10c5d4 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -802,27 +802,36 @@ cntrlEnd: } /* Copy Ioctl Buffer structure */ - if (copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER))) + if (copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER))) { + up(&Adapter->fw_download_sema); return -EFAULT; + } BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Length for FW DLD is : %lx\n", IoBuffer.InputLength); - if (IoBuffer.InputLength > sizeof(FIRMWARE_INFO)) + if (IoBuffer.InputLength > sizeof(FIRMWARE_INFO)) { + up(&Adapter->fw_download_sema); return -EINVAL; + } psFwInfo = kmalloc(sizeof(*psFwInfo), GFP_KERNEL); - if (!psFwInfo) + if (!psFwInfo) { + up(&Adapter->fw_download_sema); return -ENOMEM; + } - if (copy_from_user(psFwInfo, IoBuffer.InputBuffer, IoBuffer.InputLength)) + if (copy_from_user(psFwInfo, IoBuffer.InputBuffer, IoBuffer.InputLength)) { + up(&Adapter->fw_download_sema); return -EFAULT; + } if (!psFwInfo->pvMappedFirmwareAddress || (psFwInfo->u32FirmwareLength == 0)) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Something else is wrong %lu\n", psFwInfo->u32FirmwareLength); + up(&Adapter->fw_download_sema); Status = -EINVAL; break; } -- cgit v1.2.3 From fef5675ecbeb0c92d49c8b350e1324164bb348a5 Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Sun, 27 Nov 2011 20:51:45 -0500 Subject: Staging: bcm: Clean up patch that calls semaphore down_trylock directly. This patch evaluates/calls the down_trylock locking function directly, instead of storing the results in a variable and evaluating the variable. These changes were made in: IOCTL_BCM_BUFFER_DOWNLOAD_STOP and IOCTL_BCM_BUFFER_DOWNLOAD_START. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index d0a0b10c5d4..e6b20c80db5 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -756,8 +756,7 @@ cntrlEnd: } case IOCTL_BCM_BUFFER_DOWNLOAD_START: { - INT NVMAccess = down_trylock(&Adapter->NVMRdmWrmLock); - if (NVMAccess) { + if (down_trylock(&Adapter->NVMRdmWrmLock)) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, "IOCTL_BCM_CHIP_RESET not allowed as EEPROM Read/Write is in progress\n"); return -EACCES; @@ -862,9 +861,7 @@ cntrlEnd: } case IOCTL_BCM_BUFFER_DOWNLOAD_STOP: { - INT NVMAccess = down_trylock(&Adapter->NVMRdmWrmLock); - - if (NVMAccess) { + if (down_trylock(&Adapter->NVMRdmWrmLock)) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "FW download blocked as EEPROM Read/Write is in progress\n"); up(&Adapter->fw_download_sema); -- cgit v1.2.3 From 8fbebb091a1695b8896597573c2d75831e41929a Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Sun, 27 Nov 2011 20:51:46 -0500 Subject: Staging: bcm: Alter code to move error handling closer to the calls. This is a cleanup patch. I've shuffled the code around to move the error handling closer to the calls. I've removed some indent levels. I've replaced break statements with direct returns. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 132 +++++++++++++++++++++--------------------- 1 file changed, 66 insertions(+), 66 deletions(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index e6b20c80db5..3b7c15039ee 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -765,27 +765,26 @@ cntrlEnd: BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Starting the firmware download PID =0x%x!!!!\n", current->pid); - if (!down_trylock(&Adapter->fw_download_sema)) { - Adapter->bBinDownloaded = FALSE; - Adapter->fw_download_process_pid = current->pid; - Adapter->bCfgDownloaded = FALSE; - Adapter->fw_download_done = FALSE; - netif_carrier_off(Adapter->dev); - netif_stop_queue(Adapter->dev); - Status = reset_card_proc(Adapter); - if (Status) { - pr_err(PFX "%s: reset_card_proc Failed!\n", Adapter->dev->name); - up(&Adapter->fw_download_sema); - up(&Adapter->NVMRdmWrmLock); - break; - } - mdelay(10); - } else { - Status = -EBUSY; + if (down_trylock(&Adapter->fw_download_sema)) + return -EBUSY; + + Adapter->bBinDownloaded = FALSE; + Adapter->fw_download_process_pid = current->pid; + Adapter->bCfgDownloaded = FALSE; + Adapter->fw_download_done = FALSE; + netif_carrier_off(Adapter->dev); + netif_stop_queue(Adapter->dev); + Status = reset_card_proc(Adapter); + if (Status) { + pr_err(PFX "%s: reset_card_proc Failed!\n", Adapter->dev->name); + up(&Adapter->fw_download_sema); + up(&Adapter->NVMRdmWrmLock); + return Status; } + mdelay(10); up(&Adapter->NVMRdmWrmLock); - break; + return Status; } case IOCTL_BCM_BUFFER_DOWNLOAD: { @@ -797,7 +796,7 @@ cntrlEnd: "Invalid way to download buffer. Use Start and then call this!!!\n"); up(&Adapter->fw_download_sema); Status = -EINVAL; - break; + return Status; } /* Copy Ioctl Buffer structure */ @@ -832,7 +831,7 @@ cntrlEnd: psFwInfo->u32FirmwareLength); up(&Adapter->fw_download_sema); Status = -EINVAL; - break; + return Status; } Status = bcm_ioctl_fw_download(Adapter, psFwInfo); @@ -857,7 +856,7 @@ cntrlEnd: BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, OSAL_DBG, DBG_LVL_ALL, "IOCTL: Firmware File Uploaded\n"); kfree(psFwInfo); - break; + return Status; } case IOCTL_BCM_BUFFER_DOWNLOAD_STOP: { @@ -868,59 +867,60 @@ cntrlEnd: return -EACCES; } - if (down_trylock(&Adapter->fw_download_sema)) { - Adapter->bBinDownloaded = TRUE; - Adapter->bCfgDownloaded = TRUE; - atomic_set(&Adapter->CurrNumFreeTxDesc, 0); - Adapter->CurrNumRecvDescs = 0; - Adapter->downloadDDR = 0; - - /* setting the Mips to Run */ - Status = run_card_proc(Adapter); - - if (Status) { - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Firm Download Failed\n"); - up(&Adapter->fw_download_sema); - up(&Adapter->NVMRdmWrmLock); - break; - } else { - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, - DBG_LVL_ALL, "Firm Download Over...\n"); - } + if (!down_trylock(&Adapter->fw_download_sema)) { + up(&Adapter->fw_download_sema); + return -EINVAL; + } - mdelay(10); - - /* Wait for MailBox Interrupt */ - if (StartInterruptUrb((PS_INTERFACE_ADAPTER)Adapter->pvInterfaceAdapter)) - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Unable to send interrupt...\n"); - - timeout = 5*HZ; - Adapter->waiting_to_fw_download_done = FALSE; - wait_event_timeout(Adapter->ioctl_fw_dnld_wait_queue, - Adapter->waiting_to_fw_download_done, timeout); - Adapter->fw_download_process_pid = INVALID_PID; - Adapter->fw_download_done = TRUE; - atomic_set(&Adapter->CurrNumFreeTxDesc, 0); - Adapter->CurrNumRecvDescs = 0; - Adapter->PrevNumRecvDescs = 0; - atomic_set(&Adapter->cntrlpktCnt, 0); - Adapter->LinkUpStatus = 0; - Adapter->LinkStatus = 0; + Adapter->bBinDownloaded = TRUE; + Adapter->bCfgDownloaded = TRUE; + atomic_set(&Adapter->CurrNumFreeTxDesc, 0); + Adapter->CurrNumRecvDescs = 0; + Adapter->downloadDDR = 0; - if (Adapter->LEDInfo.led_thread_running & BCM_LED_THREAD_RUNNING_ACTIVELY) { - Adapter->DriverState = FW_DOWNLOAD_DONE; - wake_up(&Adapter->LEDInfo.notify_led_event); - } + /* setting the Mips to Run */ + Status = run_card_proc(Adapter); - if (!timeout) - Status = -ENODEV; + if (Status) { + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Firm Download Failed\n"); + up(&Adapter->fw_download_sema); + up(&Adapter->NVMRdmWrmLock); + return Status; } else { - Status = -EINVAL; + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, + DBG_LVL_ALL, "Firm Download Over...\n"); + } + + mdelay(10); + + /* Wait for MailBox Interrupt */ + if (StartInterruptUrb((PS_INTERFACE_ADAPTER)Adapter->pvInterfaceAdapter)) + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Unable to send interrupt...\n"); + + timeout = 5*HZ; + Adapter->waiting_to_fw_download_done = FALSE; + wait_event_timeout(Adapter->ioctl_fw_dnld_wait_queue, + Adapter->waiting_to_fw_download_done, timeout); + Adapter->fw_download_process_pid = INVALID_PID; + Adapter->fw_download_done = TRUE; + atomic_set(&Adapter->CurrNumFreeTxDesc, 0); + Adapter->CurrNumRecvDescs = 0; + Adapter->PrevNumRecvDescs = 0; + atomic_set(&Adapter->cntrlpktCnt, 0); + Adapter->LinkUpStatus = 0; + Adapter->LinkStatus = 0; + + if (Adapter->LEDInfo.led_thread_running & BCM_LED_THREAD_RUNNING_ACTIVELY) { + Adapter->DriverState = FW_DOWNLOAD_DONE; + wake_up(&Adapter->LEDInfo.notify_led_event); } + if (!timeout) + Status = -ENODEV; + up(&Adapter->fw_download_sema); up(&Adapter->NVMRdmWrmLock); - break; + return Status; } case IOCTL_BE_BUCKET_SIZE: -- cgit v1.2.3 From 19a177e6fb097c2a0dd94aeeb2805f0c04a4185f Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Sun, 27 Nov 2011 20:51:47 -0500 Subject: Staging: bcm: Reverse semaphore locking in IOCTL_BCM_BUFFER_DOWNLOAD_STOP. This patch reorders the semaphore locking. It makes better sense to first evaluate fw_download_sema semaphore then NVMRdmWrmLocl semaphore. The fw_download_sema is suppose to be acquired in the START ioctl. If this is not true, then it does not make sense to continue. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index 3b7c15039ee..72a699c1270 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -860,6 +860,11 @@ cntrlEnd: } case IOCTL_BCM_BUFFER_DOWNLOAD_STOP: { + if (!down_trylock(&Adapter->fw_download_sema)) { + up(&Adapter->fw_download_sema); + return -EINVAL; + } + if (down_trylock(&Adapter->NVMRdmWrmLock)) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "FW download blocked as EEPROM Read/Write is in progress\n"); @@ -867,11 +872,6 @@ cntrlEnd: return -EACCES; } - if (!down_trylock(&Adapter->fw_download_sema)) { - up(&Adapter->fw_download_sema); - return -EINVAL; - } - Adapter->bBinDownloaded = TRUE; Adapter->bCfgDownloaded = TRUE; atomic_set(&Adapter->CurrNumFreeTxDesc, 0); -- cgit v1.2.3 From 09468b0392b1d2b079b334d60a23d7da8105dc53 Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Thu, 1 Dec 2011 22:02:15 -0500 Subject: Staging: bcm: Fix double free of 'pReadData' in IOCTL_BCM_NVM_WRITE. This patch fixes a memory error in ioctl, IOCTL_BCM_NVM_WRITE. While copying data to user space, if an error occurs, pReadData is freed. Then, at the end of the ioctl, pReadData was being freed again. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index 72a699c1270..ad1cc5598dc 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -1336,7 +1336,7 @@ cntrlEnd: if (copy_to_user(stNVMReadWrite.pBuffer, pReadData, stNVMReadWrite.uiNumBytes)) { kfree(pReadData); - Status = -EFAULT; + return -EFAULT; } } else { down(&Adapter->NVMRdmWrmLock); -- cgit v1.2.3 From 6561f91d68e6f93cba91e64a3b9e0feba8b84fb8 Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Thu, 1 Dec 2011 22:02:16 -0500 Subject: Staging: bcm: Alter code to move error handling closer to the calls; and remove white space, IOCTL_BCM_NVM_WRITE. This is a clean up patch for IOCTL_BCM_NVM_WRITE that replaces the assignment of the Status variable with direct returns of the error code, replaces the break statements with direct returns, and removes a white space. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index ad1cc5598dc..5655ce3418c 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -1269,8 +1269,7 @@ cntrlEnd: memset(&tv1, 0, sizeof(struct timeval)); if ((Adapter->eNVMType == NVM_FLASH) && (Adapter->uiFlashLayoutMajorVersion == 0)) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "The Flash Control Section is Corrupted. Hence Rejection on NVM Read/Write\n"); - Status = -EFAULT; - break; + return -EFAULT; } if (IsFlash2x(Adapter)) { @@ -1279,7 +1278,7 @@ cntrlEnd: (Adapter->eActiveDSD != DSD2)) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "No DSD is active..hence NVM Command is blocked"); - return STATUS_FAILURE ; + return STATUS_FAILURE; } } @@ -1298,8 +1297,7 @@ cntrlEnd: if ((stNVMReadWrite.uiOffset + stNVMReadWrite.uiNumBytes) > Adapter->uiNVMDSDSize) { /* BCM_DEBUG_PRINT(Adapter,DBG_TYPE_PRINTK, 0, 0,"Can't allow access beyond NVM Size: 0x%x 0x%x\n", stNVMReadWrite.uiOffset, stNVMReadWrite.uiNumBytes); */ - Status = STATUS_FAILURE; - break; + return STATUS_FAILURE; } pReadData = kzalloc(stNVMReadWrite.uiNumBytes, GFP_KERNEL); @@ -1307,9 +1305,8 @@ cntrlEnd: return -ENOMEM; if (copy_from_user(pReadData, stNVMReadWrite.pBuffer, stNVMReadWrite.uiNumBytes)) { - Status = -EFAULT; kfree(pReadData); - break; + return -EFAULT; } do_gettimeofday(&tv0); @@ -1404,9 +1401,8 @@ cntrlEnd: BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, " timetaken by Write/read :%ld msec\n", (tv1.tv_sec - tv0.tv_sec)*1000 + (tv1.tv_usec - tv0.tv_usec)/1000); kfree(pReadData); - Status = STATUS_SUCCESS; + return STATUS_SUCCESS; } - break; case IOCTL_BCM_FLASH2X_SECTION_READ: { FLASH2X_READWRITE sFlash2xRead = {0}; -- cgit v1.2.3 From d3a21c3c4bbab3d51bde0e4828cb8165d04d6399 Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Mon, 5 Dec 2011 22:17:03 -0500 Subject: Staging: bcm: Alter return value for copy_to/from_user() to "return -EFAULT" when an error occurs. In this clean up patch, I altered functions: copy_to/ from_user() to return -EFAULT when an error occurs. I also replaced break statements when an error occurs from copy_to/from_user() with direct returns of -EFAULT. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 75 +++++++++++++++++++++++-------------------- 1 file changed, 41 insertions(+), 34 deletions(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index 5655ce3418c..c4d7a619418 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -235,8 +235,10 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) (PUINT)temp_buff, Bufflen); if (bytes > 0) { Status = STATUS_SUCCESS; - if (copy_to_user(IoBuffer.OutputBuffer, temp_buff, bytes)) - Status = -EFAULT; + if (copy_to_user(IoBuffer.OutputBuffer, temp_buff, bytes)) { + kfree(temp_buff); + return -EFAULT; + } } else { Status = bytes; } @@ -330,8 +332,10 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) if (bytes > 0) { Status = STATUS_SUCCESS; - if (copy_to_user(IoBuffer.OutputBuffer, temp_buff, bytes)) - Status = -EFAULT; + if (copy_to_user(IoBuffer.OutputBuffer, temp_buff, bytes)) { + kfree(temp_buff); + return -EFAULT; + } } else { Status = bytes; } @@ -625,7 +629,7 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Failed while copying Content to IOBufer for user space err:%d", Status); - break; + return -EFAULT; } } break; @@ -701,7 +705,7 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Failed while copying Content to IOBufer for user space err:%d", Status); - break; + return -EFAULT; } } break; @@ -729,9 +733,8 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, ULONG arg) return -ENOMEM; if (copy_from_user(pvBuffer, IoBuffer.InputBuffer, IoBuffer.InputLength)) { - Status = -EFAULT; kfree(pvBuffer); - break; + return -EFAULT; } down(&Adapter->LowPowerModeSync); @@ -1012,8 +1015,7 @@ cntrlEnd: /* Copy Ioctl Buffer structure */ if (copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER))) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "copy_from_user failed..\n"); - Status = -EFAULT; - break; + return -EFAULT; } if (IoBuffer.OutputLength != sizeof(link_state)) { @@ -1028,8 +1030,7 @@ cntrlEnd: if (copy_to_user(IoBuffer.OutputBuffer, &link_state, min_t(size_t, sizeof(link_state), IoBuffer.OutputLength))) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Copy_to_user Failed..\n"); - Status = -EFAULT; - break; + return -EFAULT; } Status = STATUS_SUCCESS; break; @@ -1095,8 +1096,10 @@ cntrlEnd: GetDroppedAppCntrlPktMibs(temp_buff, pTarang); if (Status != STATUS_FAILURE) - if (copy_to_user(IoBuffer.OutputBuffer, temp_buff, sizeof(S_MIBS_HOST_STATS_MIBS))) - Status = -EFAULT; + if (copy_to_user(IoBuffer.OutputBuffer, temp_buff, sizeof(S_MIBS_HOST_STATS_MIBS))) { + kfree(temp_buff); + return -EFAULT; + } kfree(temp_buff); break; @@ -1138,8 +1141,7 @@ cntrlEnd: /* Get WrmBuffer structure */ if (copy_from_user(pvBuffer, IoBuffer.InputBuffer, IoBuffer.InputLength)) { kfree(pvBuffer); - Status = -EFAULT; - break; + return -EFAULT; } pBulkBuffer = (PBULKWRM_BUFFER)pvBuffer; @@ -1479,7 +1481,9 @@ cntrlEnd: Status = copy_to_user(OutPutBuff, pReadBuff, ReadBytes); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, "Copy to use failed with status :%d", Status); - break; + up(&Adapter->NVMRdmWrmLock); + kfree(pReadBuff); + return -EFAULT; } NOB = NOB - ReadBytes; if (NOB) { @@ -1571,7 +1575,9 @@ cntrlEnd: Status = copy_from_user(pWriteBuff, InputAddr, WriteBytes); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Copy to user failed with status :%d", Status); - break; + up(&Adapter->NVMRdmWrmLock); + kfree(pWriteBuff); + return -EFAULT; } BCM_DEBUG_PRINT_BUFFER(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, pWriteBuff, WriteBytes); @@ -1631,8 +1637,10 @@ cntrlEnd: BcmGetFlash2xSectionalBitMap(Adapter, psFlash2xBitMap); up(&Adapter->NVMRdmWrmLock); - if (copy_to_user(IoBuffer.OutputBuffer, psFlash2xBitMap, sizeof(FLASH2X_BITMAP))) - Status = -EFAULT; + if (copy_to_user(IoBuffer.OutputBuffer, psFlash2xBitMap, sizeof(FLASH2X_BITMAP))) { + kfree(psFlash2xBitMap); + return -EFAULT; + } kfree(psFlash2xBitMap); } @@ -1650,13 +1658,13 @@ cntrlEnd: Status = copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER)); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Copy of IOCTL BUFFER failed"); - return Status; + return -EFAULT; } Status = copy_from_user(&eFlash2xSectionVal, IoBuffer.InputBuffer, sizeof(INT)); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Copy of flash section val failed"); - return Status; + return -EFAULT; } down(&Adapter->NVMRdmWrmLock); @@ -1700,13 +1708,13 @@ cntrlEnd: Status = copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER)); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Copy of IOCTL BUFFER failed Status :%d", Status); - return Status; + return -EFAULT; } Status = copy_from_user(&sCopySectStrut, IoBuffer.InputBuffer, sizeof(FLASH2X_COPY_SECTION)); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Copy of Copy_Section_Struct failed with Status :%d", Status); - return Status; + return -EFAULT; } BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, "Source SEction :%x", sCopySectStrut.SrcSection); @@ -1767,7 +1775,7 @@ cntrlEnd: Status = copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER)); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Copy of IOCTL BUFFER failed"); - break; + return -EFAULT; } if (Adapter->eNVMType != NVM_FLASH) { @@ -1806,12 +1814,12 @@ cntrlEnd: Status = copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER)); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Copy of IOCTL BUFFER failed"); - return Status; + return -EFAULT; } Status = copy_from_user(&eFlash2xSectionVal, IoBuffer.InputBuffer, sizeof(INT)); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Copy of flash section val failed"); - return Status; + return -EFAULT; } BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, "Read Section :%d", eFlash2xSectionVal); @@ -1853,8 +1861,7 @@ cntrlEnd: /* Copy Ioctl Buffer structure */ if (copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER))) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "copy_from_user 1 failed\n"); - Status = -EFAULT; - break; + return -EFAULT; } if (copy_from_user(&stNVMRead, IoBuffer.OutputBuffer, sizeof(NVM_READWRITE))) @@ -1909,7 +1916,9 @@ cntrlEnd: Status = copy_to_user(OutPutBuff, pReadBuff, ReadBytes); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Copy to use failed with status :%d", Status); - break; + up(&Adapter->NVMRdmWrmLock); + kfree(pReadBuff); + return -EFAULT; } NOB = NOB - ReadBytes; if (NOB) { @@ -1930,8 +1939,7 @@ cntrlEnd: Status = copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER)); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, "copy of Ioctl buffer is failed from user space"); - Status = -EFAULT; - break; + return -EFAULT; } if (IoBuffer.InputLength != sizeof(unsigned long)) { @@ -1942,8 +1950,7 @@ cntrlEnd: Status = copy_from_user(&RxCntrlMsgBitMask, IoBuffer.InputBuffer, IoBuffer.InputLength); if (Status) { BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, "copy of control bit mask failed from user space"); - Status = -EFAULT; - break; + return -EFAULT; } BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG, DBG_LVL_ALL, "\n Got user defined cntrl msg bit mask :%lx", RxCntrlMsgBitMask); pTarang->RxCntrlMsgBitMask = RxCntrlMsgBitMask; -- cgit v1.2.3 From b72a7c859efc9e0cf13600b30a555457a08dd86f Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Wed, 14 Dec 2011 22:44:33 -0500 Subject: Staging: bcm: Fix information leak in IOCTL_BCM_GET_DRIVER_VERSION This ioctl, IOCTL_BCM_GET_DRIVER_VERSION, is responsible for sending the driver version to userspace. However, the requested size stored in IoBuffer.OutputLength may be incorrect. Therefore, we altered the code to send the exact length of the version, plus one for the null character. Signed-off-by: Kevin McKinney Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index c4d7a619418..fa4a854ba05 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -999,11 +999,15 @@ cntrlEnd: } case IOCTL_BCM_GET_DRIVER_VERSION: { + ulong len; + /* Copy Ioctl Buffer structure */ if (copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER))) return -EFAULT; - if (copy_to_user(IoBuffer.OutputBuffer, VER_FILEVERSION_STR, IoBuffer.OutputLength)) + len = min_t(ulong, IoBuffer.OutputLength, strlen(VER_FILEVERSION_STR) + 1); + + if (copy_to_user(IoBuffer.OutputBuffer, VER_FILEVERSION_STR, len)) return -EFAULT; Status = STATUS_SUCCESS; break; -- cgit v1.2.3 From 221fd753dd002222b595f8af0e289fff0c9cf5a8 Mon Sep 17 00:00:00 2001 From: Kevin McKinney Date: Sat, 17 Dec 2011 11:53:37 -0500 Subject: Staging: bcm: Fix an invalid dereference to a kmalloc in IOCTL_BCM_BULK_WRM Variable IoBuffer.InputLength is chosen from userspace, and can therefore be less than the intended size. In this case,the memory from the kmalloc call is eventually cast to a PBULKWRM_BUFFER. If the IoBuffer.InputLength does not meet the minimum size of PBULKWRM_BUFFER, then we will get a kernel Oops. To resolve this issue, this patch verifies IoBuffer.InputLength meets the minimum size before invoking the kmalloc call. Signed-off-by: Kevin McKinney Reviewed-by: Dan Carpenter Signed-off-by: Greg Kroah-Hartman --- drivers/staging/bcm/Bcmchar.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/staging/bcm/Bcmchar.c') diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c index fa4a854ba05..179707b5e7c 100644 --- a/drivers/staging/bcm/Bcmchar.c +++ b/drivers/staging/bcm/Bcmchar.c @@ -1137,7 +1137,9 @@ cntrlEnd: if (copy_from_user(&IoBuffer, argp, sizeof(IOCTL_BUFFER))) return -EFAULT; - /* FIXME: restrict length */ + if (IoBuffer.InputLength < sizeof(ULONG) * 2) + return -EINVAL; + pvBuffer = kmalloc(IoBuffer.InputLength, GFP_KERNEL); if (!pvBuffer) return -ENOMEM; -- cgit v1.2.3