Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1586462

Re: <Query> Looking more details and reasons for using orig_add_limit.

From Mauro Carvalho Chehab <mchehab@s-opensource.com>
Newsgroups linux.kernel
Subject Re: <Query> Looking more details and reasons for using orig_add_limit.
Date 2017-02-22 21:40 +0100
Message-ID <tdLRE-7eL-25@gated-at.bofh.it> (permalink)
References <tb0Nb-44T-3@gated-at.bofh.it> <tbrDI-4Tz-21@gated-at.bofh.it> <tdjC2-3LX-3@gated-at.bofh.it> <tdLeW-6IG-15@gated-at.bofh.it>
Organization Samsung

Show all headers | View raw


Em Wed, 22 Feb 2017 21:53:08 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> escreveu:

> Hi Prasad,
> 
> 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.

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.

Thanks,
Mauro

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

<Query> Looking more details and reasons for using orig_add_limit. Sodagudi Prasad <psodagud@codeaurora.org> - 2017-02-15 07:00 +0100
  Re: <Query> Looking more details and reasons for using  orig_add_limit. Will Deacon <will.deacon@arm.com> - 2017-02-15 12:40 +0100
  Re: <Query> Looking more details and reasons for using orig_add_limit. James Morse <james.morse@arm.com> - 2017-02-15 13:20 +0100
    Re: <Query> Looking more details and reasons for using  orig_add_limit. Sodagudi Prasad <psodagud@codeaurora.org> - 2017-02-15 22:20 +0100
      Re: <Query> Looking more details and reasons for using orig_add_limit. James Morse <james.morse@arm.com> - 2017-02-16 11:40 +0100
        Re: <Query> Looking more details and reasons for using  orig_add_limit. Sodagudi Prasad <psodagud@codeaurora.org> - 2017-02-21 15:30 +0100
          Re: <Query> Looking more details and reasons for using  orig_add_limit. Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-02-22 21:00 +0100
          Re: <Query> Looking more details and reasons for using orig_add_limit. Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2017-02-22 21:00 +0100
            Re: <Query> Looking more details and reasons for using  orig_add_limit. Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-02-22 21:40 +0100
              Re: <Query> Looking more details and reasons for using orig_add_limit. Laurent Pinchart <laurent.pinchart@ideasonboard.com> - 2017-02-23 01:30 +0100

csiph-web