[PATCH net 0/4] various compat ioctl fixes

[PATCH net 0/4] various compat ioctl fixes

[Johannes Berg]

Back a long time ago, I already fixed a few of these by passing
the size of the struct ifreq to do_sock_ioctl(). However, Robert
found more cases, and now it won't be as simple because we'd have
to pass that down all the way to e.g. bond_do_ioctl() which isn't
really feasible.

Therefore, restore the old code.

While looking at why SIOCGIFNAME was broken, I realized that Al
had removed that case - which had been handled in an explicit
separate function - as well, and looking through his work at the
time I saw that bond ioctls were also affected by the erroneous
removal.

I've restored SIOCGIFNAME and bond ioctls by going through the
(now renamed) dev_ifsioc() instead of reintroducing their own
helper functions, which I hope is correct but have only tested
with SIOCGIFNAME.

johannes

[PATCH net 1/4] Revert "socket: fix struct ifreq size in compat ioctl"

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

This reverts commit 1cebf8f143c2 ("socket: fix struct ifreq
size in compat ioctl"), it's a bugfix for another commit that
I'll revert next.

This is not a 'perfect' revert, I'm keeping some coding style
intact rather than revert to the state with indentation errors.

Cc: stable@vger.kernel.org
Fixes: 1cebf8f143c2 ("socket: fix struct ifreq size in compat ioctl")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/socket.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index e89884e2197b..63b53af7379b 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -941,8 +941,7 @@ void dlci_ioctl_set(int (*hook) (unsigned int, void __user *))
 EXPORT_SYMBOL(dlci_ioctl_set);
 
 static long sock_do_ioctl(struct net *net, struct socket *sock,
-			  unsigned int cmd, unsigned long arg,
-			  unsigned int ifreq_size)
+			  unsigned int cmd, unsigned long arg)
 {
 	int err;
 	void __user *argp = (void __user *)arg;
@@ -968,11 +967,11 @@ static long sock_do_ioctl(struct net *net, struct socket *sock,
 	} else {
 		struct ifreq ifr;
 		bool need_copyout;
-		if (copy_from_user(&ifr, argp, ifreq_size))
+		if (copy_from_user(&ifr, argp, sizeof(struct ifreq)))
 			return -EFAULT;
 		err = dev_ioctl(net, cmd, &ifr, &need_copyout);
 		if (!err && need_copyout)
-			if (copy_to_user(argp, &ifr, ifreq_size))
+			if (copy_to_user(argp, &ifr, sizeof(struct ifreq)))
 				return -EFAULT;
 	}
 	return err;
@@ -1071,8 +1070,7 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 			err = open_related_ns(&net->ns, get_net_ns);
 			break;
 		default:
-			err = sock_do_ioctl(net, sock, cmd, arg,
-					    sizeof(struct ifreq));
+			err = sock_do_ioctl(net, sock, cmd, arg);
 			break;
 		}
 	return err;
@@ -2780,8 +2778,7 @@ static int do_siocgstamp(struct net *net, struct socket *sock,
 	int err;
 
 	set_fs(KERNEL_DS);
-	err = sock_do_ioctl(net, sock, cmd, (unsigned long)&ktv,
-			    sizeof(struct compat_ifreq));
+	err = sock_do_ioctl(net, sock, cmd, (unsigned long)&ktv);
 	set_fs(old_fs);
 	if (!err)
 		err = compat_put_timeval(&ktv, up);
@@ -2797,8 +2794,7 @@ static int do_siocgstampns(struct net *net, struct socket *sock,
 	int err;
 
 	set_fs(KERNEL_DS);
-	err = sock_do_ioctl(net, sock, cmd, (unsigned long)&kts,
-			    sizeof(struct compat_ifreq));
+	err = sock_do_ioctl(net, sock, cmd, (unsigned long)&kts);
 	set_fs(old_fs);
 	if (!err)
 		err = compat_put_timespec(&kts, up);
@@ -3109,8 +3105,7 @@ static int routing_ioctl(struct net *net, struct socket *sock,
 	}
 
 	set_fs(KERNEL_DS);
-	ret = sock_do_ioctl(net, sock, cmd, (unsigned long) r,
-			    sizeof(struct compat_ifreq));
+	ret = sock_do_ioctl(net, sock, cmd, (unsigned long) r);
 	set_fs(old_fs);
 
 out:
@@ -3223,8 +3218,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
 	case SIOCBONDSETHWADDR:
 	case SIOCBONDCHANGEACTIVE:
 	case SIOCGIFNAME:
-		return sock_do_ioctl(net, sock, cmd, arg,
-				     sizeof(struct compat_ifreq));
+		return sock_do_ioctl(net, sock, cmd, arg);
 	}
 
 	return -ENOIOCTLCMD;
-- 
2.17.2

[PATCH net 2/4] Revert "kill dev_ifsioc()"

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

This reverts commit bf4405737f9f ("kill dev_ifsioc()").

This wasn't really unused as implied by the original commit,
it still handles the copy to/from user differently, and the
commit thus caused issues such as
  https://bugzilla.kernel.org/show_bug.cgi?id=199469
and
  https://bugzilla.kernel.org/show_bug.cgi?id=202273

However, deviating from a strict revert, rename dev_ifsioc()
to compat_ifreq_ioctl() to be clearer as to its purpose and
add a comment.

Cc: stable@vger.kernel.org
Fixes: bf4405737f9f ("kill dev_ifsioc()")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/socket.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/net/socket.c b/net/socket.c
index 63b53af7379b..fbf80f9fb057 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2990,6 +2990,53 @@ static int compat_ifr_data_ioctl(struct net *net, unsigned int cmd,
 	return dev_ioctl(net, cmd, &ifreq, NULL);
 }
 
+static int compat_ifreq_ioctl(struct net *net, struct socket *sock,
+			      unsigned int cmd,
+			      struct compat_ifreq __user *uifr32)
+{
+	struct ifreq __user *uifr;
+	int err;
+
+	/* Handle the fact that while struct ifreq has the same *layout* on
+	 * 32/64 for everything but ifreq::ifru_ifmap and ifreq::ifru_data,
+	 * which are handled elsewhere, it still has different *size* due to
+	 * ifreq::ifru_ifmap (which is 16 bytes on 32 bit, 24 bytes on 64-bit,
+	 * resulting in struct ifreq being 32 and 40 bytes respectively).
+	 * As a result, if the struct happens to be at the end of a page and
+	 * the next page isn't readable/writable, we get a fault. To prevent
+	 * that, copy back and forth to the full size.
+	 */
+
+	uifr = compat_alloc_user_space(sizeof(*uifr));
+	if (copy_in_user(uifr, uifr32, sizeof(*uifr32)))
+		return -EFAULT;
+
+	err = sock_do_ioctl(net, sock, cmd, (unsigned long)uifr);
+
+	if (!err) {
+		switch (cmd) {
+		case SIOCGIFFLAGS:
+		case SIOCGIFMETRIC:
+		case SIOCGIFMTU:
+		case SIOCGIFMEM:
+		case SIOCGIFHWADDR:
+		case SIOCGIFINDEX:
+		case SIOCGIFADDR:
+		case SIOCGIFBRDADDR:
+		case SIOCGIFDSTADDR:
+		case SIOCGIFNETMASK:
+		case SIOCGIFPFLAGS:
+		case SIOCGIFTXQLEN:
+		case SIOCGMIIPHY:
+		case SIOCGMIIREG:
+			if (copy_in_user(uifr32, uifr, sizeof(*uifr32)))
+				err = -EFAULT;
+			break;
+		}
+	}
+	return err;
+}
+
 static int compat_sioc_ifmap(struct net *net, unsigned int cmd,
 			struct compat_ifreq __user *uifr32)
 {
@@ -3209,6 +3256,8 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
 	case SIOCGMIIPHY:
 	case SIOCGMIIREG:
 	case SIOCSMIIREG:
+		return compat_ifreq_ioctl(net, sock, cmd, argp);
+
 	case SIOCSARP:
 	case SIOCGARP:
 	case SIOCDARP:
-- 
2.17.2

Re: [PATCH net 2/4] Revert "kill dev_ifsioc()"

[Al Viro]

On Fri, Jan 25, 2019 at 10:43:18PM +0100, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> This reverts commit bf4405737f9f ("kill dev_ifsioc()").
> 
> This wasn't really unused as implied by the original commit,
> it still handles the copy to/from user differently, and the
> commit thus caused issues such as
>   https://bugzilla.kernel.org/show_bug.cgi?id=199469
> and
>   https://bugzilla.kernel.org/show_bug.cgi?id=202273
> 
> However, deviating from a strict revert, rename dev_ifsioc()
> to compat_ifreq_ioctl() to be clearer as to its purpose and
> add a comment.

I disagree with solution.  Look at what's happening here:

> +	uifr = compat_alloc_user_space(sizeof(*uifr));
> +	if (copy_in_user(uifr, uifr32, sizeof(*uifr32)))
> +		return -EFAULT;

an enlarged copy is made.

> +	err = sock_do_ioctl(net, sock, cmd, (unsigned long)uifr);

... which hits this:
                if (copy_from_user(&ifr, argp, ifreq_size))
                        return -EFAULT;
                err = dev_ioctl(net, cmd, &ifr, &need_copyout);
                if (!err && need_copyout)
                        if (copy_to_user(argp, &ifr, ifreq_size))
                                return -EFAULT;
copying that copy into the kernel space, passing _that_ to dev_ioctl(),
then, if dev_ioctl() says that this one needs a copyout, we take the
modified kernel copy and copy it to (enlarged) userland one.

Then
> +
> +	if (!err) {
> +		switch (cmd) {
> +		case SIOCGIFFLAGS:
> +		case SIOCGIFMETRIC:
> +		case SIOCGIFMTU:
> +		case SIOCGIFMEM:
> +		case SIOCGIFHWADDR:
> +		case SIOCGIFINDEX:
> +		case SIOCGIFADDR:
> +		case SIOCGIFBRDADDR:
> +		case SIOCGIFDSTADDR:
> +		case SIOCGIFNETMASK:
> +		case SIOCGIFPFLAGS:
> +		case SIOCGIFTXQLEN:
> +		case SIOCGMIIPHY:
> +		case SIOCGMIIREG:
> +			if (copy_in_user(uifr32, uifr, sizeof(*uifr32)))
> +				err = -EFAULT;

We duplicate the "needs copyout" logics here, and copy from the enlarged
userland instance to the original.

It's much too convoluted, and I really wonder if ifreq_size argument is
a good idea - AFAICS, it's only introduced to be able to (ab)use
sock_do_ioctl() here.

Re: [PATCH net 2/4] Revert "kill dev_ifsioc()"

[Johannes Berg]

On Sat, 2019-01-26 at 17:29 +0000, Al Viro wrote:
> 
> I disagree with solution.  Look at what's happening here:
> 
> > +	uifr = compat_alloc_user_space(sizeof(*uifr));
> > +	if (copy_in_user(uifr, uifr32, sizeof(*uifr32)))
> > +		return -EFAULT;
> 
> an enlarged copy is made.

Yes, that's the point :-)

> > +	err = sock_do_ioctl(net, sock, cmd, (unsigned long)uifr);
> 
> ... which hits this:
>                 if (copy_from_user(&ifr, argp, ifreq_size))
>                         return -EFAULT;
>                 err = dev_ioctl(net, cmd, &ifr, &need_copyout);
>                 if (!err && need_copyout)
>                         if (copy_to_user(argp, &ifr, ifreq_size))
>                                 return -EFAULT;
> copying that copy into the kernel space, passing _that_ to dev_ioctl(),
> then, if dev_ioctl() says that this one needs a copyout, we take the
> modified kernel copy and copy it to (enlarged) userland one.

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

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;

and *not* get to the code you quote, i.e. *not* be a dev_ioctl().

> It's much too convoluted, and I really wonder if ifreq_size argument is
> a good idea - AFAICS, it's only introduced to be able to (ab)use
> sock_do_ioctl() here.

That was the other (my) patch I reverted, it was done so we could do the
copy in/out here safely for dev_ioctl(), but it clearly doesn't work for
the "sock->ops->ioctl()" case.

If you have any better suggestions, I don't mind, but I don't see
anything short of passing compat knowledge down to sock->ops->ioctl()
which seems like an even worse idea.

johannes

Re: [PATCH net 2/4] Revert "kill dev_ifsioc()"

[Johannes Berg]

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.

johannes

Re: [PATCH net 2/4] Revert "kill dev_ifsioc()"

[Johannes Berg]

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

[PATCH net 3/4] net: socket: fix SIOCGIFNAME in compat

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

As reported by Robert O'Callahan in
https://bugzilla.kernel.org/show_bug.cgi?id=202273
reverting the previous changes in this area broke
the SIOCGIFNAME ioctl in compat again (I'd previously
fixed it after his previous report of breakage in
https://bugzilla.kernel.org/show_bug.cgi?id=199469).

This is obviously because I fixed SIOCGIFNAME more or
less by accident.

Fix it explicitly now by making it pass through the
restored compat translation code.

Cc: stable@vger.kernel.org
Fixes: 4cf808e7ac32 ("kill dev_ifname32()")
Reported-by: Robert O'Callahan <robert@ocallahan.org>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/socket.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/socket.c b/net/socket.c
index fbf80f9fb057..473ac8d7c54e 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3029,6 +3029,7 @@ static int compat_ifreq_ioctl(struct net *net, struct socket *sock,
 		case SIOCGIFTXQLEN:
 		case SIOCGMIIPHY:
 		case SIOCGMIIREG:
+		case SIOCGIFNAME:
 			if (copy_in_user(uifr32, uifr, sizeof(*uifr32)))
 				err = -EFAULT;
 			break;
@@ -3252,6 +3253,7 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
 	case SIOCSIFTXQLEN:
 	case SIOCBRADDIF:
 	case SIOCBRDELIF:
+	case SIOCGIFNAME:
 	case SIOCSIFNAME:
 	case SIOCGMIIPHY:
 	case SIOCGMIIREG:
@@ -3266,7 +3268,6 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
 	case SIOCBONDRELEASE:
 	case SIOCBONDSETHWADDR:
 	case SIOCBONDCHANGEACTIVE:
-	case SIOCGIFNAME:
 		return sock_do_ioctl(net, sock, cmd, arg);
 	}
 
-- 
2.17.2

[PATCH net 4/4] net: socket: make bond ioctls go through compat_ifreq_ioctl()

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Same story as before, these use struct ifreq and thus need
to be read with the shorter version to not cause faults.

Cc: stable@vger.kernel.org
Fixes: f92d4fc95341 ("kill bond_ioctl()")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/socket.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 473ac8d7c54e..d80d87a395ea 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -3258,16 +3258,16 @@ static int compat_sock_ioctl_trans(struct file *file, struct socket *sock,
 	case SIOCGMIIPHY:
 	case SIOCGMIIREG:
 	case SIOCSMIIREG:
+	case SIOCBONDENSLAVE:
+	case SIOCBONDRELEASE:
+	case SIOCBONDSETHWADDR:
+	case SIOCBONDCHANGEACTIVE:
 		return compat_ifreq_ioctl(net, sock, cmd, argp);
 
 	case SIOCSARP:
 	case SIOCGARP:
 	case SIOCDARP:
 	case SIOCATMARK:
-	case SIOCBONDENSLAVE:
-	case SIOCBONDRELEASE:
-	case SIOCBONDSETHWADDR:
-	case SIOCBONDCHANGEACTIVE:
 		return sock_do_ioctl(net, sock, cmd, arg);
 	}
 
-- 
2.17.2

Re: [PATCH net 0/4] various compat ioctl fixes

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Fri, 25 Jan 2019 22:43:16 +0100

> Back a long time ago, I already fixed a few of these by passing
> the size of the struct ifreq to do_sock_ioctl(). However, Robert
> found more cases, and now it won't be as simple because we'd have
> to pass that down all the way to e.g. bond_do_ioctl() which isn't
> really feasible.
> 
> Therefore, restore the old code.
> 
> While looking at why SIOCGIFNAME was broken, I realized that Al
> had removed that case - which had been handled in an explicit
> separate function - as well, and looking through his work at the
> time I saw that bond ioctls were also affected by the erroneous
> removal.
> 
> I've restored SIOCGIFNAME and bond ioctls by going through the
> (now renamed) dev_ifsioc() instead of reintroducing their own
> helper functions, which I hope is correct but have only tested
> with SIOCGIFNAME.

I see some back and forth between you and Al, where do we stand at
this point?

From what I can see this looks like probably the simplest way to
fix this in net and -stable currently.

Please let me know.

Thanks.

Re: [PATCH net 0/4] various compat ioctl fixes

[Johannes Berg]

On Mon, 2019-01-28 at 11:22 -0800, David Miller wrote:

> I see some back and forth between you and Al, where do we stand at
> this point?

I don't really know. I think neither of us _likes_ this code, in
particular the whole copy_in_user() thing is quite a mess. The
copy_in_user() also means that decnet (and similar things, if they
exist, I didn't see any but didn't audit all protocols carefully) have
no way of working in compat - it's not even clear to me if that'd return
-EFAULT or just do something really stupid, and maybe even dangerous?

(Dangerous because at least on x86, compat_alloc_user_space() uses stack
space, and if we alloc 40 bytes but decnet writes up to 42 (?) then we
could overwrite some stack by that? Maybe the 16-byte alignment in
compat_alloc_user_space() saves us, but it's all very fragile. Even with
the previous patch fixed, decnet's idea of "struct ifreq" is bigger than
"struct ifreq" actually is because sockaddr_dn is bigger, if I'm
counting it right then that's 42 in total)

At the same time, fixing all this _completely_ is not very realistic, it
would require passing the ifreq size through to lots of places and
making the user copy there take the size rather than sizeof(ifreq),
obviously the very least to the method decnet uses, i.e. sock->ioctl() I
think, but clearly that affects every other protocol too.
This was what my previous patch had done partially for the directly
handled ioctls (the revert of which is the first patch in this series).

> From what I can see this looks like probably the simplest way to
> fix this in net and -stable currently.

I tend to agree, at least to fix the regression.

We can still deliberate separately if we want to fix decnet for compat
or if nobody cares now. But perhaps better decnet broken (quite
obviously and detectably) like it basically always was, than IP broken
(subtly, if your struct ends up landing at the end of a page).

Al, care to speak up about this here?

johannes

Re: [PATCH net 0/4] various compat ioctl fixes

[David Miller]

From: Johannes Berg <johannes@sipsolutions.net>
Date: Mon, 28 Jan 2019 22:32:30 +0100

> Al, care to speak up about this here?

I'll give Al one day to respond.

I'll apply this series if he agrees or fails to give feedback.

Re: [PATCH net 0/4] various compat ioctl fixes

[Al Viro]

On Mon, Jan 28, 2019 at 10:32:30PM +0100, Johannes Berg wrote:

> At the same time, fixing all this _completely_ is not very realistic, it
> would require passing the ifreq size through to lots of places and
> making the user copy there take the size rather than sizeof(ifreq),
> obviously the very least to the method decnet uses, i.e. sock->ioctl() I
> think, but clearly that affects every other protocol too.
> This was what my previous patch had done partially for the directly
> handled ioctls (the revert of which is the first patch in this series).
> 
> > From what I can see this looks like probably the simplest way to
> > fix this in net and -stable currently.
> 
> I tend to agree, at least to fix the regression.
> 
> We can still deliberate separately if we want to fix decnet for compat
> or if nobody cares now. But perhaps better decnet broken (quite
> obviously and detectably) like it basically always was, than IP broken
> (subtly, if your struct ends up landing at the end of a page).
> 
> Al, care to speak up about this here?

Umm...  Short-term I don't see anything better; long-term I would really
like to see compat_alloc_user_space()/copy_in_user() crap gone and
copyin-copyout for anything more or less generic lifted up as far as
cleanly possible, but let's not mix it with regression fixing.

So for the lack of better short-term solutions,
Acked-by: Al Viro <viro@zeniv.linux.org.uk>
on the series.

Re: [PATCH net 0/4] various compat ioctl fixes

[David Miller]

From: Al Viro <viro@zeniv.linux.org.uk>
Date: Wed, 30 Jan 2019 15:40:09 +0000

> On Mon, Jan 28, 2019 at 10:32:30PM +0100, Johannes Berg wrote:
> 
>> At the same time, fixing all this _completely_ is not very realistic, it
>> would require passing the ifreq size through to lots of places and
>> making the user copy there take the size rather than sizeof(ifreq),
>> obviously the very least to the method decnet uses, i.e. sock->ioctl() I
>> think, but clearly that affects every other protocol too.
>> This was what my previous patch had done partially for the directly
>> handled ioctls (the revert of which is the first patch in this series).
>> 
>> > From what I can see this looks like probably the simplest way to
>> > fix this in net and -stable currently.
>> 
>> I tend to agree, at least to fix the regression.
>> 
>> We can still deliberate separately if we want to fix decnet for compat
>> or if nobody cares now. But perhaps better decnet broken (quite
>> obviously and detectably) like it basically always was, than IP broken
>> (subtly, if your struct ends up landing at the end of a page).
>> 
>> Al, care to speak up about this here?
> 
> Umm...  Short-term I don't see anything better; long-term I would really
> like to see compat_alloc_user_space()/copy_in_user() crap gone and
> copyin-copyout for anything more or less generic lifted up as far as
> cleanly possible, but let's not mix it with regression fixing.

It's a real shame, I thought it was a super clever solution to that
problem space at the time we added it.

> So for the lack of better short-term solutions,
> Acked-by: Al Viro <viro@zeniv.linux.org.uk>
> on the series.

Ok, series applied, thanks everyone.

I'll queue this up for -stable too.