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=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=ham 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 9CFE0C282C7 for ; Sat, 26 Jan 2019 18:53:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6EC872184B for ; Sat, 26 Jan 2019 18:53:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726160AbfAZSx0 (ORCPT ); Sat, 26 Jan 2019 13:53:26 -0500 Received: from s3.sipsolutions.net ([144.76.43.62]:43452 "EHLO sipsolutions.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726089AbfAZSx0 (ORCPT ); Sat, 26 Jan 2019 13:53:26 -0500 Received: by sipsolutions.net with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92-RC4) (envelope-from ) id 1gnT51-0006vP-F2; Sat, 26 Jan 2019 19:53:23 +0100 Message-ID: Subject: Re: [PATCH net 2/4] Revert "kill dev_ifsioc()" From: Johannes Berg To: Al Viro Cc: netdev@vger.kernel.org, Robert O'Callahan Date: Sat, 26 Jan 2019 19:53:21 +0100 In-Reply-To: <5812aacf62c99fcf98da7d4fc7eaa0ef9cd6afa1.camel@sipsolutions.net> References: <20190125214320.17685-1-johannes@sipsolutions.net> <20190125214320.17685-3-johannes@sipsolutions.net> <20190126172926.GG2217@ZenIV.linux.org.uk> <8e8423d668eb4e38809618c89afbdfa1a1e772af.camel@sipsolutions.net> <5812aacf62c99fcf98da7d4fc7eaa0ef9cd6afa1.camel@sipsolutions.net> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 (3.28.5-2.fc28) Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Sat, 2019-01-26 at 18:49 +0100, Johannes Berg wrote: > On Sat, 2019-01-26 at 18:45 +0100, Johannes Berg wrote: > > > > Yes and no. It *sometimes* (actually rarely, since we don't really have > > dev_ioctls that much, afaict) hits this, but it could also just hit > > Actually, no, I'm wrong. We do mostly hit dev_ioctl(), since that's the > common case for things like SIOCGIFNAME. > > However, e.g. for SIOCGIFADDR we do go into > > > static long sock_do_ioctl(struct net *net, struct socket *sock, > > unsigned int cmd, unsigned long arg) > > { > > [...] > > err = sock->ops->ioctl(sock, cmd, arg); > > [...] > > if (err != -ENOIOCTLCMD) > > return err; > > This, and like I said, plumbing the whole compat stuff through to the > sock->ops->ioctl() there doesn't seem like a great idea. So - discussing this further on IRC I thought we could get away with making struct ifreq just not include the members that are too big (ifr_map and ifr_settings), but that's also a non-starter because we need to copy. Al also points out that all of these reverts break decnet, because that does some really messy things in dn_dev_ioctl(). Turns out though that on 64-bit decnet is already broken anyway, because DN_IFREQ_SIZE is actually wrong. It should presumably be equivalent to something like struct ifreq_dn { char ifrn_name[IFNAMSIZ]; struct sockaddr_dn ifru_dnaddr; }; but in fact *isn't* because #define DN_IFREQ_SIZE (sizeof(struct ifreq) - sizeof(struct sockaddr) + sizeof(struct sockaddr_dn)) wouldn't be sizeof(struct ifreq_dn), because in this expression "sizeof(struct ifreq) - sizeof(struct sockaddr)" isn't the same as "offsetof(struct ifreq, ifr_ifru)" which would be the right thing. So with these patches we go back to the original state before Al's patches, but that does mean: * decnet doesn't work right on 64-bit (kernel & userland) because it will attempt to copy a bigger buffer than the user would presumably be expected to provide a struct ifreq_dn like I defined above to SIOCGIFADDR, and if this is at the end of a page boundary it faults * 32-bit userland on 64-bit kernel is completely broken here for decnet ioctls because we copy too little data (struct ifreq, while struct ifreq_dn is bigger) The first of these isn't that hard to fix, just fix the DN_IFREQ_SIZE. The second one I don't see a good way to fix at all. johannes