Bug#288180: A possible workaround for nis locking up the machine on sparc64
Hello,
After some investigation I have found that this bug is caused by the
blocking of copy_in_user function in the path of SIOCGIFCONF ioctl
handler (dev_ifconf in fs/compat_ioctl.c). As solving this is a little bit
over my head, I have forwarded information to the sparclinux@vger mailing
list [0], hopefully someone there will be able to offer some advice. A
possible way to solve this problem is to revert the patch which introduced
the offending code [1]. The patch reverting this change is attached and
also available at [2]. I have built a kernel with it applied and the
system no longer lock-ups. The fixed kernel is available at [3], please
test it to confirm that it indeed solves it.
[0] http://marc.theaimsgroup.com/?t=110780063700005&r=1&w=2
[1] http://linux.bkbits.net:8080/linux-2.6/diffs/fs/compat_ioctl.c@1.25.1.9?nav=index.html|src/|src/fs|hist/fs/compat_ioctl.c
[2] http://www.wooyd.org/debian/patches/2.6.8-dev_ifconf.patch
[3] http://www.wooyd.org/debian/bug288180
Best regards,
Jurij Smakov jurij@wooyd.org
Key: http://www.wooyd.org/pgpkey/ KeyID: C99E03CC
--- a/fs/compat_ioctl.c 2004-08-14 01:38:04.000000000 -0400
+++ b/fs/compat_ioctl.c 2005-02-07 12:33:18.000000000 -0500
@@ -487,71 +487,75 @@
{
struct ifconf32 ifc32;
struct ifconf ifc;
- struct ifconf __user *uifc;
- struct ifreq32 __user *ifr32;
- struct ifreq __user *ifr;
+ struct ifreq32 *ifr32;
+ struct ifreq *ifr;
+ mm_segment_t old_fs;
unsigned int i, j;
int err;
if (copy_from_user(&ifc32, compat_ptr(arg), sizeof(struct ifconf32)))
return -EFAULT;
- if (ifc32.ifcbuf == 0) {
+ if(ifc32.ifcbuf == 0) {
ifc32.ifc_len = 0;
ifc.ifc_len = 0;
- ifc.ifc_req = NULL;
- uifc = compat_alloc_user_space(sizeof(struct ifconf));
+ ifc.ifc_buf = NULL;
} else {
- size_t len =((ifc32.ifc_len / sizeof (struct ifreq32)) + 1) *
+ ifc.ifc_len = ((ifc32.ifc_len / sizeof (struct ifreq32)) + 1) *
sizeof (struct ifreq);
- uifc = compat_alloc_user_space(sizeof(struct ifconf) + len);
- ifc.ifc_len = len;
- ifr = ifc.ifc_req = (void __user *)(uifc + 1);
- ifr32 = compat_ptr(ifc32.ifcbuf);
- for (i = 0; i < ifc32.ifc_len; i += sizeof (struct ifreq32)) {
- if (copy_in_user(ifr, ifr32, sizeof(struct ifreq32)))
- return -EFAULT;
- ifr++;
- ifr32++;
- }
+ /* should the size be limited? -arnd */
+ ifc.ifc_buf = kmalloc (ifc.ifc_len, GFP_KERNEL);
+ if (!ifc.ifc_buf)
+ return -ENOMEM;
}
- if (copy_to_user(uifc, &ifc, sizeof(struct ifconf)))
- return -EFAULT;
-
- err = sys_ioctl (fd, SIOCGIFCONF, (unsigned long)uifc);
- if (err)
- return err;
-
- if (copy_from_user(&ifc, uifc, sizeof(struct ifconf)))
- return -EFAULT;
-
ifr = ifc.ifc_req;
ifr32 = compat_ptr(ifc32.ifcbuf);
- for (i = 0, j = 0; i < ifc32.ifc_len && j < ifc.ifc_len;
- i += sizeof (struct ifreq32), j += sizeof (struct ifreq)) {
- if (copy_in_user(ifr32, ifr, sizeof (struct ifreq32)))
+ for (i = 0; i < ifc32.ifc_len; i += sizeof (struct ifreq32)) {
+ if (copy_from_user(ifr, ifr32, sizeof (struct ifreq32))) {
+ kfree (ifc.ifc_buf);
return -EFAULT;
- ifr32++;
+ }
ifr++;
+ ifr32++;
}
-
- if (ifc32.ifcbuf == 0) {
- /* Translate from 64-bit structure multiple to
- * a 32-bit one.
- */
- i = ifc.ifc_len;
- i = ((i / sizeof(struct ifreq)) * sizeof(struct ifreq32));
- ifc32.ifc_len = i;
- } else {
- if (i <= ifc32.ifc_len)
- ifc32.ifc_len = i;
- else
- ifc32.ifc_len = i - sizeof (struct ifreq32);
+ old_fs = get_fs(); set_fs (KERNEL_DS);
+ err = sys_ioctl (fd, SIOCGIFCONF, (unsigned long)&ifc);
+ set_fs (old_fs);
+ if (!err) {
+ ifr = ifc.ifc_req;
+ ifr32 = compat_ptr(ifc32.ifcbuf);
+ for (i = 0, j = 0; i < ifc32.ifc_len && j < ifc.ifc_len;
+ i += sizeof (struct ifreq32), j += sizeof (struct ifreq)) {
+ int k = copy_to_user(ifr32, ifr, sizeof (struct ifreq32));
+ ifr32++;
+ ifr++;
+ if (k) {
+ err = -EFAULT;
+ break;
+ }
+
+ }
+ if (!err) {
+ if (ifc32.ifcbuf == 0) {
+ /* Translate from 64-bit structure multiple to
+ * a 32-bit one.
+ */
+ i = ifc.ifc_len;
+ i = ((i / sizeof(struct ifreq)) * sizeof(struct ifreq32));
+ ifc32.ifc_len = i;
+ } else {
+ if (i <= ifc32.ifc_len)
+ ifc32.ifc_len = i;
+ else
+ ifc32.ifc_len = i - sizeof (struct ifreq32);
+ }
+ if (copy_to_user(compat_ptr(arg), &ifc32, sizeof(struct ifconf32)))
+ err = -EFAULT;
+ }
}
- if (copy_to_user(compat_ptr(arg), &ifc32, sizeof(struct ifconf32)))
- return -EFAULT;
-
- return 0;
+ if(ifc.ifc_buf != NULL)
+ kfree (ifc.ifc_buf);
+ return err;
}
static int ethtool_ioctl(unsigned int fd, unsigned int cmd, unsigned long arg)
Reply to: