[buytenh@gnu.org: [cry for advice] sparc64 bridging troubles]

[buytenh@gnu.org: [cry for advice] sparc64 bridging troubles]

[Lennert Buytenhek]

In the hope that people on this list will at least read this email until
the end before hitting reply and starting to rant..

Please CC on replies.  Thanks.


----- Forwarded message from Lennert Buytenhek <buytenh@gnu.org> -----

Date: Thu, 10 Jan 2002 11:59:25 -0500
From: Lennert Buytenhek <buytenh@gnu.org>
To: [unnamed sparc64 person]
Cc: narancs@narancs.tii.matav.hu
Subject: [cry for advice] sparc64 bridging troubles
User-Agent: Mutt/1.3.25i

Hi,

After hacking around the fact that the sparc64 PPP ioctl32 conversion
functions mess up the ifreq struct passed to _every_ SIOCDEVPRIVATE device
ioctl ever issued (since the numbers alias, yuck yuck fuck argh yuck), I'm
seeing a truly weird problem.

From net/bridge/br_device.c::br_dev_do_ioctl:

        data = (unsigned long *)rq->ifr_data;
==>    if (copy_from_user(args, data, 4*sizeof(unsigned long)))
                return -EFAULT;

This copy_from_user invocation hangs the box solid, every single time.  The
arguments it's called with are fffff8001395f910, 00000000effff9f8, 32.  I
would think these look OK (and even if they wouldn't I guess they shouldn't
hang the box).

Any good ideas?  The machine in question is (I think) a netra t1 200.  The
box itself is in Hungary, and I can't reboot it by myself, which makes it
inconvenient both for me and the CC'ed person to test things.


thanks,
Lennert

----- End forwarded message -----

Re: [buytenh@gnu.org: [cry for advice] sparc64 bridging troubles]

[David S. Miller]

   From: Lennert Buytenhek <buytenh@gnu.org>
   Date: Thu, 10 Jan 2002 13:49:20 -0500
   
   This copy_from_user invocation hangs the box solid, every single time.  The
   arguments it's called with are fffff8001395f910, 00000000effff9f8, 32.  I
   would think these look OK (and even if they wouldn't I guess they shouldn't
   hang the box).

If set_fs(KERNEL_DS) this will hang the box because that means that
both pointers need to be kernel points.  I bet that is the state
you've left it in when sys_ioctl() is invoked.

The whole gist of ioctl32.c's workings is:

1) copy user struct into kernel copy
2) translate into 64-bit kernel copy
3) orig_fs = get_fs(); set_fs(KERNEL_DS);
4) pass kernel copy to sys_ioctl()
5) set_fs(orig_fs);

The real solution is to move away from SIOCDEVPRIVATE since those
are deprecated anyways, but you appear to understand this already.
:-)

Franks a lot,
David S. Miller
davem@redhat.com

Re: [buytenh@gnu.org: [cry for advice] sparc64 bridging troubles]

[Lennert Buytenhek]

On Fri, Jan 11, 2002 at 12:15:54AM -0800, David S. Miller wrote:

>    This copy_from_user invocation hangs the box solid, every single time.  The
>    arguments it's called with are fffff8001395f910, 00000000effff9f8, 32.  I
>    would think these look OK (and even if they wouldn't I guess they shouldn't
>    hang the box).
> 
> If set_fs(KERNEL_DS) this will hang the box because that means that
> both pointers need to be kernel points.

Whoops, missed that! (I knew it, but didn't realise this could mess
things up)


> The real solution is to move away from SIOCDEVPRIVATE since those
> are deprecated anyways, but you appear to understand this already.
> :-)

Yup.  The attached patch (plus some userspace changes) is what makes
things tick again, but it's not exactly a marvel of beauty.. :(


Thanks a lot!
Lennert


--- linux-2.4.17-br-sparc64/net/bridge/br_device.c.orig	Wed Jan 16 12:36:28 2002
+++ linux-2.4.17-br-sparc64/net/bridge/br_device.c	Wed Jan 16 12:40:22 2002
@@ -23,15 +23,26 @@
 {
 	unsigned long args[4];
 	unsigned long *data;
+	mm_segment_t oldfs = get_fs();
+	int ret;
+	int retval;
 
-	if (cmd != SIOCDEVPRIVATE)
+	if (cmd != SIOCDEVPRIVATE && cmd != SIOCDEVPRIVATE + 3)
 		return -EOPNOTSUPP;
 
 	data = (unsigned long *)rq->ifr_data;
-	if (copy_from_user(args, data, 4*sizeof(unsigned long)))
-		return -EFAULT;
+	set_fs(USER_DS);
+	ret = copy_from_user(args, data, 4*sizeof(unsigned long));
 
-	return br_ioctl(dev->priv, args[0], args[1], args[2], args[3]);
+	retval = -EFAULT;
+	if (ret)
+		goto out;
+
+	retval = br_ioctl(dev->priv, args[0], args[1], args[2], args[3]);
+
+out:
+	set_fs(oldfs);
+	return retval;
 }
 
 static struct net_device_stats *br_dev_get_stats(struct net_device *dev)
--- linux-2.4.17-br-sparc64/arch/sparc64/kernel/ioctl32.c.orig	Wed Jan 16 12:36:05 2002
+++ linux-2.4.17-br-sparc64/arch/sparc64/kernel/ioctl32.c	Wed Jan 16 12:38:07 2002
@@ -472,6 +472,7 @@
 		return -ENODEV;
 
 	strcpy(ifr32.ifr_name, dev->name);
+	dev_put(dev);
 
 	err = copy_to_user((struct ifreq32 *)arg, &ifr32, sizeof(struct ifreq32));
 	return (err ? -EFAULT : 0);
@@ -4605,6 +4606,7 @@
 HANDLE_IOCTL(SIOCGPPPSTATS, dev_ifsioc)
 HANDLE_IOCTL(SIOCGPPPCSTATS, dev_ifsioc)
 HANDLE_IOCTL(SIOCGPPPVER, dev_ifsioc)
+HANDLE_IOCTL(SIOCDEVPRIVATE + 3, dev_ifsioc)
 HANDLE_IOCTL(SIOCGIFTXQLEN, dev_ifsioc)
 HANDLE_IOCTL(SIOCSIFTXQLEN, dev_ifsioc)
 HANDLE_IOCTL(SIOCETHTOOL, ethtool_ioctl)

Re: [buytenh@gnu.org: [cry for advice] sparc64 bridging troubles]

[David S. Miller]

   From: Lennert Buytenhek <buytenh@gnu.org>
   Date: Wed, 16 Jan 2002 12:46:46 -0500
   
   Yup.  The attached patch (plus some userspace changes) is what makes
   things tick again, but it's not exactly a marvel of beauty.. :(

I'm not applying this patch ever to my tree.  What about other private
ioctls overloaded to SIOCDEVPRIVATE+3?

See why none of this SIOCDEVPRIVATE crap can ever work properly?

Re: [buytenh@gnu.org: [cry for advice] sparc64 bridging troubles]

[Lennert Buytenhek]

On Wed, Jan 16, 2002 at 09:54:51AM -0800, David S. Miller wrote:

>    From: Lennert Buytenhek <buytenh@gnu.org>
>    Date: Wed, 16 Jan 2002 12:46:46 -0500
>    
>    Yup.  The attached patch (plus some userspace changes) is what makes
>    things tick again, but it's not exactly a marvel of beauty.. :(
> 
> I'm not applying this patch ever to my tree.  What about other private
> ioctls overloaded to SIOCDEVPRIVATE+3?

I'm not suggesting you should..


> See why none of this SIOCDEVPRIVATE crap can ever work properly?

My point exactly.. glad we understand eachother :-)


cheers,
Lennert