linux-kernel - RE: [PATCH v2] staging: comedi: remove _

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 9 May 2012 16:21:15 -0500
From:	H Hartley Sweeten <hartleys@...ionengravers.com>
To:	Greg KH <gregkh@...uxfoundation.org>
CC:	Linux Kernel <linux-kernel@...r.kernel.org>,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"fmhess@...rs.sourceforge.net" <fmhess@...rs.sourceforge.net>,
	"abbotti@....co.uk" <abbotti@....co.uk>
Subject: RE: [PATCH v2] staging: comedi: remove __user annotation inside of
 struct's

On Wednesday, May 09, 2012 1:42 PM, Greg KH wrote:
> On Wed, May 09, 2012 at 09:36:03AM -0700, H Hartley Sweeten wrote:
>> Remove the __user annotations in the struct definitions in
>> comedi.h.
>> 
>> These structs are used to pass information from user-space
>> to kernel-space. The copy_from_user and copy_to_user functions
>> are used to transfer the data between the address spaces.
>> 
>> The drivers then use the information internally under the
>> assumption that they are kernel-space objects. Having the
>> __user annotations inside the structs produces a number of
>> sparse warnings of the type:
>> 
>> warning: dereference of noderef expression
>> 
>> According to Grek Kroah-Hartman:

Sorry about that typo... ;-)

>> 
>> "When the kernel exports .h files, stuff like this should work
>> "automatically", so there is no need to not put __user markings."
>
> Wait, for some reason I thought you were adding __user markings, not
> removing them from the structures.  That is why I said the above
> statement.

Ah.. Missed the "not" in your comment...

>
> Why again are you removing them?  Didn't you just cause more sparse
> warnings to pop up?

I still think they should be removed but I need to dig thru it a bit.

The structs in question are used internally in the comedi drivers after
the data has been passed into the kernel using the various ioctl's.
The ioctl code does do the copy_from/to_user correctly (kind of,
I did notice one place where the copy_from_user is referencing a
kernel pointer). But, when the driver's try to use the data, assuming
it's kernel space data, it's still annotated as __user so we get all the
"warning: dereference of noderef expression" along with a couple
other sparse warnings.

Ian Abbott did reply that he thought the patch was correct. With my
miss-read of your comment I thought you were agreeing.

Regardless, I'll keep looking it over...

Regards,
Hartley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/