On Sun, 23 Jan 2011 17:34:46 +0100
Javier Martinez Canillas <martinez.javier@xxxxxxxxx> 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 <martinez.javier@xxxxxxxxx>
> ---
> 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
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
