bcm driver copies from userpace with the following statement:

copy_from_user(&RxCntrlMsgBitMask, IoBuffer.InputBuffer, IoBuffer.InputLength);

Compiling gives the following warning:

at drivers/staging/bcm/Bcmchar.c:2030:
linux-next/arch/x86/include/asm/uaccess_32.h:212: warning: call to
copy_from_user_overflow declared with attribute warning: copy_from_user()
buffer size is not provably correct.

RxCntrlMsgBitMask is of type unsigned long so we have to check that
IoBuffer.InputLength is equal to sizeof(unsigned long).

Signed-off-by: Javier Martinez Canillas <[email protected]>
---

 V2: Check the user provided length recommended by Stephen Hemminger
     Check for copy_from_user() and return -EFAULT; do not use ULONG and 
     get rid of unnecessary log information recommended by Dan Carpenter.
 V3: Check that user provided length is equal to sizeof(unsigned long) 
     recommended by Dan Carpenter. Also, get rid of the innecesary scope 
     brackets in the switch case.

 drivers/staging/bcm/Bcmchar.c |   46 +++++++++++++++++++++++-----------------
 1 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c
index 31674ea..efea42e 100644
--- a/drivers/staging/bcm/Bcmchar.c
+++ b/drivers/staging/bcm/Bcmchar.c
@@ -163,6 +163,7 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, 
ULONG arg)
        INT                     Status = STATUS_FAILURE;
        int timeout = 0;
        IOCTL_BUFFER    IoBuffer;
+       unsigned long RxCntrlMsgBitMask;
 
        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);
 
@@ -2015,28 +2016,33 @@ static long bcm_char_ioctl(struct file *filp, UINT cmd, 
ULONG arg)
                                break ;
                         }
 
-               case IOCTL_BCM_CNTRLMSG_MASK:
-                        {
-                               ULONG RxCntrlMsgBitMask = 0 ;
+       case IOCTL_BCM_CNTRLMSG_MASK:
+               RxCntrlMsgBitMask = 0;
+               /* Copy Ioctl Buffer structure */
+               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;
+               }
 
-                               /* Copy Ioctl Buffer structure */
-                               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");
-                                       break;
-                               }
+               if (IoBuffer.InputLength != sizeof(unsigned long)) {
+                       Status = -EINVAL;
+                       break;
+               }
 
-                               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");
-                                       break;
-                               }
-                               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 ;
-                        }
-                        break;
+               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;
+               }
+
+               pTarang->RxCntrlMsgBitMask = RxCntrlMsgBitMask;
+               break;
                        case IOCTL_BCM_GET_DEVICE_DRIVER_INFO:
                        {
                                DEVICE_DRIVER_INFO DevInfo;
-- 
1.7.0.4

_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Reply via email to