On Sun, 23 Jan 2011 17:34:46 +0100 Javier Martinez Canillas <[email protected]> wrote:
> bcm driver copies from userpace with the following statement: > > copy_from_user(&RxCntrlMsgBitMask, IoBuffer.InputBuffer, > IoBuffer.InputLength); > > Compiling gives the following warning: > > In function ‘copy_from_user’, > inlined from ‘bcm_char_ioctl’ 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 it is wrong > to copy InputLenght bytes. Only we have to copy sizeof(unsigned long) bytes. > > This patch solves the issue and also make some style cleanups. > > Signed-off-by: Javier Martinez Canillas <[email protected]> > --- > drivers/staging/bcm/Bcmchar.c | 44 ++++++++++++++++++++++------------------ > 1 files changed, 24 insertions(+), 20 deletions(-) > > diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c > index 31674ea..c78afe8 100644 > --- a/drivers/staging/bcm/Bcmchar.c > +++ b/drivers/staging/bcm/Bcmchar.c > @@ -2015,28 +2015,32 @@ 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: > + { > + ULONG 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"); > - 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; > + } > > - 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, > + sizeof(ULONG)); > + 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; > case IOCTL_BCM_GET_DEVICE_DRIVER_INFO: > { > DEVICE_DRIVER_INFO DevInfo; Close to correct but: 1. Don't mix indentation changes with code changes. 2. The user provided length should also be checked. for example: if (IoBuffer.InputLength < sizeof(ULONG)) { ... } _______________________________________________ devel mailing list [email protected] http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
