Messages in this thread | | | Date | Fri, 12 Nov 2010 11:14:26 +0100 | Subject | Re: [PATCH] staging: ft1000: Copy from user into correct data | From | Belisko Marek <> |
| |
Hi Steven,
On Thu, Nov 11, 2010 at 5:29 PM, Steven Rostedt <rostedt@goodmis.org> wrote: > While doing a ktest.pl I used a MIN_CONFIG that had STAGING enabled, and > a randconfig with CONFIG_DEBUG_STRICT_USER_COPY_CHECKS enabled caught > the following bug: > > In file included from /home/rostedt/work/autotest/nobackup/linux-test.git/arch/x86/include/asm/uaccess.h:571:0, > from /home/rostedt/work/autotest/nobackup/linux-test.git/include/linux/poll.h:14, > from /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/staging/ft1000/ft1000-usb/ft1000_chdev.c:32: > In function 'copy_from_user', > inlined from 'ft1000_ChIoctl' at /home/rostedt/work/autotest/nobackup/linux-test.git/drivers/staging/ft1000/ft1000-usb/ft1000_chdev.c:702:36: > /home/rostedt/work/autotest/nobackup/linux-test.git/arch/x86/include/asm/uaccess_32.h:212:26: error: call to 'copy_from_user_overflow' declared with attribute error: copy_from_user() buffer size is not provably correct > > > Looking at the code it was obvious what the problem was. The pointer > dpram_data was being allocated but the address was being written to. > Looking at the comment above the code shows that it use to write into an > element of that pointer where the '&' is appropriate. But now that it > writes to the pointer itself, we need to remove the '&' otherwise we > write over the pointer and not into the data it points to. Good catch but anyway this interface is removed. Already send patches to Greg-KH but there was some patch problem so they're not applied to next yet. Thanks for effort. > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > > diff --git a/drivers/staging/ft1000/ft1000-usb/ft1000_chdev.c b/drivers/staging/ft1000/ft1000-usb/ft1000_chdev.c > index 87a6487..8e8197d 100644 > --- a/drivers/staging/ft1000/ft1000-usb/ft1000_chdev.c > +++ b/drivers/staging/ft1000/ft1000-usb/ft1000_chdev.c > @@ -699,7 +699,7 @@ static long ft1000_ChIoctl (struct file *File, unsigned int Command, > break; > > //if ( copy_from_user(&(dpram_command.dpram_blk), (PIOCTL_DPRAM_BLK)Argument, msgsz+2) ) { > - if ( copy_from_user(&dpram_data, argp, msgsz+2) ) { > + if ( copy_from_user(dpram_data, argp, msgsz+2) ) { > DEBUG("FT1000:ft1000_ChIoctl: copy fault occurred\n"); > result = -EFAULT; > } > > >
thanks,
marek
-- as simple and primitive as possible ------------------------------------------------- Marek Belisko - OPEN-NANDRA Freelance Developer
Ruska Nova Ves 219 | Presov, 08005 Slovak Republic Tel: +421 915 052 184 skype: marekwhite icq: 290551086 web: http://open-nandra.com -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |