From: laurent.pinchart@ideasonboard•com (Laurent Pinchart)
To: linux-arm-kernel@lists•infradead.org
Subject: <Query> Looking more details and reasons for using orig_add_limit.
Date: Thu, 23 Feb 2017 02:25:53 +0200 [thread overview]
Message-ID: <1721361.FT1A3EpsKm@avalon> (raw)
In-Reply-To: <20170222172541.49b7cbb1@vento.lan>
Hi Mauro,
On Wednesday 22 Feb 2017 17:25:41 Mauro Carvalho Chehab wrote:
> Em Wed, 22 Feb 2017 21:53:08 +0200 Laurent Pinchart escreveu:
> > On Tuesday 21 Feb 2017 06:20:58 Sodagudi Prasad wrote:
> >> Hi mchehab/linux-media,
> >>
> >> It is not clear why KERNEL_DS was set explicitly here. In this path
> >> video_usercopy() gets called and it
> >> copies the ?struct v4l2_buffer? struct to user space stack memory.
> >>
> >> Can you please share reasons for setting to KERNEL_DS here?
> >
> > It's a bit of historical hack. To implement compat ioctl handling, we copy
> > the ioctl 32-bit argument from userspace, turn it into a native 64-bit
> > ioctl argument, and call the native ioctl code. That code expects the
> > argument to be stored in userspace memory and uses get_user() and
> > put_user() to access it. As the 64-bit argument now lives in kernel
> > memory, my understanding is that we fake things up with KERNEL_DS.
>
> Precisely. Actually, if I remember well, this was needed to pass pointer
> arguments from 32 bits userspace to 64 bits kernelspace. There are a lot of
> V4L2 ioctls that pass structures with pointers on it. Setting DS cause
> those pointers to do the right thing, but yeah, it is hackish.
We should restructure the core ioctl code to decouple copy from/to user and
ioctl execution (this might just be a matter of exporting a currently static
function), and change the compat code to perform the copy/from to user
directly when converting between 32-bit and 64-bit structures (dropping all
the alloc in userspace hacks) and call the ioctl execution handler. That will
fix the problem. Any volunteer ? :-)
> This used to work fine on x86_64 (when such code was written e. g. Kernel
> 2.6.1x). I never tested myself on ARM64, but I guess it used to work, as we
> received some patches fixing support for some ioctl compat code due to
> x86_64/arm64 differences in the past.
>
> On what Kernel version it started to cause troubles? 4.9? If so, then
> maybe the breakage is a side effect of VM stack changes.
>
> > The ioctl code should be refactored to get rid of this hack.
>
> Agreed.
--
Regards,
Laurent Pinchart
next prev parent reply other threads:[~2017-02-23 0:25 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-15 5:52 <Query> Looking more details and reasons for using orig_add_limit Sodagudi Prasad
2017-02-15 11:38 ` Will Deacon
2017-02-15 12:09 ` James Morse
2017-02-15 21:12 ` Sodagudi Prasad
2017-02-16 10:39 ` James Morse
2017-02-21 14:20 ` Sodagudi Prasad
2017-02-22 19:53 ` Laurent Pinchart
2017-02-22 20:25 ` Mauro Carvalho Chehab
2017-02-23 0:25 ` Laurent Pinchart [this message]
2017-02-22 19:53 ` Mauro Carvalho Chehab
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=1721361.FT1A3EpsKm@avalon \
--to=laurent.pinchart@ideasonboard$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox