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
