From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934549AbdBWA02 (ORCPT ); Wed, 22 Feb 2017 19:26:28 -0500 Received: from galahad.ideasonboard.com ([185.26.127.97]:40553 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933883AbdBWA0R (ORCPT ); Wed, 22 Feb 2017 19:26:17 -0500 From: Laurent Pinchart To: Mauro Carvalho Chehab Cc: Sodagudi Prasad , James Morse , linux-media@vger.kernel.org, shijie.huang@arm.com, catalin.marinas@arm.com, will.deacon@arm.com, mark.rutland@arm.com, akpm@linux-foundation.org, sandeepa.s.prabhu@gmail.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, hans.verkuil@cisco.com, sakari.ailus@linux.intel.com, tiffany.lin@mediatek.com, nick@shmanahar.org, shuah@kernel.org, ricardo.ribalda@gmail.com Subject: Re: Looking more details and reasons for using orig_add_limit. Date: Thu, 23 Feb 2017 02:25:53 +0200 Message-ID: <1721361.FT1A3EpsKm@avalon> User-Agent: KMail/4.14.10 (Linux/4.9.6-gentoo-r1; KDE/4.14.28; x86_64; ; ) In-Reply-To: <20170222172541.49b7cbb1@vento.lan> References: <2944633.ljab0sy3Dg@avalon> <20170222172541.49b7cbb1@vento.lan> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v1N0QmpQ028928 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