From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl Date: Thu, 25 Apr 2019 17:42:35 +0100 Message-ID: <20190425164235.GT2217@ZenIV.linux.org.uk> References: <20190416202013.4034148-1-arnd@arndb.de> <20190416202701.127745-1-arnd@arndb.de> <20190425122153.450fc094@coco.lan> <20190425153534.GS2217@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: netdev-owner@vger.kernel.org To: Arnd Bergmann Cc: Mauro Carvalho Chehab , Linux FS-devel Mailing List , y2038 Mailman List , Linux Kernel Mailing List , Jason Gunthorpe , Daniel Vetter , Greg Kroah-Hartman , David Sterba , Darren Hart , Jonathan Cameron , Bjorn Andersson , driverdevel , qat-linux@intel.com, "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" , Linux Media Mailing List , dri-devel , linaro-mm-sig@lis List-Id: amd-gfx.lists.freedesktop.org On Thu, Apr 25, 2019 at 05:55:23PM +0200, Arnd Bergmann wrote: > On Thu, Apr 25, 2019 at 5:35 PM Al Viro wrote: > > > > On Thu, Apr 25, 2019 at 12:21:53PM -0300, Mauro Carvalho Chehab wrote: > > > > > If I understand your patch description well, using compat_ptr_ioctl > > > only works if the driver is not for s390, right? > > > > No; s390 is where "oh, just set ->compat_ioctl same as ->unlocked_ioctl > > and be done with that; compat_ptr() is a no-op anyway" breaks. IOW, > > s390 is the reason for having compat_ptr_ioctl() in the first place; > > that thing works on all biarch architectures, as long as all stuff > > handled by ->ioctl() takes pointer to arch-independent object as > > argument. IOW, > > argument ignored => OK > > any arithmetical type => no go, compat_ptr() would bugger it > > pointer to int => OK > > pointer to string => OK > > pointer to u64 => OK > > pointer to struct {u64 addr; char s[11];} => OK > > To be extra pedantic, the 'struct {u64 addr; char s[11];} ' > case is also broken on x86, because sizeof (obj) is smaller > on i386, even though the location of the members are > the same. i.e. you can copy_from_user() this, but not > copy_to_user(), which overwrites 4 bytes after the end of > the 20-byte user structure. D'oh! FWIW, it might be worth putting into Documentation/ somewhere; basically, what is and what isn't biarch-neutral. Or arch-neutral, for that matter - it's very close. The only real exception, IIRC, is an extra twist on m68k, where int behaves like x86 long long - its alignment is only half its size, so sizeof(struct {char c; int x;}) is 6, not 8 as everywhere else. Irrelevant for biarch, thankfully (until somebody gets insane enough to implement 64bit coldfire, kernel port for it *and* biarch support for m68k binaries on that thing, that is)... 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=-2.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT 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 60B5DC43218 for ; Thu, 25 Apr 2019 16:43:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C875F206BF for ; Thu, 25 Apr 2019 16:43:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729407AbfDYQms (ORCPT ); Thu, 25 Apr 2019 12:42:48 -0400 Received: from zeniv.linux.org.uk ([195.92.253.2]:48282 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726116AbfDYQmq (ORCPT ); Thu, 25 Apr 2019 12:42:46 -0400 Received: from viro by ZenIV.linux.org.uk with local (Exim 4.92 #3 (Red Hat Linux)) id 1hJhSF-0003K2-7F; Thu, 25 Apr 2019 16:42:35 +0000 Date: Thu, 25 Apr 2019 17:42:35 +0100 From: Al Viro To: Arnd Bergmann Cc: Mauro Carvalho Chehab , Linux FS-devel Mailing List , y2038 Mailman List , Linux Kernel Mailing List , Jason Gunthorpe , Daniel Vetter , Greg Kroah-Hartman , David Sterba , Darren Hart , Jonathan Cameron , Bjorn Andersson , driverdevel , 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-btrfs , ceph-devel , linux-wireless , Networking , Sean Young Subject: Re: [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl Message-ID: <20190425164235.GT2217@ZenIV.linux.org.uk> References: <20190416202013.4034148-1-arnd@arndb.de> <20190416202701.127745-1-arnd@arndb.de> <20190425122153.450fc094@coco.lan> <20190425153534.GS2217@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@vger.kernel.org On Thu, Apr 25, 2019 at 05:55:23PM +0200, Arnd Bergmann wrote: > On Thu, Apr 25, 2019 at 5:35 PM Al Viro wrote: > > > > On Thu, Apr 25, 2019 at 12:21:53PM -0300, Mauro Carvalho Chehab wrote: > > > > > If I understand your patch description well, using compat_ptr_ioctl > > > only works if the driver is not for s390, right? > > > > No; s390 is where "oh, just set ->compat_ioctl same as ->unlocked_ioctl > > and be done with that; compat_ptr() is a no-op anyway" breaks. IOW, > > s390 is the reason for having compat_ptr_ioctl() in the first place; > > that thing works on all biarch architectures, as long as all stuff > > handled by ->ioctl() takes pointer to arch-independent object as > > argument. IOW, > > argument ignored => OK > > any arithmetical type => no go, compat_ptr() would bugger it > > pointer to int => OK > > pointer to string => OK > > pointer to u64 => OK > > pointer to struct {u64 addr; char s[11];} => OK > > To be extra pedantic, the 'struct {u64 addr; char s[11];} ' > case is also broken on x86, because sizeof (obj) is smaller > on i386, even though the location of the members are > the same. i.e. you can copy_from_user() this, but not > copy_to_user(), which overwrites 4 bytes after the end of > the 20-byte user structure. D'oh! FWIW, it might be worth putting into Documentation/ somewhere; basically, what is and what isn't biarch-neutral. Or arch-neutral, for that matter - it's very close. The only real exception, IIRC, is an extra twist on m68k, where int behaves like x86 long long - its alignment is only half its size, so sizeof(struct {char c; int x;}) is 6, not 8 as everywhere else. Irrelevant for biarch, thankfully (until somebody gets insane enough to implement 64bit coldfire, kernel port for it *and* biarch support for m68k binaries on that thing, that is)... 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: [v3,12/26] compat_ioctl: move more drivers to compat_ptr_ioctl From: Al Viro Message-Id: <20190425164235.GT2217@ZenIV.linux.org.uk> Date: Thu, 25 Apr 2019 17:42:35 +0100 To: Arnd Bergmann Cc: Mauro Carvalho Chehab , Linux FS-devel Mailing List , y2038 Mailman List , Linux Kernel Mailing List , Jason Gunthorpe , Daniel Vetter , Greg Kroah-Hartman , David Sterba , Darren Hart , Jonathan Cameron , Bjorn Andersson , driverdevel , 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-btrfs , ceph-devel , linux-wireless , Networking , Sean Young List-ID: T24gVGh1LCBBcHIgMjUsIDIwMTkgYXQgMDU6NTU6MjNQTSArMDIwMCwgQXJuZCBCZXJnbWFubiB3 cm90ZToKPiBPbiBUaHUsIEFwciAyNSwgMjAxOSBhdCA1OjM1IFBNIEFsIFZpcm8gPHZpcm9AemVu aXYubGludXgub3JnLnVrPiB3cm90ZToKPiA+Cj4gPiBPbiBUaHUsIEFwciAyNSwgMjAxOSBhdCAx MjoyMTo1M1BNIC0wMzAwLCBNYXVybyBDYXJ2YWxobyBDaGVoYWIgd3JvdGU6Cj4gPgo+ID4gPiBJ ZiBJIHVuZGVyc3RhbmQgeW91ciBwYXRjaCBkZXNjcmlwdGlvbiB3ZWxsLCB1c2luZyBjb21wYXRf cHRyX2lvY3RsCj4gPiA+IG9ubHkgd29ya3MgaWYgdGhlIGRyaXZlciBpcyBub3QgZm9yIHMzOTAs IHJpZ2h0Pwo+ID4KPiA+IE5vOyBzMzkwIGlzIHdoZXJlICJvaCwganVzdCBzZXQgLT5jb21wYXRf aW9jdGwgc2FtZSBhcyAtPnVubG9ja2VkX2lvY3RsCj4gPiBhbmQgYmUgZG9uZSB3aXRoIHRoYXQ7 IGNvbXBhdF9wdHIoKSBpcyBhIG5vLW9wIGFueXdheSIgYnJlYWtzLiAgSU9XLAo+ID4gczM5MCBp cyB0aGUgcmVhc29uIGZvciBoYXZpbmcgY29tcGF0X3B0cl9pb2N0bCgpIGluIHRoZSBmaXJzdCBw bGFjZTsKPiA+IHRoYXQgdGhpbmcgd29ya3Mgb24gYWxsIGJpYXJjaCBhcmNoaXRlY3R1cmVzLCBh cyBsb25nIGFzIGFsbCBzdHVmZgo+ID4gaGFuZGxlZCBieSAtPmlvY3RsKCkgdGFrZXMgcG9pbnRl ciB0byBhcmNoLWluZGVwZW5kZW50IG9iamVjdCBhcwo+ID4gYXJndW1lbnQuICBJT1csCj4gPiAg ICAgICAgIGFyZ3VtZW50IGlnbm9yZWQgPT4gT0sKPiA+ICAgICAgICAgYW55IGFyaXRobWV0aWNh bCB0eXBlID0+IG5vIGdvLCBjb21wYXRfcHRyKCkgd291bGQgYnVnZ2VyIGl0Cj4gPiAgICAgICAg IHBvaW50ZXIgdG8gaW50ID0+IE9LCj4gPiAgICAgICAgIHBvaW50ZXIgdG8gc3RyaW5nID0+IE9L Cj4gPiAgICAgICAgIHBvaW50ZXIgdG8gdTY0ID0+IE9LCj4gPiAgICAgICAgIHBvaW50ZXIgdG8g c3RydWN0IHt1NjQgYWRkcjsgY2hhciBzWzExXTt9ID0+IE9LCj4gCj4gVG8gYmUgZXh0cmEgcGVk YW50aWMsIHRoZSAnc3RydWN0IHt1NjQgYWRkcjsgY2hhciBzWzExXTt9ICcKPiBjYXNlIGlzIGFs c28gYnJva2VuIG9uIHg4NiwgYmVjYXVzZSBzaXplb2YgKG9iaikgaXMgc21hbGxlcgo+IG9uIGkz ODYsIGV2ZW4gdGhvdWdoIHRoZSBsb2NhdGlvbiBvZiB0aGUgbWVtYmVycyBhcmUKPiB0aGUgc2Ft ZS4gaS5lLiB5b3UgY2FuIGNvcHlfZnJvbV91c2VyKCkgdGhpcywgYnV0IG5vdAo+IGNvcHlfdG9f dXNlcigpLCB3aGljaCBvdmVyd3JpdGVzIDQgYnl0ZXMgYWZ0ZXIgdGhlIGVuZCBvZgo+IHRoZSAy MC1ieXRlIHVzZXIgc3RydWN0dXJlLgoKRCdvaCEgIEZXSVcsIGl0IG1pZ2h0IGJlIHdvcnRoIHB1 dHRpbmcgaW50byBEb2N1bWVudGF0aW9uLyBzb21ld2hlcmU7CmJhc2ljYWxseSwgd2hhdCBpcyBh bmQgd2hhdCBpc24ndCBiaWFyY2gtbmV1dHJhbC4KCk9yIGFyY2gtbmV1dHJhbCwgZm9yIHRoYXQg bWF0dGVyIC0gaXQncyB2ZXJ5IGNsb3NlLiAgVGhlIG9ubHkgcmVhbApleGNlcHRpb24sIElJUkMs IGlzIGFuIGV4dHJhIHR3aXN0IG9uIG02OGssIHdoZXJlIGludCBiZWhhdmVzCmxpa2UgeDg2IGxv bmcgbG9uZyAtIGl0cyBhbGlnbm1lbnQgaXMgb25seSBoYWxmIGl0cyBzaXplLCBzbwpzaXplb2Yo c3RydWN0IHtjaGFyIGM7IGludCB4O30pIGlzIDYsIG5vdCA4IGFzIGV2ZXJ5d2hlcmUKZWxzZS4g IElycmVsZXZhbnQgZm9yIGJpYXJjaCwgdGhhbmtmdWxseSAodW50aWwgc29tZWJvZHkgZ2V0cyBp bnNhbmUKZW5vdWdoIHRvIGltcGxlbWVudCA2NGJpdCBjb2xkZmlyZSwga2VybmVsIHBvcnQgZm9y IGl0ICphbmQqIGJpYXJjaApzdXBwb3J0IGZvciBtNjhrIGJpbmFyaWVzIG9uIHRoYXQgdGhpbmcs IHRoYXQgaXMpLi4uCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Date: Thu, 25 Apr 2019 16:42:35 +0000 Subject: Re: [PATCH v3 12/26] compat_ioctl: move more drivers to compat_ptr_ioctl Message-Id: <20190425164235.GT2217@ZenIV.linux.org.uk> List-Id: References: <20190416202013.4034148-1-arnd@arndb.de> <20190416202701.127745-1-arnd@arndb.de> <20190425122153.450fc094@coco.lan> <20190425153534.GS2217@ZenIV.linux.org.uk> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Arnd Bergmann Cc: Mauro Carvalho Chehab , Linux FS-devel Mailing List , y2038 Mailman List , Linux Kernel Mailing List , Jason Gunthorpe , Daniel Vetter , Greg Kroah-Hartman , David Sterba , Darren Hart , Jonathan Cameron , Bjorn Andersson , driverdevel , qat-linux@intel.com, "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" , Linux Media Mailing List , dri-devel , linaro-mm-sig@lis On Thu, Apr 25, 2019 at 05:55:23PM +0200, Arnd Bergmann wrote: > On Thu, Apr 25, 2019 at 5:35 PM Al Viro wrote: > > > > On Thu, Apr 25, 2019 at 12:21:53PM -0300, Mauro Carvalho Chehab wrote: > > > > > If I understand your patch description well, using compat_ptr_ioctl > > > only works if the driver is not for s390, right? > > > > No; s390 is where "oh, just set ->compat_ioctl same as ->unlocked_ioctl > > and be done with that; compat_ptr() is a no-op anyway" breaks. IOW, > > s390 is the reason for having compat_ptr_ioctl() in the first place; > > that thing works on all biarch architectures, as long as all stuff > > handled by ->ioctl() takes pointer to arch-independent object as > > argument. IOW, > > argument ignored => OK > > any arithmetical type => no go, compat_ptr() would bugger it > > pointer to int => OK > > pointer to string => OK > > pointer to u64 => OK > > pointer to struct {u64 addr; char s[11];} => OK > > To be extra pedantic, the 'struct {u64 addr; char s[11];} ' > case is also broken on x86, because sizeof (obj) is smaller > on i386, even though the location of the members are > the same. i.e. you can copy_from_user() this, but not > copy_to_user(), which overwrites 4 bytes after the end of > the 20-byte user structure. D'oh! FWIW, it might be worth putting into Documentation/ somewhere; basically, what is and what isn't biarch-neutral. Or arch-neutral, for that matter - it's very close. The only real exception, IIRC, is an extra twist on m68k, where int behaves like x86 long long - its alignment is only half its size, so sizeof(struct {char c; int x;}) is 6, not 8 as everywhere else. Irrelevant for biarch, thankfully (until somebody gets insane enough to implement 64bit coldfire, kernel port for it *and* biarch support for m68k binaries on that thing, that is)...