From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-f195.google.com (mail-qt1-f195.google.com [209.85.160.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BC1D921A02937 for ; Mon, 24 Sep 2018 13:19:10 -0700 (PDT) Received: by mail-qt1-f195.google.com with SMTP id z8-v6so10940054qto.9 for ; Mon, 24 Sep 2018 13:19:10 -0700 (PDT) MIME-Version: 1.0 References: <20180912150142.157913-1-arnd@arndb.de> <20180912151134.436719-1-arnd@arndb.de> <20180914203506.GE35251@wrath> <20180914205748.GC19965@ZenIV.linux.org.uk> <20180918175108.GF35251@wrath> <20180918175952.GJ11367@ziepe.ca> In-Reply-To: <20180918175952.GJ11367@ziepe.ca> From: Arnd Bergmann Date: Mon, 24 Sep 2018 22:18:52 +0200 Message-ID: Subject: Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-nvdimm-bounces@lists.01.org Sender: "Linux-nvdimm" To: Jason Gunthorpe Cc: linux-fbdev@vger.kernel.org, linux-iio@vger.kernel.org, linux-pci , linux-remoteproc@vger.kernel.org, linux-nvme@lists.infradead.org, Platform Driver , sparclinux , driverdevel , linux-scsi , linux-nvdimm@lists.01.org, linux-rdma , qat-linux@intel.com, amd-gfx@lists.freedesktop.org, "open list:HID CORE LAYER" , Darren Hart , Linux Media Mailing List , linaro-mm-sig@lists.linaro.org, dri-devel , ceph-devel , gregkh , USB list , linux-wireless , Linux Kernel Mailing List , "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" , Networking , Linux FS-devel Mailing List , linuxppc-dev , David Miller , linux-btrfs , Al Viro List-ID: On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > Acked-by: Darren Hart (VMware) > > > > > > > > As for a longer term solution, would it be possible to init fops in such > > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > structure? > > > > > > Bad idea, that... Because several years down the road somebody will add > > > an ioctl that takes an unsigned int for argument. Without so much as looking > > > at your magical mystery macro being used to initialize file_operations. > > > > Fair, being explicit in the declaration as it is currently may be > > preferable then. > > It would be much cleaner and safer if you could arrange things to add > something like this to struct file_operations: > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > Where the core code automatically converts the unsigned long to the > void __user * as appropriate. > > Then it just works right always and the compiler will help address > Al's concern down the road. I think if we wanted to do this with a new file operation, the best way would be to do the copy_from_user()/copy_to_user() in the caller as well. We already do this inside of some subsystems, notably drivers/media/, and it simplifies the implementation of the ioctl handler function significantly. We obviously cannot do this in general, both because of traditional drivers that have 16-bit command codes (drivers/tty and others) and also because of drivers that by accident defined the commands incorrectly and use the wrong type or the wrong direction in the definition. Arnd _______________________________________________ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 References: <20180912150142.157913-1-arnd@arndb.de> <20180912151134.436719-1-arnd@arndb.de> <20180914203506.GE35251@wrath> <20180914205748.GC19965@ZenIV.linux.org.uk> <20180918175108.GF35251@wrath> <20180918175952.GJ11367@ziepe.ca> In-Reply-To: <20180918175952.GJ11367@ziepe.ca> From: Arnd Bergmann Date: Mon, 24 Sep 2018 22:18:52 +0200 Message-ID: Subject: Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Content-Type: text/plain; charset="UTF-8" Sender: netdev-owner@vger.kernel.org To: Jason Gunthorpe Cc: Darren Hart , Al Viro , Linux FS-devel Mailing List , gregkh , David Miller , driverdevel , Linux Kernel Mailing List , qat-linux@intel.com, "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" , Linux Media Mailing List , dri-devel , linaro-mm-sig@lists.linaro.org, amd-gfx@lists.freedesktop.org, "open list:HID CORE LAYER" , linux-iio@vger.kernel.org, linux-rdma , linux-nvdimm@lists.01.org, linux-nvme@lists.infradead.org, linux-pci , Platform Driver , linux-remoteproc@vger.kernel.org, sparclinux , linux-scsi , USB list , linux-fbdev@vger.kernel.org, linuxppc-dev , linux-btrfs , ceph-devel , linux-wireless , Networking List-ID: On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > Acked-by: Darren Hart (VMware) > > > > > > > > As for a longer term solution, would it be possible to init fops in such > > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > structure? > > > > > > Bad idea, that... Because several years down the road somebody will add > > > an ioctl that takes an unsigned int for argument. Without so much as looking > > > at your magical mystery macro being used to initialize file_operations. > > > > Fair, being explicit in the declaration as it is currently may be > > preferable then. > > It would be much cleaner and safer if you could arrange things to add > something like this to struct file_operations: > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > Where the core code automatically converts the unsigned long to the > void __user * as appropriate. > > Then it just works right always and the compiler will help address > Al's concern down the road. I think if we wanted to do this with a new file operation, the best way would be to do the copy_from_user()/copy_to_user() in the caller as well. We already do this inside of some subsystems, notably drivers/media/, and it simplifies the implementation of the ioctl handler function significantly. We obviously cannot do this in general, both because of traditional drivers that have 16-bit command codes (drivers/tty and others) and also because of drivers that by accident defined the commands incorrectly and use the wrong type or the wrong direction in the definition. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6E407ECE568 for ; Mon, 24 Sep 2018 20:19:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1E2EB2087A for ; Mon, 24 Sep 2018 20:19:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1E2EB2087A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arndb.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-wireless-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727453AbeIYCXI (ORCPT ); Mon, 24 Sep 2018 22:23:08 -0400 Received: from mail-qt1-f196.google.com ([209.85.160.196]:37611 "EHLO mail-qt1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726316AbeIYCXH (ORCPT ); Mon, 24 Sep 2018 22:23:07 -0400 Received: by mail-qt1-f196.google.com with SMTP id n6-v6so10943050qtl.4; Mon, 24 Sep 2018 13:19:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OsYidU1hlYJ+klx90AxIRQwB+6wMrrJr/JJ5pU6q0MY=; b=Bq/5Ah6K21QrKgRd+sjq/p5dJ91R2dGXcpqxjIP6Bg+7vpN08kjY+Dt47M4ZWLQpy7 jWD2Xl247Vp9nhJwHCKu4MLoc/EaiAJKxWCbQTyBPprQ3k0pW3+ANb0vjgsMD/Z997c3 TTQErOEhXe37g034dqRyqZnxjDcbV1O5xAvhemXC0trlHhq8VTSBTaXu/lCeZNMzLdwF f7arq0xyFJiacUrcyltM3Jq9mrkP0ImHO0BXL5vt10A1ggljVrqJJhAOH/MD2YwFTJg3 /+K/bIdKv6ZCGQxQGe3C/o10WAVA0/aUOVl11Oud2OwxwgE+VuN73daPLfyknval13lO Gsfw== X-Gm-Message-State: ABuFfohThuABX9vh8S5yordOs4kPicfzJH7PP/gcS6K9DccXXnG2e6nk a05V5KLBS8tfiTJs1N6C94upuBmoFFwCm9LwbLQ= X-Google-Smtp-Source: ACcGV60Ps3U5LLCo0RjeCpaPr9VTAwVS7tqdLJYGXjikqVVB7pTkjFqFcc8hPBx92H8kqqx7rZwu+cIOb887IRHt2U8= X-Received: by 2002:a0c:a8cc:: with SMTP id h12-v6mr393998qvc.161.1537820349291; Mon, 24 Sep 2018 13:19:09 -0700 (PDT) MIME-Version: 1.0 References: <20180912150142.157913-1-arnd@arndb.de> <20180912151134.436719-1-arnd@arndb.de> <20180914203506.GE35251@wrath> <20180914205748.GC19965@ZenIV.linux.org.uk> <20180918175108.GF35251@wrath> <20180918175952.GJ11367@ziepe.ca> In-Reply-To: <20180918175952.GJ11367@ziepe.ca> From: Arnd Bergmann Date: Mon, 24 Sep 2018 22:18:52 +0200 Message-ID: Subject: Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg To: Jason Gunthorpe Cc: Darren Hart , Al Viro , Linux FS-devel Mailing List , gregkh , David Miller , driverdevel , Linux Kernel Mailing List , qat-linux@intel.com, "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" , Linux Media Mailing List , dri-devel , linaro-mm-sig@lists.linaro.org, amd-gfx@lists.freedesktop.org, "open list:HID CORE LAYER" , linux-iio@vger.kernel.org, linux-rdma , linux-nvdimm@lists.01.org, linux-nvme@lists.infradead.org, linux-pci , Platform Driver , linux-remoteproc@vger.kernel.org, sparclinux , linux-scsi , USB list , linux-fbdev@vger.kernel.org, linuxppc-dev , linux-btrfs , ceph-devel , linux-wireless , Networking Content-Type: text/plain; charset="UTF-8" Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > Acked-by: Darren Hart (VMware) > > > > > > > > As for a longer term solution, would it be possible to init fops in such > > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > structure? > > > > > > Bad idea, that... Because several years down the road somebody will add > > > an ioctl that takes an unsigned int for argument. Without so much as looking > > > at your magical mystery macro being used to initialize file_operations. > > > > Fair, being explicit in the declaration as it is currently may be > > preferable then. > > It would be much cleaner and safer if you could arrange things to add > something like this to struct file_operations: > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > Where the core code automatically converts the unsigned long to the > void __user * as appropriate. > > Then it just works right always and the compiler will help address > Al's concern down the road. I think if we wanted to do this with a new file operation, the best way would be to do the copy_from_user()/copy_to_user() in the caller as well. We already do this inside of some subsystems, notably drivers/media/, and it simplifies the implementation of the ioctl handler function significantly. We obviously cannot do this in general, both because of traditional drivers that have 16-bit command codes (drivers/tty and others) and also because of drivers that by accident defined the commands incorrectly and use the wrong type or the wrong direction in the definition. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Date: Mon, 24 Sep 2018 22:18:52 +0200 Message-ID: References: <20180912150142.157913-1-arnd@arndb.de> <20180912151134.436719-1-arnd@arndb.de> <20180914203506.GE35251@wrath> <20180914205748.GC19965@ZenIV.linux.org.uk> <20180918175108.GF35251@wrath> <20180918175952.GJ11367@ziepe.ca> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-pci , linux-remoteproc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Platform Driver , sparclinux , driverdevel , linux-scsi , linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org, linux-rdma , qat-linux-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, "open list:HID CORE LAYER" , Darren Hart , Linux Media Mailing List , linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw@public.gmane.org, dri-devel , ceph-devel , gregkh , USB list , linux-wireless , Linux Kernel Mailing List Return-path: In-Reply-To: <20180918175952.GJ11367-uk2M96/98Pc@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org Sender: "Linux-nvdimm" List-Id: linux-crypto.vger.kernel.org On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > Acked-by: Darren Hart (VMware) > > > > > > > > As for a longer term solution, would it be possible to init fops in such > > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > structure? > > > > > > Bad idea, that... Because several years down the road somebody will add > > > an ioctl that takes an unsigned int for argument. Without so much as looking > > > at your magical mystery macro being used to initialize file_operations. > > > > Fair, being explicit in the declaration as it is currently may be > > preferable then. > > It would be much cleaner and safer if you could arrange things to add > something like this to struct file_operations: > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > Where the core code automatically converts the unsigned long to the > void __user * as appropriate. > > Then it just works right always and the compiler will help address > Al's concern down the road. I think if we wanted to do this with a new file operation, the best way would be to do the copy_from_user()/copy_to_user() in the caller as well. We already do this inside of some subsystems, notably drivers/media/, and it simplifies the implementation of the ioctl handler function significantly. We obviously cannot do this in general, both because of traditional drivers that have 16-bit command codes (drivers/tty and others) and also because of drivers that by accident defined the commands incorrectly and use the wrong type or the wrong direction in the definition. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Date: Mon, 24 Sep 2018 22:18:52 +0200 Message-ID: References: <20180912150142.157913-1-arnd@arndb.de> <20180912151134.436719-1-arnd@arndb.de> <20180914203506.GE35251@wrath> <20180914205748.GC19965@ZenIV.linux.org.uk> <20180918175108.GF35251@wrath> <20180918175952.GJ11367@ziepe.ca> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20180918175952.GJ11367-uk2M96/98Pc@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linux-nvdimm-bounces-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org Sender: "Linux-nvdimm" To: Jason Gunthorpe Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-pci , linux-remoteproc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Platform Driver , sparclinux , driverdevel , linux-scsi , linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org, linux-rdma , qat-linux-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, "open list:HID CORE LAYER" , Darren Hart , Linux Media Mailing List , linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw@public.gmane.org, dri-devel , ceph-devel , gregkh , USB list , linux-wireless Linux Kernel Mailing List List-Id: linux-rdma@vger.kernel.org On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > Acked-by: Darren Hart (VMware) > > > > > > > > As for a longer term solution, would it be possible to init fops in such > > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > structure? > > > > > > Bad idea, that... Because several years down the road somebody will add > > > an ioctl that takes an unsigned int for argument. Without so much as looking > > > at your magical mystery macro being used to initialize file_operations. > > > > Fair, being explicit in the declaration as it is currently may be > > preferable then. > > It would be much cleaner and safer if you could arrange things to add > something like this to struct file_operations: > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > Where the core code automatically converts the unsigned long to the > void __user * as appropriate. > > Then it just works right always and the compiler will help address > Al's concern down the road. I think if we wanted to do this with a new file operation, the best way would be to do the copy_from_user()/copy_to_user() in the caller as well. We already do this inside of some subsystems, notably drivers/media/, and it simplifies the implementation of the ioctl handler function significantly. We obviously cannot do this in general, both because of traditional drivers that have 16-bit command codes (drivers/tty and others) and also because of drivers that by accident defined the commands incorrectly and use the wrong type or the wrong direction in the definition. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [v2,05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg From: Arnd Bergmann Message-Id: Date: Mon, 24 Sep 2018 22:18:52 +0200 To: Jason Gunthorpe Cc: Darren Hart , Al Viro , Linux FS-devel Mailing List , gregkh , David Miller , driverdevel , Linux Kernel Mailing List , qat-linux@intel.com, "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" , Linux Media Mailing List , dri-devel , linaro-mm-sig@lists.linaro.org, amd-gfx@lists.freedesktop.org, "open list:HID CORE LAYER" , linux-iio@vger.kernel.org, linux-rdma , linux-nvdimm@lists.01.org, linux-nvme@lists.infradead.org, linux-pci , Platform Driver , linux-remoteproc@vger.kernel.org, sparclinux , linux-scsi , USB list , linux-fbdev@vger.kernel.org, linuxppc-dev , linux-btrfs , ceph-devel , linux-wireless , Networking List-ID: T24gVHVlLCBTZXAgMTgsIDIwMTggYXQgNzo1OSBQTSBKYXNvbiBHdW50aG9ycGUgPGpnZ0B6aWVw ZS5jYT4gd3JvdGU6Cj4KPiBPbiBUdWUsIFNlcCAxOCwgMjAxOCBhdCAxMDo1MTowOEFNIC0wNzAw LCBEYXJyZW4gSGFydCB3cm90ZToKPiA+IE9uIEZyaSwgU2VwIDE0LCAyMDE4IGF0IDA5OjU3OjQ4 UE0gKzAxMDAsIEFsIFZpcm8gd3JvdGU6Cj4gPiA+IE9uIEZyaSwgU2VwIDE0LCAyMDE4IGF0IDAx OjM1OjA2UE0gLTA3MDAsIERhcnJlbiBIYXJ0IHdyb3RlOgo+ID4gPgo+ID4gPiA+IEFja2VkLWJ5 OiBEYXJyZW4gSGFydCAoVk13YXJlKSA8ZHZoYXJ0QGluZnJhZGVhZC5vcmc+Cj4gPiA+ID4KPiA+ ID4gPiBBcyBmb3IgYSBsb25nZXIgdGVybSBzb2x1dGlvbiwgd291bGQgaXQgYmUgcG9zc2libGUg dG8gaW5pdCBmb3BzIGluIHN1Y2gKPiA+ID4gPiBhIHdheSB0aGF0IHRoZSBjb21wYXRfaW9jdGwg Y2FsbCBkZWZhdWx0cyB0byBnZW5lcmljX2NvbXBhdF9pb2N0bF9wdHJhcmcKPiA+ID4gPiBzbyB3 ZSBkb24ndCBoYXZlIHRvIGR1cGxpY2F0ZSB0aGlzIGJvaWxlcnBsYXRlIGZvciBldmVyeSBpb2N0 bCBmb3BzCj4gPiA+ID4gc3RydWN0dXJlPwo+ID4gPgo+ID4gPiAgICAgQmFkIGlkZWEsIHRoYXQu Li4gIEJlY2F1c2Ugc2V2ZXJhbCB5ZWFycyBkb3duIHRoZSByb2FkIHNvbWVib2R5IHdpbGwgYWRk Cj4gPiA+IGFuIGlvY3RsIHRoYXQgdGFrZXMgYW4gdW5zaWduZWQgaW50IGZvciBhcmd1bWVudC4g IFdpdGhvdXQgc28gbXVjaCBhcyBsb29raW5nCj4gPiA+IGF0IHlvdXIgbWFnaWNhbCBteXN0ZXJ5 IG1hY3JvIGJlaW5nIHVzZWQgdG8gaW5pdGlhbGl6ZSBmaWxlX29wZXJhdGlvbnMuCj4gPgo+ID4g RmFpciwgYmVpbmcgZXhwbGljaXQgaW4gdGhlIGRlY2xhcmF0aW9uIGFzIGl0IGlzIGN1cnJlbnRs eSBtYXkgYmUKPiA+IHByZWZlcmFibGUgdGhlbi4KPgo+IEl0IHdvdWxkIGJlIG11Y2ggY2xlYW5l ciBhbmQgc2FmZXIgaWYgeW91IGNvdWxkIGFycmFuZ2UgdGhpbmdzIHRvIGFkZAo+IHNvbWV0aGlu ZyBsaWtlIHRoaXMgdG8gc3RydWN0IGZpbGVfb3BlcmF0aW9uczoKPgo+ICAgbG9uZyAoKnB0cl9p b2N0bCkgKHN0cnVjdCBmaWxlICosIHVuc2lnbmVkIGludCwgdm9pZCBfX3VzZXIgKik7Cj4KPiBX aGVyZSB0aGUgY29yZSBjb2RlIGF1dG9tYXRpY2FsbHkgY29udmVydHMgdGhlIHVuc2lnbmVkIGxv bmcgdG8gdGhlCj4gdm9pZCBfX3VzZXIgKiBhcyBhcHByb3ByaWF0ZS4KPgo+IFRoZW4gaXQganVz dCB3b3JrcyByaWdodCBhbHdheXMgYW5kIHRoZSBjb21waWxlciB3aWxsIGhlbHAgYWRkcmVzcwo+ IEFsJ3MgY29uY2VybiBkb3duIHRoZSByb2FkLgoKSSB0aGluayBpZiB3ZSB3YW50ZWQgdG8gZG8g dGhpcyB3aXRoIGEgbmV3IGZpbGUgb3BlcmF0aW9uLCB0aGUgYmVzdAp3YXkgd291bGQgYmUgdG8g ZG8gdGhlIGNvcHlfZnJvbV91c2VyKCkvY29weV90b191c2VyKCkgaW4gdGhlIGNhbGxlcgphcyB3 ZWxsLgoKV2UgYWxyZWFkeSBkbyB0aGlzIGluc2lkZSBvZiBzb21lIHN1YnN5c3RlbXMsIG5vdGFi bHkgZHJpdmVycy9tZWRpYS8sCmFuZCBpdCBzaW1wbGlmaWVzIHRoZSBpbXBsZW1lbnRhdGlvbiBv ZiB0aGUgaW9jdGwgaGFuZGxlciBmdW5jdGlvbgpzaWduaWZpY2FudGx5LiBXZSBvYnZpb3VzbHkg Y2Fubm90IGRvIHRoaXMgaW4gZ2VuZXJhbCwgYm90aCBiZWNhdXNlIG9mCnRyYWRpdGlvbmFsIGRy aXZlcnMgdGhhdCBoYXZlIDE2LWJpdCBjb21tYW5kIGNvZGVzIChkcml2ZXJzL3R0eSBhbmQgb3Ro ZXJzKQphbmQgYWxzbyBiZWNhdXNlIG9mIGRyaXZlcnMgdGhhdCBieSBhY2NpZGVudCBkZWZpbmVk IHRoZSBjb21tYW5kcwppbmNvcnJlY3RseSBhbmQgdXNlIHRoZSB3cm9uZyB0eXBlIG9yIHRoZSB3 cm9uZyBkaXJlY3Rpb24gaW4gdGhlCmRlZmluaXRpb24uCgogICAgICAgQXJuZAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Date: Mon, 24 Sep 2018 20:18:52 +0000 Subject: Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg Message-Id: List-Id: References: <20180912150142.157913-1-arnd@arndb.de> <20180912151134.436719-1-arnd@arndb.de> <20180914203506.GE35251@wrath> <20180914205748.GC19965@ZenIV.linux.org.uk> <20180918175108.GF35251@wrath> <20180918175952.GJ11367@ziepe.ca> In-Reply-To: <20180918175952.GJ11367-uk2M96/98Pc@public.gmane.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Jason Gunthorpe Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-pci , linux-remoteproc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Platform Driver , sparclinux , driverdevel , linux-scsi , linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org, linux-rdma , qat-linux-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, "open list:HID CORE LAYER" , Darren Hart , Linux Media Mailing List , linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw@public.gmane.org, dri-devel , ceph-devel , gregkh , USB list , linux-wireless On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe wrote: > > On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote: > > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote: > > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote: > > > > > > > Acked-by: Darren Hart (VMware) > > > > > > > > As for a longer term solution, would it be possible to init fops in such > > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg > > > > so we don't have to duplicate this boilerplate for every ioctl fops > > > > structure? > > > > > > Bad idea, that... Because several years down the road somebody will add > > > an ioctl that takes an unsigned int for argument. Without so much as looking > > > at your magical mystery macro being used to initialize file_operations. > > > > Fair, being explicit in the declaration as it is currently may be > > preferable then. > > It would be much cleaner and safer if you could arrange things to add > something like this to struct file_operations: > > long (*ptr_ioctl) (struct file *, unsigned int, void __user *); > > Where the core code automatically converts the unsigned long to the > void __user * as appropriate. > > Then it just works right always and the compiler will help address > Al's concern down the road. I think if we wanted to do this with a new file operation, the best way would be to do the copy_from_user()/copy_to_user() in the caller as well. We already do this inside of some subsystems, notably drivers/media/, and it simplifies the implementation of the ioctl handler function significantly. We obviously cannot do this in general, both because of traditional drivers that have 16-bit command codes (drivers/tty and others) and also because of drivers that by accident defined the commands incorrectly and use the wrong type or the wrong direction in the definition. Arnd