On Fri, Dec 20, 2013 at 03:13:16PM +0800, Wenliang Fan wrote:
> The checking condition in 'validateFlash2xReadWrite()' is not sufficient.
> A large number invalid would cause an integer overflow and pass
> the condition, which could cause further integer overflows in
> 'Bcmchar.c:bcm_char_ioctl()'.
>
This patch has a couple typos and it breaks the build.
On the other hand it's a real bug because uiNumOfBytes comes from the
user in bcm_char_ioctl(). This function doesn't seem to be restricted
to privaleged users, which is terrifying.
I think it would be cleaner to fix it in the caller instead of here.
Please look over this totally untested patch and send something like
that, if that's ok.
diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c
index 62415342ee28..2ea885e36077 100644
--- a/drivers/staging/bcm/Bcmchar.c
+++ b/drivers/staging/bcm/Bcmchar.c
@@ -1456,8 +1456,7 @@ cntrlEnd:
case IOCTL_BCM_FLASH2X_SECTION_READ: {
struct bcm_flash2x_readwrite sFlash2xRead = {0};
PUCHAR pReadBuff = NULL;
- UINT NOB = 0;
- UINT BuffSize = 0;
+ UINT NOB;
UINT ReadBytes = 0;
UINT ReadOffset = 0;
void __user *OutPutBuff;
@@ -1480,19 +1479,17 @@ cntrlEnd:
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG,
DBG_LVL_ALL, "\nsFlash2xRead.numOfBytes :%x", sFlash2xRead.numOfBytes);
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, OSAL_DBG,
DBG_LVL_ALL, "\nsFlash2xRead.bVerify :%x\n", sFlash2xRead.bVerify);
+ if (sFlash2xRead.numOfBytes > Adapter->uiSectorSize)
+ sFlash2xRead.numOfBytes = Adapter->uiSectorSize;
+ NOB = sFlash2xRead.numOfBytes;
+
/* This was internal to driver for raw read. now it has ben
exposed to user space app. */
if (validateFlash2xReadWrite(Adapter, &sFlash2xRead) == false)
return STATUS_FAILURE;
- NOB = sFlash2xRead.numOfBytes;
- if (NOB > Adapter->uiSectorSize)
- BuffSize = Adapter->uiSectorSize;
- else
- BuffSize = NOB;
-
ReadOffset = sFlash2xRead.offset;
OutPutBuff = IoBuffer.OutputBuffer;
- pReadBuff = (PCHAR)kzalloc(BuffSize , GFP_KERNEL);
+ pReadBuff = (PCHAR)kzalloc(sFlash2xRead.numOfBytes, GFP_KERNEL);
if (pReadBuff == NULL) {
BCM_DEBUG_PRINT(Adapter, DBG_TYPE_PRINTK, 0, 0, "Memory
allocation failed for Flash 2.x Read Structure");
@@ -1548,8 +1545,7 @@ cntrlEnd:
struct bcm_flash2x_readwrite sFlash2xWrite = {0};
PUCHAR pWriteBuff;
void __user *InputAddr;
- UINT NOB = 0;
- UINT BuffSize = 0;
+ UINT NOB;
UINT WriteOffset = 0;
UINT WriteBytes = 0;
@@ -1580,19 +1576,17 @@ cntrlEnd:
return -EINVAL;
}
+ if (sFlash2xWrite.numOfBytes > Adapter->uiSectorSize)
+ sFlash2xWrite.numOfBytes = Adapter->uiSectorSize;
+ NOB = sFlash2xWrite.numOfBytes;
+
if (validateFlash2xReadWrite(Adapter, &sFlash2xWrite) == false)
return STATUS_FAILURE;
InputAddr = sFlash2xWrite.pDataBuff;
WriteOffset = sFlash2xWrite.offset;
- NOB = sFlash2xWrite.numOfBytes;
-
- if (NOB > Adapter->uiSectorSize)
- BuffSize = Adapter->uiSectorSize;
- else
- BuffSize = NOB;
- pWriteBuff = kmalloc(BuffSize, GFP_KERNEL);
+ pWriteBuff = kmalloc(sFlash2xWrite.numOfBytes, GFP_KERNEL);
if (pWriteBuff == NULL)
return -ENOMEM;
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
