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
