From: Ian Abbott <abbotti@mev.co.uk>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Ian Abbott <ian.abbott@mev.co.uk>,
H Hartley Sweeten <hartleys@visionengravers.com>,
"devel@driverdev.osuosl.org" <devel@driverdev.osuosl.org>,
"fmhess@users.sourceforge.net" <fmhess@users.sourceforge.net>,
Linux Kernel <linux-kernel@vger.kernel.org>,
"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
Subject: Re: [PATCH] staging: comedi: remove __user annotation inside of struct's
Date: Wed, 9 May 2012 12:01:25 +0100 [thread overview]
Message-ID: <4FAA4E85.6020305@mev.co.uk> (raw)
In-Reply-To: <20120509103111.GR22134@mwanda>
On 2012-05-09 11:31, Dan Carpenter wrote:
> On Wed, May 09, 2012 at 11:20:07AM +0100, Ian Abbott wrote:
>> On 2012-05-09 00:55, H Hartley Sweeten wrote:
>>> On Tuesday, May 08, 2012 4:41 PM, H Hartley Sweeten wrote:
>>>>
>>>> The structs' comedi_insn, coomedi_insnlist, comedi_cmd,
>>>> comedi_chaninfo, and comedi_rangeinfo are all passed to
>>>> the kernel from user space using ioctl commands. They
>>>> are then copied to kernel space using copy_from_user()
>>>> before the data is passed to the drivers.
>>>>
>>>> The __user annotation should not be used with variables
>>>> inside the struct. This produces a lot of sparse warnings
>>>> like:
>>>>
>>>> warning: dereference of noderef expression
>>>>
>>>> Note: This patch exposes some new warnings about different
>>>> address space. These will be addressed.
>>>
>>> Please ignore this patch.
>>>
>>> It appears the annotations in the struct definitions are correct.
>>
>> Personally, I think you were on the mark with the patch. It's
>> better to avoid using __user in comedi.h so it can be used as-is in
>> user-space.
>
> Sparse is useful so we shouldn't break it. I always run sparse over
> my patches before submission and look at the warnings. Except if
> they scroll off the page. In that case, I just figure that the
> author deserves the bugs.
>
> We could just do some ifdeferry to fix it for userspace.
That doesn't help in cases such as 'struct comedi_insn' where the 'data'
pointer is a user-space pointer in the user-space copy of the object and
a kernel-space pointer in the kernel-space copy of the object. The only
fix for that is to have separate "k" versions of the struct or to do a
load of casting, which is slightly error-prone and makes the code less
readable.
Are there any handy macros for casting pointers to __user pointers,
something like
#define _user(p) ((typeof(*(p)) __user *)(p))
but preferably without the repeated expansion of 'p' in case of
side-effects?
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
next prev parent reply other threads:[~2012-05-09 11:01 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-08 23:41 [PATCH] staging: comedi: remove __user annotation inside of struct's H Hartley Sweeten
2012-05-08 23:55 ` H Hartley Sweeten
2012-05-09 10:20 ` Ian Abbott
2012-05-09 10:31 ` Dan Carpenter
2012-05-09 11:01 ` Ian Abbott [this message]
2012-05-09 14:19 ` Dan Carpenter
2012-05-10 11:05 ` Ian Abbott
2012-05-09 14:24 ` gregkh
2012-05-09 15:52 ` H Hartley Sweeten
2012-05-09 15:56 ` gregkh
2012-05-09 16:03 ` H Hartley Sweeten
2012-05-09 16:41 ` H Hartley Sweeten
2012-05-09 20:42 ` gregkh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4FAA4E85.6020305@mev.co.uk \
--to=abbotti@mev.co.uk \
--cc=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=fmhess@users.sourceforge.net \
--cc=gregkh@linuxfoundation.org \
--cc=hartleys@visionengravers.com \
--cc=ian.abbott@mev.co.uk \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.