Skip to contentSkip to conversation

: Fix wrong interpretation of some copy_to_user() results.

PATCH In review Pavel Emelyanov 2 participants 2 messages Apr 25, 2008

Subsystems (5)
Networking Network drivers TCP TIPC network layer +1 more subsystems
lore
Share & tools
lore

Pavel Emelyanov

author
#0 lore raw

PATCH 1/1 : Fix wrong interpretation of some copy_to_user() results.

I found some places, that erroneously return the value obtained from the copy_to_user() call: if some amount of bytes were not able to get to the user (this is what this one returns) the proper behavior is to return the -EFAULT error, not that number itself.

Signed-off-by Pavel Emelyanov <[email protected]>

---

net/can/raw.c +2 -1
index ead50c7..201cbfc 100644
@@ -573,7 +573,8 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
 			int fsize = ro->count * sizeof(struct can_filter);
 			if (len > fsize)
 				len = fsize;
-			err = copy_to_user(optval, ro->filter, len);
+			if (copy_to_user(optval, ro->filter, len))
+				err = -EFAULT;
 		} else
 			len = 0;
 		release_sock(sk);
net/dccp/probe.c +1 -1
index 6e1df62..0bcdc92 100644
@@ -140,7 +140,7 @@ static ssize_t dccpprobe_read(struct file *file, char __user *buf,
 		goto out_free;
 
 	cnt = kfifo_get(dccpw.fifo, tbuf, len);
-	error = copy_to_user(buf, tbuf, cnt);
+	error = copy_to_user(buf, tbuf, cnt) ? -EFAULT : 0;
 
 out_free:
 	vfree(tbuf);
net/ipv4/tcp_probe.c +3 -2
index 1c50959..82a09aa 100644
@@ -200,9 +200,10 @@ static ssize_t tcpprobe_read(struct file *file, char __user *buf,
 		if (width >= len)
 			break;
 
-		error = copy_to_user(buf + cnt, tbuf, width);
-		if (error)
+		if (copy_to_user(buf + cnt, tbuf, width)) {
+			error = -EFAULT;
 			break;
+		}
 		cnt += width;
 	}
net/tipc/socket.c +2 -2
index 0585315..230f9ca 100644
@@ -1756,8 +1756,8 @@ static int getsockopt(struct socket *sock,
 	else if (len < sizeof(value)) {
 		res = -EINVAL;
 	}
-	else if ((res = copy_to_user(ov, &value, sizeof(value)))) {
-		/* couldn't return value */
+	else if (copy_to_user(ov, &value, sizeof(value))) {
+		res = -EFAULT;
 	}
 	else {
 		res = put_user(sizeof(value), ol);
maintainer
#1 lore raw

From: Pavel Emelyanov <[email protected]>
Date: Fri, 25 Apr 2008 12:41:55 +0400

Quoted 6 lines

I found some places, that erroneously return the value obtained from the copy_to_user() call: if some amount of bytes were not able to get to the user (this is what this one returns) the proper behavior is to return the -EFAULT error, not that number itself.

Signed-off-by Pavel Emelyanov <[email protected]>

I gues you're not a real kernel programmer unless you've made this mistake at least once :-) It kind of prove this is a bad interface, since it's so easy to use incorrectly.

This was already caught in a tcp_probe fix I applied earlier today, so I removed that part.

The rest is applied and I'll queue for -stable, thanks Pavel.