* [PATCH] staging: ft1000: Copy from user into correct data
@ 2010-11-11 16:29 Steven Rostedt
2010-11-12 10:14 ` Belisko Marek
0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2010-11-11 16:29 UTC (permalink / raw)
To: LKML; +Cc: Greg KH, Marek Belisko, Andrew Morton
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.
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;
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: ft1000: Copy from user into correct data
2010-11-11 16:29 [PATCH] staging: ft1000: Copy from user into correct data Steven Rostedt
@ 2010-11-12 10:14 ` Belisko Marek
2010-11-16 19:33 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Belisko Marek @ 2010-11-12 10:14 UTC (permalink / raw)
To: Steven Rostedt; +Cc: LKML, Greg KH, Andrew Morton
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: ft1000: Copy from user into correct data
2010-11-12 10:14 ` Belisko Marek
@ 2010-11-16 19:33 ` Greg KH
0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2010-11-16 19:33 UTC (permalink / raw)
To: Belisko Marek; +Cc: Steven Rostedt, LKML, Andrew Morton
On Fri, Nov 12, 2010 at 11:14:26AM +0100, Belisko Marek wrote:
> 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.
But this patch is correct, and I don't have your other patches in my
inbox, so I'm going to apply this one.
Please rebase your fixes on the next linux-next and all should be fine.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-11-16 19:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-11 16:29 [PATCH] staging: ft1000: Copy from user into correct data Steven Rostedt
2010-11-12 10:14 ` Belisko Marek
2010-11-16 19:33 ` Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.