[patch 0/5] various user space access fixes

[patch 0/5] various user space access fixes

[Heiko Carstens]

Replacing the get/put_user macros with some __must_check dummy functions
quite a few places where user space accesses aren't handled properly.
I fixed most of these (that come up with a warning on standard s390 build).

But for the ones below I haven't done anything:

include/asm/uaccess.h: In function `schedule_tail':
kernel/sched.c:1811: warning: ignoring return value of `put_user', declared with attribute warn_unused_result

include/asm/uaccess.h: In function `mm_release':
kernel/fork.c:459: warning: ignoring return value of `put_user', declared with attribute warn_unused_result

include/asm/uaccess.h: In function `sys_getcpu':
kernel/sys.c:2189: warning: ignoring return value of `get_user', declared with attribute warn_unused_result
kernel/sys.c:2190: warning: ignoring return value of `get_user', declared with attribute warn_unused_result
kernel/sys.c:2193: warning: ignoring return value of `put_user', declared with attribute warn_unused_result
kernel/sys.c:2194: warning: ignoring return value of `put_user', declared with attribute warn_unused_result

Not sure if these three need to be fixed at all... Even though sys_getcpu
looks like it needs to be fixed, but how?


include/asm/uaccess.h: In function `set_termios':
drivers/char/tty_ioctl.c:207: warning: ignoring return value of `get_user', declared with attribute warn_unused_result
drivers/char/tty_ioctl.c:207: warning: ignoring return value of `get_user', declared with attribute warn_unused_result
drivers/char/tty_ioctl.c:207: warning: ignoring return value of `get_user', declared with attribute warn_unused_result
drivers/char/tty_ioctl.c:207: warning: ignoring return value of `get_user', declared with attribute warn_unused_result

include/asm/uaccess.h: In function `n_tty_ioctl':
drivers/char/tty_ioctl.c:236: warning: ignoring return value of `put_user', declared with attribute warn_unused_result
drivers/char/tty_ioctl.c:236: warning: ignoring return value of `put_user', declared with attribute warn_unused_result
drivers/char/tty_ioctl.c:236: warning: ignoring return value of `put_user', declared with attribute warn_unused_result
drivers/char/tty_ioctl.c:236: warning: ignoring return value of `put_user', declared with attribute warn_unused_result
drivers/char/tty_ioctl.c:236: warning: ignoring return value of `put_user', declared with attribute warn_unused_result

These two are because of the nice user_termio_to_kernel_termios and
kernel_termios_to_user_termio macros in asm/termios.h...
Might have a look at these two later.

[patch 1/5] binfmt: fix uaccess handling

[Heiko Carstens]

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 fs/binfmt_elf.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

Index: linux-2.6/fs/binfmt_elf.c
===================================================================
--- linux-2.6.orig/fs/binfmt_elf.c	2006-10-26 14:40:58.000000000 +0200
+++ linux-2.6/fs/binfmt_elf.c	2006-10-26 14:41:59.000000000 +0200
@@ -243,8 +243,9 @@
 	if (interp_aout) {
 		argv = sp + 2;
 		envp = argv + argc + 1;
-		__put_user((elf_addr_t)(unsigned long)argv, sp++);
-		__put_user((elf_addr_t)(unsigned long)envp, sp++);
+		if (__put_user((elf_addr_t)(unsigned long)argv, sp++) ||
+		    __put_user((elf_addr_t)(unsigned long)envp, sp++))
+			return -EFAULT;
 	} else {
 		argv = sp;
 		envp = argv + argc + 1;
@@ -254,7 +255,8 @@
 	p = current->mm->arg_end = current->mm->arg_start;
 	while (argc-- > 0) {
 		size_t len;
-		__put_user((elf_addr_t)p, argv++);
+		if (__put_user((elf_addr_t)p, argv++))
+			return -EFAULT;
 		len = strnlen_user((void __user *)p, PAGE_SIZE*MAX_ARG_PAGES);
 		if (!len || len > PAGE_SIZE*MAX_ARG_PAGES)
 			return 0;
@@ -265,7 +267,8 @@
 	current->mm->arg_end = current->mm->env_start = p;
 	while (envc-- > 0) {
 		size_t len;
-		__put_user((elf_addr_t)p, envp++);
+		if (__put_user((elf_addr_t)p, envp++))
+			return -EFAULT;
 		len = strnlen_user((void __user *)p, PAGE_SIZE*MAX_ARG_PAGES);
 		if (!len || len > PAGE_SIZE*MAX_ARG_PAGES)
 			return 0;

[patch 2/5] compat: fix uaccess handling

[Heiko Carstens]

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 fs/compat.c       |   35 +++++++++++++++++++----------------
 fs/compat_ioctl.c |   33 ++++++++++++++++++++-------------
 2 files changed, 39 insertions(+), 29 deletions(-)

Index: linux-2.6/fs/compat.c
===================================================================
--- linux-2.6.orig/fs/compat.c	2006-10-26 14:40:56.000000000 +0200
+++ linux-2.6/fs/compat.c	2006-10-26 14:42:09.000000000 +0200
@@ -1142,7 +1142,9 @@
 	lastdirent = buf.previous;
 	if (lastdirent) {
 		typeof(lastdirent->d_off) d_off = file->f_pos;
-		__put_user_unaligned(d_off, &lastdirent->d_off);
+		error = -EFAULT;
+		if (__put_user_unaligned(d_off, &lastdirent->d_off))
+			goto out_putf;
 		error = count - buf.count;
 	}
 
@@ -1609,14 +1611,14 @@
 		nr &= ~1UL;
 		while (nr) {
 			unsigned long h, l;
-			__get_user(l, ufdset);
-			__get_user(h, ufdset+1);
+			if (__get_user(l, ufdset) || __get_user(h, ufdset+1))
+				return -EFAULT;
 			ufdset += 2;
 			*fdset++ = h << 32 | l;
 			nr -= 2;
 		}
-		if (odd)
-			__get_user(*fdset, ufdset);
+		if (odd && __get_user(*fdset, ufdset))
+			return -EFAULT;
 	} else {
 		/* Tricky, must clear full unsigned long in the
 		 * kernel fdset at the end, this makes sure that
@@ -1628,14 +1630,14 @@
 }
 
 static
-void compat_set_fd_set(unsigned long nr, compat_ulong_t __user *ufdset,
-			unsigned long *fdset)
+int compat_set_fd_set(unsigned long nr, compat_ulong_t __user *ufdset,
+		      unsigned long *fdset)
 {
 	unsigned long odd;
 	nr = ROUND_UP(nr, __COMPAT_NFDBITS);
 
 	if (!ufdset)
-		return;
+		return 0;
 
 	odd = nr & 1UL;
 	nr &= ~1UL;
@@ -1643,13 +1645,14 @@
 		unsigned long h, l;
 		l = *fdset++;
 		h = l >> 32;
-		__put_user(l, ufdset);
-		__put_user(h, ufdset+1);
+		if (__put_user(l, ufdset) || __put_user(h, ufdset+1))
+			return -EFAULT;
 		ufdset += 2;
 		nr -= 2;
 	}
-	if (odd)
-		__put_user(*fdset, ufdset);
+	if (odd && __put_user(*fdset, ufdset))
+		return -EFAULT;
+	return 0;
 }
 
 
@@ -1724,10 +1727,10 @@
 		ret = 0;
 	}
 
-	compat_set_fd_set(n, inp, fds.res_in);
-	compat_set_fd_set(n, outp, fds.res_out);
-	compat_set_fd_set(n, exp, fds.res_ex);
-
+	if (compat_set_fd_set(n, inp, fds.res_in) ||
+	    compat_set_fd_set(n, outp, fds.res_out) ||
+	    compat_set_fd_set(n, exp, fds.res_ex))
+		ret = -EFAULT;
 out:
 	kfree(bits);
 out_nofds:
Index: linux-2.6/fs/compat_ioctl.c
===================================================================
--- linux-2.6.orig/fs/compat_ioctl.c	2006-10-26 14:40:56.000000000 +0200
+++ linux-2.6/fs/compat_ioctl.c	2006-10-26 14:42:09.000000000 +0200
@@ -211,8 +211,10 @@
 	up_native =
 		compat_alloc_user_space(sizeof(struct video_still_picture));
 
-	put_user(compat_ptr(fp), &up_native->iFrame);
-	put_user(size, &up_native->size);
+	err =  put_user(compat_ptr(fp), &up_native->iFrame);
+	err |= put_user(size, &up_native->size);
+	if (err)
+		return -EFAULT;
 
 	err = sys_ioctl(fd, cmd, (unsigned long) up_native);
 
@@ -236,8 +238,10 @@
 	err |= get_user(length, &up->length);
 
 	up_native = compat_alloc_user_space(sizeof(struct video_spu_palette));
-	put_user(compat_ptr(palp), &up_native->palette);
-	put_user(length, &up_native->length);
+	err  = put_user(compat_ptr(palp), &up_native->palette);
+	err |= put_user(length, &up_native->length);
+	if (err)
+		return -EFAULT;
 
 	err = sys_ioctl(fd, cmd, (unsigned long) up_native);
 
@@ -2043,16 +2047,19 @@
         struct serial_struct ss;
         mm_segment_t oldseg = get_fs();
         __u32 udata;
+	unsigned int base;
 
         if (cmd == TIOCSSERIAL) {
                 if (!access_ok(VERIFY_READ, ss32, sizeof(SS32)))
                         return -EFAULT;
                 if (__copy_from_user(&ss, ss32, offsetof(SS32, iomem_base)))
 			return -EFAULT;
-                __get_user(udata, &ss32->iomem_base);
+                if (__get_user(udata, &ss32->iomem_base))
+			return -EFAULT;
                 ss.iomem_base = compat_ptr(udata);
-                __get_user(ss.iomem_reg_shift, &ss32->iomem_reg_shift);
-                __get_user(ss.port_high, &ss32->port_high);
+                if (__get_user(ss.iomem_reg_shift, &ss32->iomem_reg_shift) ||
+		    __get_user(ss.port_high, &ss32->port_high))
+			return -EFAULT;
                 ss.iomap_base = 0UL;
         }
         set_fs(KERNEL_DS);
@@ -2063,12 +2070,12 @@
                         return -EFAULT;
                 if (__copy_to_user(ss32,&ss,offsetof(SS32,iomem_base)))
 			return -EFAULT;
-                __put_user((unsigned long)ss.iomem_base  >> 32 ?
-                            0xffffffff : (unsigned)(unsigned long)ss.iomem_base,
-                            &ss32->iomem_base);
-                __put_user(ss.iomem_reg_shift, &ss32->iomem_reg_shift);
-                __put_user(ss.port_high, &ss32->port_high);
-
+		base = (unsigned long)ss.iomem_base  >> 32 ?
+			0xffffffff : (unsigned)(unsigned long)ss.iomem_base;
+		if (__put_user(base, &ss32->iomem_base) ||
+		    __put_user(ss.iomem_reg_shift, &ss32->iomem_reg_shift) ||
+		    __put_user(ss.port_high, &ss32->port_high))
+			return -EFAULT;
         }
         return err;
 }

[patch 3/5] net: fix uaccess handling

[Heiko Carstens]

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 net/ipv4/raw.c           |   17 +++++++++++------
 net/ipv6/raw.c           |   17 +++++++++++------
 net/netlink/af_netlink.c |    5 +++--
 3 files changed, 25 insertions(+), 14 deletions(-)

Index: linux-2.6/net/ipv4/raw.c
===================================================================
--- linux-2.6.orig/net/ipv4/raw.c	2006-10-26 14:40:56.000000000 +0200
+++ linux-2.6/net/ipv4/raw.c	2006-10-26 14:42:12.000000000 +0200
@@ -329,7 +329,7 @@
 	return err; 
 }
 
-static void raw_probe_proto_opt(struct flowi *fl, struct msghdr *msg)
+static int raw_probe_proto_opt(struct flowi *fl, struct msghdr *msg)
 {
 	struct iovec *iov;
 	u8 __user *type = NULL;
@@ -338,7 +338,7 @@
 	unsigned int i;
 
 	if (!msg->msg_iov)
-		return;
+		return 0;
 
 	for (i = 0; i < msg->msg_iovlen; i++) {
 		iov = &msg->msg_iov[i];
@@ -360,8 +360,9 @@
 				code = iov->iov_base;
 
 			if (type && code) {
-				get_user(fl->fl_icmp_type, type);
-			        get_user(fl->fl_icmp_code, code);
+				if (get_user(fl->fl_icmp_type, type) ||
+				    get_user(fl->fl_icmp_code, code))
+					return -EFAULT;
 				probed = 1;
 			}
 			break;
@@ -372,6 +373,7 @@
 		if (probed)
 			break;
 	}
+	return 0;
 }
 
 static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
@@ -480,8 +482,11 @@
 				    .proto = inet->hdrincl ? IPPROTO_RAW :
 					    		     sk->sk_protocol,
 				  };
-		if (!inet->hdrincl)
-			raw_probe_proto_opt(&fl, msg);
+		if (!inet->hdrincl) {
+			err = raw_probe_proto_opt(&fl, msg);
+			if (err)
+				goto done;
+		}
 
 		security_sk_classify_flow(sk, &fl);
 		err = ip_route_output_flow(&rt, &fl, sk, !(msg->msg_flags&MSG_DONTWAIT));
Index: linux-2.6/net/ipv6/raw.c
===================================================================
--- linux-2.6.orig/net/ipv6/raw.c	2006-10-26 14:40:56.000000000 +0200
+++ linux-2.6/net/ipv6/raw.c	2006-10-26 14:42:12.000000000 +0200
@@ -604,7 +604,7 @@
 	return err; 
 }
 
-static void rawv6_probe_proto_opt(struct flowi *fl, struct msghdr *msg)
+static int rawv6_probe_proto_opt(struct flowi *fl, struct msghdr *msg)
 {
 	struct iovec *iov;
 	u8 __user *type = NULL;
@@ -616,7 +616,7 @@
 	int i;
 
 	if (!msg->msg_iov)
-		return;
+		return 0;
 
 	for (i = 0; i < msg->msg_iovlen; i++) {
 		iov = &msg->msg_iov[i];
@@ -638,8 +638,9 @@
 				code = iov->iov_base;
 
 			if (type && code) {
-				get_user(fl->fl_icmp_type, type);
-				get_user(fl->fl_icmp_code, code);
+				if (get_user(fl->fl_icmp_type, type) ||
+				    get_user(fl->fl_icmp_code, code))
+					return -EFAULT;
 				probed = 1;
 			}
 			break;
@@ -650,7 +651,8 @@
 			/* check if type field is readable or not. */
 			if (iov->iov_len > 2 - len) {
 				u8 __user *p = iov->iov_base;
-				get_user(fl->fl_mh_type, &p[2 - len]);
+				if (get_user(fl->fl_mh_type, &p[2 - len]))
+					return -EFAULT;
 				probed = 1;
 			} else
 				len += iov->iov_len;
@@ -664,6 +666,7 @@
 		if (probed)
 			break;
 	}
+	return 0;
 }
 
 static int rawv6_sendmsg(struct kiocb *iocb, struct sock *sk,
@@ -787,7 +790,9 @@
 	opt = ipv6_fixup_options(&opt_space, opt);
 
 	fl.proto = proto;
-	rawv6_probe_proto_opt(&fl, msg);
+	err = rawv6_probe_proto_opt(&fl, msg);
+	if (err)
+		goto out;
  
 	ipv6_addr_copy(&fl.fl6_dst, daddr);
 	if (ipv6_addr_any(&fl.fl6_src) && !ipv6_addr_any(&np->saddr))
Index: linux-2.6/net/netlink/af_netlink.c
===================================================================
--- linux-2.6.orig/net/netlink/af_netlink.c	2006-10-26 14:40:56.000000000 +0200
+++ linux-2.6/net/netlink/af_netlink.c	2006-10-26 14:42:12.000000000 +0200
@@ -1075,8 +1075,9 @@
 			return -EINVAL;
 		len = sizeof(int);
 		val = nlk->flags & NETLINK_RECV_PKTINFO ? 1 : 0;
-		put_user(len, optlen);
-		put_user(val, optval);
+		if (put_user(len, optlen) ||
+		    put_user(val, optval))
+			return -EFAULT;
 		err = 0;
 		break;
 	default:

[patch 4/5] profile: fix uaccess handling

[Heiko Carstens]

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 kernel/profile.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6/kernel/profile.c
===================================================================
--- linux-2.6.orig/kernel/profile.c	2006-10-26 14:40:57.000000000 +0200
+++ linux-2.6/kernel/profile.c	2006-10-26 14:42:07.000000000 +0200
@@ -442,7 +442,8 @@
 	read = 0;
 
 	while (p < sizeof(unsigned int) && count > 0) {
-		put_user(*((char *)(&sample_step)+p),buf);
+		if (put_user(*((char *)(&sample_step)+p),buf))
+			return -EFAULT;
 		buf++; p++; count--; read++;
 	}
 	pnt = (char *)prof_buffer + p - sizeof(atomic_t);

[patch 5/5] scsi: fix uaccess handling

[Heiko Carstens]

Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 drivers/scsi/scsi_ioctl.c |   17 +++++++++-------
 drivers/scsi/sg.c         |   47 ++++++++++++++++++++++++----------------------
 2 files changed, 35 insertions(+), 29 deletions(-)

Index: linux-2.6/drivers/scsi/scsi_ioctl.c
===================================================================
--- linux-2.6.orig/drivers/scsi/scsi_ioctl.c	2006-10-26 14:40:55.000000000 +0200
+++ linux-2.6/drivers/scsi/scsi_ioctl.c	2006-10-26 14:42:14.000000000 +0200
@@ -217,13 +217,16 @@
 		if (!access_ok(VERIFY_WRITE, arg, sizeof(struct scsi_idlun)))
 			return -EFAULT;
 
-		__put_user((sdev->id & 0xff)
-			 + ((sdev->lun & 0xff) << 8)
-			 + ((sdev->channel & 0xff) << 16)
-			 + ((sdev->host->host_no & 0xff) << 24),
-			 &((struct scsi_idlun __user *)arg)->dev_id);
-		__put_user(sdev->host->unique_id,
-			 &((struct scsi_idlun __user *)arg)->host_unique_id);
+		if (__put_user((sdev->id & 0xff)
+			       + ((sdev->lun & 0xff) << 8)
+			       + ((sdev->channel & 0xff) << 16)
+			       + ((sdev->host->host_no & 0xff) << 24),
+			       &((struct scsi_idlun __user *)arg)->dev_id))
+			return -EFAULT;
+
+		if(__put_user(sdev->host->unique_id,
+			&((struct scsi_idlun __user *)arg)->host_unique_id))
+			return -EFAULT;
 		return 0;
 	case SCSI_IOCTL_GET_BUS_NUMBER:
 		return put_user(sdev->host->host_no, (int __user *)arg);
Index: linux-2.6/drivers/scsi/sg.c
===================================================================
--- linux-2.6.orig/drivers/scsi/sg.c	2006-10-26 14:40:55.000000000 +0200
+++ linux-2.6/drivers/scsi/sg.c	2006-10-26 14:42:14.000000000 +0200
@@ -556,7 +556,8 @@
 		return -EDOM;
 	}
 	buf += SZ_SG_HEADER;
-	__get_user(opcode, buf);
+	if (__get_user(opcode, buf))
+		return -EFAULT;
 	if (sfp->next_cmd_len > 0) {
 		if (sfp->next_cmd_len > MAX_COMMAND_SIZE) {
 			SCSI_LOG_TIMEOUT(1, printk("sg_write: command length too long\n"));
@@ -779,6 +780,7 @@
 	Sg_fd *sfp;
 	Sg_request *srp;
 	unsigned long iflags;
+	sg_scsi_id_t __user *sg_idp;
 
 	if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp)))
 		return -ENXIO;
@@ -860,26 +862,25 @@
 	case SG_GET_SCSI_ID:
 		if (!access_ok(VERIFY_WRITE, p, sizeof (sg_scsi_id_t)))
 			return -EFAULT;
-		else {
-			sg_scsi_id_t __user *sg_idp = p;
+		sg_idp = p;
 
-			if (sdp->detached)
-				return -ENODEV;
-			__put_user((int) sdp->device->host->host_no,
-				   &sg_idp->host_no);
-			__put_user((int) sdp->device->channel,
-				   &sg_idp->channel);
-			__put_user((int) sdp->device->id, &sg_idp->scsi_id);
-			__put_user((int) sdp->device->lun, &sg_idp->lun);
-			__put_user((int) sdp->device->type, &sg_idp->scsi_type);
-			__put_user((short) sdp->device->host->cmd_per_lun,
-				   &sg_idp->h_cmd_per_lun);
-			__put_user((short) sdp->device->queue_depth,
-				   &sg_idp->d_queue_depth);
-			__put_user(0, &sg_idp->unused[0]);
-			__put_user(0, &sg_idp->unused[1]);
-			return 0;
-		}
+		if (sdp->detached)
+			return -ENODEV;
+		if (__put_user((int) sdp->device->host->host_no,
+			       &sg_idp->host_no) ||
+		    __put_user((int) sdp->device->channel,
+			       &sg_idp->channel) ||
+		    __put_user((int) sdp->device->id, &sg_idp->scsi_id) ||
+		    __put_user((int) sdp->device->lun, &sg_idp->lun) ||
+		    __put_user((int) sdp->device->type, &sg_idp->scsi_type) ||
+		    __put_user((short) sdp->device->host->cmd_per_lun,
+			       &sg_idp->h_cmd_per_lun) ||
+		    __put_user((short) sdp->device->queue_depth,
+			       &sg_idp->d_queue_depth) ||
+		    __put_user(0, &sg_idp->unused[0]) ||
+		    __put_user(0, &sg_idp->unused[1]))
+			return -EFAULT;
+		return 0;
 	case SG_SET_FORCE_PACK_ID:
 		result = get_user(val, ip);
 		if (result)
@@ -894,12 +895,14 @@
 			if ((1 == srp->done) && (!srp->sg_io_owned)) {
 				read_unlock_irqrestore(&sfp->rq_list_lock,
 						       iflags);
-				__put_user(srp->header.pack_id, ip);
+				if (__put_user(srp->header.pack_id, ip))
+					return -EFAULT;
 				return 0;
 			}
 		}
 		read_unlock_irqrestore(&sfp->rq_list_lock, iflags);
-		__put_user(-1, ip);
+		if (__put_user(-1, ip))
+			return -EFAULT;
 		return 0;
 	case SG_GET_NUM_WAITING:
 		read_lock_irqsave(&sfp->rq_list_lock, iflags);

Re: [patch 1/5] binfmt: fix uaccess handling

[Andrew Morton]

On Thu, 26 Oct 2006 15:01:46 +0200
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:

> @@ -254,7 +255,8 @@
>  	p = current->mm->arg_end = current->mm->arg_start;
>  	while (argc-- > 0) {
>  		size_t len;
> -		__put_user((elf_addr_t)p, argv++);
> +		if (__put_user((elf_addr_t)p, argv++))
> +			return -EFAULT;
>  		len = strnlen_user((void __user *)p, PAGE_SIZE*MAX_ARG_PAGES);
>  		if (!len || len > PAGE_SIZE*MAX_ARG_PAGES)
>  			return 0;

We return EFAULT, but if strnlen_user() gets a fault, we return zero.  But
then, the return value of create_elf_tables() gets cheerfully ignored
anyway.  So I assume we start up some new program with a
partially-constructed environment and it then mysteriously malfunctions.

Nice, eh?

Re: [patch 5/5] scsi: fix uaccess handling

[Christoph Hellwig]

On Thu, Oct 26, 2006 at 03:04:52PM +0200, Heiko Carstens wrote:
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
>  drivers/scsi/scsi_ioctl.c |   17 +++++++++-------
>  drivers/scsi/sg.c         |   47 ++++++++++++++++++++++++----------------------
>  2 files changed, 35 insertions(+), 29 deletions(-)
> 
> Index: linux-2.6/drivers/scsi/scsi_ioctl.c
> ===================================================================
> --- linux-2.6.orig/drivers/scsi/scsi_ioctl.c	2006-10-26 14:40:55.000000000 +0200
> +++ linux-2.6/drivers/scsi/scsi_ioctl.c	2006-10-26 14:42:14.000000000 +0200
> @@ -217,13 +217,16 @@
>  		if (!access_ok(VERIFY_WRITE, arg, sizeof(struct scsi_idlun)))
>  			return -EFAULT;
>  
> -		__put_user((sdev->id & 0xff)
> -			 + ((sdev->lun & 0xff) << 8)
> -			 + ((sdev->channel & 0xff) << 16)
> -			 + ((sdev->host->host_no & 0xff) << 24),
> -			 &((struct scsi_idlun __user *)arg)->dev_id);
> -		__put_user(sdev->host->unique_id,
> -			 &((struct scsi_idlun __user *)arg)->host_unique_id);
> +		if (__put_user((sdev->id & 0xff)
> +			       + ((sdev->lun & 0xff) << 8)
> +			       + ((sdev->channel & 0xff) << 16)
> +			       + ((sdev->host->host_no & 0xff) << 24),
> +			       &((struct scsi_idlun __user *)arg)->dev_id))
> +			return -EFAULT;
> +

While not your fault I'd suggest to fix the __put_user abuse at the same
time, as in the untested patch below for scsi_ioctl.c:

Index: linux-2.6/drivers/scsi/scsi_ioctl.c
===================================================================
--- linux-2.6.orig/drivers/scsi/scsi_ioctl.c	2006-10-28 13:24:18.000000000 +0200
+++ linux-2.6/drivers/scsi/scsi_ioctl.c	2006-10-28 13:30:17.000000000 +0200
@@ -173,6 +173,21 @@
         return copy_to_user(arg, dev->bus_id, sizeof(dev->bus_id))? -EFAULT: 0;
 }
 
+static int scsi_get_idlun(struct scsi_device *sdev,
+		struct scsi_idlun __user *arg)
+{
+	struct scsi_idlun karg = {
+		.dev_id		= (sdev->id & 0xff) +
+			          ((sdev->lun & 0xff) << 8) +
+				  ((sdev->channel & 0xff) << 16) +
+				  ((sdev->host->host_no & 0xff) << 24),
+		.host_unique_id	= sdev->host->unique_id
+	};
+
+	if (copy_to_user(arg, &karg, sizeof(struct scsi_idlun)))
+		return -EFAULT;
+	return 0;
+}
 
 /*
  * the scsi_ioctl() function differs from most ioctls in that it does
@@ -214,17 +229,7 @@
 
 	switch (cmd) {
 	case SCSI_IOCTL_GET_IDLUN:
-		if (!access_ok(VERIFY_WRITE, arg, sizeof(struct scsi_idlun)))
-			return -EFAULT;
-
-		__put_user((sdev->id & 0xff)
-			 + ((sdev->lun & 0xff) << 8)
-			 + ((sdev->channel & 0xff) << 16)
-			 + ((sdev->host->host_no & 0xff) << 24),
-			 &((struct scsi_idlun __user *)arg)->dev_id);
-		__put_user(sdev->host->unique_id,
-			 &((struct scsi_idlun __user *)arg)->host_unique_id);
-		return 0;
+		return scsi_get_idlun(sdev, arg);
 	case SCSI_IOCTL_GET_BUS_NUMBER:
 		return put_user(sdev->host->host_no, (int __user *)arg);
 	case SCSI_IOCTL_PROBE_HOST:

Re: [patch 5/5] scsi: fix uaccess handling

[Heiko Carstens]

> While not your fault I'd suggest to fix the __put_user abuse at the same
> time, as in the untested patch below for scsi_ioctl.c:

Makes sense. Even though the whole SCSI_IOCTL_GET_IDLUN ioctl interface
is pretty pointless.
It supports only up to 255 different ids and luns and might return the
same 'dev_id' for two different devices...
Any user space utility that depends on this interface would do the wrong
thing (whatever that would be).

> Index: linux-2.6/drivers/scsi/scsi_ioctl.c
> ===================================================================
> --- linux-2.6.orig/drivers/scsi/scsi_ioctl.c	2006-10-28 13:24:18.000000000 +0200
> +++ linux-2.6/drivers/scsi/scsi_ioctl.c	2006-10-28 13:30:17.000000000 +0200
> @@ -173,6 +173,21 @@
>          return copy_to_user(arg, dev->bus_id, sizeof(dev->bus_id))? -EFAULT: 0;
>  }
>  
> +static int scsi_get_idlun(struct scsi_device *sdev,
> +		struct scsi_idlun __user *arg)
> +{
> +	struct scsi_idlun karg = {
> +		.dev_id		= (sdev->id & 0xff) +
> +			          ((sdev->lun & 0xff) << 8) +
> +				  ((sdev->channel & 0xff) << 16) +
> +				  ((sdev->host->host_no & 0xff) << 24),
> +		.host_unique_id	= sdev->host->unique_id
> +	};
> +
> +	if (copy_to_user(arg, &karg, sizeof(struct scsi_idlun)))
> +		return -EFAULT;
> +	return 0;
> +}
>  
>  /*
>   * the scsi_ioctl() function differs from most ioctls in that it does
> @@ -214,17 +229,7 @@
>  
>  	switch (cmd) {
>  	case SCSI_IOCTL_GET_IDLUN:
> -		if (!access_ok(VERIFY_WRITE, arg, sizeof(struct scsi_idlun)))
> -			return -EFAULT;
> -
> -		__put_user((sdev->id & 0xff)
> -			 + ((sdev->lun & 0xff) << 8)
> -			 + ((sdev->channel & 0xff) << 16)
> -			 + ((sdev->host->host_no & 0xff) << 24),
> -			 &((struct scsi_idlun __user *)arg)->dev_id);
> -		__put_user(sdev->host->unique_id,
> -			 &((struct scsi_idlun __user *)arg)->host_unique_id);
> -		return 0;
> +		return scsi_get_idlun(sdev, arg);
>  	case SCSI_IOCTL_GET_BUS_NUMBER:
>  		return put_user(sdev->host->host_no, (int __user *)arg);
>  	case SCSI_IOCTL_PROBE_HOST:

Re: [patch 3/5] net: fix uaccess handling

[David Miller]

From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Thu, 26 Oct 2006 15:03:17 +0200

> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
>  net/ipv4/raw.c           |   17 +++++++++++------
>  net/ipv6/raw.c           |   17 +++++++++++------
>  net/netlink/af_netlink.c |    5 +++--
>  3 files changed, 25 insertions(+), 14 deletions(-)

Patch applied, thanks.

Re: [patch 5/5] scsi: fix uaccess handling

[James Bottomley]

On Sun, 2006-10-29 at 22:39 +0100, Heiko Carstens wrote:
> > While not your fault I'd suggest to fix the __put_user abuse at the same
> > time, as in the untested patch below for scsi_ioctl.c:
> 
> Makes sense. Even though the whole SCSI_IOCTL_GET_IDLUN ioctl interface
> is pretty pointless.
> It supports only up to 255 different ids and luns and might return the
> same 'dev_id' for two different devices...
> Any user space utility that depends on this interface would do the wrong
> thing (whatever that would be).

So has anyone actually tested this?

James