[PATCH 1/4] ax25 check error on memcpy_fromiovec

[PATCH 1/4] ax25 check error on memcpy_fromiovec

[Chris Wright]

Check the return value on memcpy_fromiovec().

===== net/ax25/af_ax25.c 1.33 vs edited =====
--- 1.33/net/ax25/af_ax25.c	Tue Oct  7 06:27:14 2003
+++ edited/net/ax25/af_ax25.c	Fri Dec  5 16:43:44 2003
@@ -1534,7 +1534,12 @@
 	SOCK_DEBUG(sk, "AX.25: Appending user data\n");
 
 	/* User data follows immediately after the AX.25 data */
-	memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len);
+	if (memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len)) {
+		err = -EFAULT;
+		kfree_skb(skb);
+		goto out;
+	}
+
 	skb->nh.raw = skb->data;
 
 	/* Add the PID if one is not supplied by the user in the skb */

[PATCH 2/4] irda check error on memcpy_fromiovec

[Chris Wright]

Check the return value on memcpy_fromiovec().


===== net/irda/af_irda.c 1.47 vs edited =====
--- 1.47/net/irda/af_irda.c	Tue Nov  4 16:29:43 2003
+++ edited/net/irda/af_irda.c	Fri Dec  5 17:29:04 2003
@@ -1307,7 +1307,11 @@
 	skb_reserve(skb, self->max_header_size + 16);
 
 	asmptr = skb->h.raw = skb_put(skb, len);
-	memcpy_fromiovec(asmptr, msg->msg_iov, len);
+	err = memcpy_fromiovec(asmptr, msg->msg_iov, len);
+	if (err) {
+		kfree_skb(skb);
+		return err;
+	}
 
 	/*
 	 * Just send the message to TinyTP, and let it deal with possible
@@ -1549,7 +1553,11 @@
 
 	IRDA_DEBUG(4, "%s(), appending user data\n", __FUNCTION__);
 	asmptr = skb->h.raw = skb_put(skb, len);
-	memcpy_fromiovec(asmptr, msg->msg_iov, len);
+	err = memcpy_fromiovec(asmptr, msg->msg_iov, len);
+	if (err) {
+		kfree_skb(skb);
+		return err;
+	}
 
 	/*
 	 * Just send the message to TinyTP, and let it deal with possible
@@ -1612,7 +1620,11 @@
 
 	IRDA_DEBUG(4, "%s(), appending user data\n", __FUNCTION__);
 	asmptr = skb->h.raw = skb_put(skb, len);
-	memcpy_fromiovec(asmptr, msg->msg_iov, len);
+	err = memcpy_fromiovec(asmptr, msg->msg_iov, len);
+	if (err) {
+		kfree_skb(skb);
+		return err;
+	}
 
 	err = irlmp_connless_data_request(self->lsap, skb);
 	if (err) {

[PATCH 3/4] netrom check error on memcpy_fromiovec

[Chris Wright]

Check the return value on memcpy_fromiovec().

===== net/netrom/af_netrom.c 1.40 vs edited =====
--- 1.40/net/netrom/af_netrom.c	Sun Sep 21 18:17:31 2003
+++ edited/net/netrom/af_netrom.c	Fri Dec  5 16:45:14 2003
@@ -1094,7 +1094,12 @@
 	SOCK_DEBUG(sk, "NET/ROM: Appending user data\n");
 
 	/* User data follows immediately after the NET/ROM transport header */
-	memcpy_fromiovec(asmptr, msg->msg_iov, len);
+	if (memcpy_fromiovec(asmptr, msg->msg_iov, len)) {
+		kfree_skb(skb);
+		err = -EFAULT;
+		goto out;
+	}
+
 	SOCK_DEBUG(sk, "NET/ROM: Transmitting buffer\n");
 
 	if (sk->sk_state != TCP_ESTABLISHED) {

[PATCH 4/4] rose check error on memcpy_fromiovec

[Chris Wright]

Check the return value on memcpy_fromiovec().

===== net/rose/af_rose.c 1.33 vs edited =====
--- 1.33/net/rose/af_rose.c	Tue Sep  9 11:22:56 2003
+++ edited/net/rose/af_rose.c	Mon Dec  8 16:58:33 2003
@@ -1079,7 +1079,11 @@
 
 	asmptr = skb->h.raw = skb_put(skb, len);
 
-	memcpy_fromiovec(asmptr, msg->msg_iov, len);
+	err = memcpy_fromiovec(asmptr, msg->msg_iov, len);
+	if (err) {
+		kfree_skb(skb);
+		return err;
+	}
 
 	/*
 	 *	If the Q BIT Include socket option is in force, the first

Re: [PATCH 1/4] ax25 check error on memcpy_fromiovec

[David S. Miller]

On Mon, 8 Dec 2003 20:23:02 -0800
Chris Wright <chrisw@osdl.org> wrote:

> Check the return value on memcpy_fromiovec().

I'm fine with all of these patches, but since these problems
are not super-critical can you resend these for 2.6.1 perhaps?

Re: [PATCH 1/4] ax25 check error on memcpy_fromiovec

[Chris Wright]

* David S. Miller (davem@redhat.com) wrote:
> I'm fine with all of these patches, but since these problems
> are not super-critical can you resend these for 2.6.1 perhaps?

Sure, no problem.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

[PATCH 1/5] tun check error on memcpy_fromiovec

[maximilian attems]

[-- Attachment #1: Type: text/plain, Size: 677 bytes --]

hey chris,

after applying your 4 patches on top of linux-2.6.0 
+ experimental net, 
found 2 last unchecked memcpy_fromiovec in tun.c
patch bellow fixes the second call, 
the first is beyond me, please complete this patch :)
compile tested

thx max



--- linux-2.6.0-net/drivers/net/tun.c	2003-12-19 11:27:38.000000000 +0100
+++ linux/drivers/net/tun.c	2003-12-19 11:19:33.000000000 +0100
@@ -193,7 +193,10 @@
 	}
 
 	skb_reserve(skb, 2);
-	memcpy_fromiovec(skb_put(skb, len), iv, len);
+	if (memcpy_fromiovec(skb_put(skb, len), iv, len)) {
+		kfree_skb(skb);
+		return -EFAULT;
+	}
 
 	skb->dev = tun->dev;
 	switch (tun->flags & TUN_TYPE_MASK) {

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 1/5] tun check error on memcpy_fromiovec

[Jeff Garzik]

I'll take a look at merging this...

Re: [PATCH 1/5] tun check error on memcpy_fromiovec

[Chris Wright]

* maximilian attems (janitor@sternwelten.at) wrote:
> hey chris,
> 
> after applying your 4 patches on top of linux-2.6.0 
> + experimental net, 
> found 2 last unchecked memcpy_fromiovec in tun.c
> patch bellow fixes the second call, 
> the first is beyond me, please complete this patch :)
> compile tested

I specifically left those alone.  They have a semi-bogus verify_area()
call that is trying to insure the memcpy_fromiovec won't EFAULT.  I'd
prefer to remove them and simply do memcpy checking.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net

Re: [PATCH 1/5] tun check error on memcpy_fromiovec

[Max Krasnyansky]

On Fri, 2004-01-16 at 16:45, Chris Wright wrote:
> * maximilian attems (janitor@sternwelten.at) wrote:
> > hey chris,
> > 
> > after applying your 4 patches on top of linux-2.6.0 
> > + experimental net, 
> > found 2 last unchecked memcpy_fromiovec in tun.c
> > patch bellow fixes the second call, 
> > the first is beyond me, please complete this patch :)
> > compile tested
> 
> I specifically left those alone.  They have a semi-bogus verify_area()
> call that is trying to insure the memcpy_fromiovec won't EFAULT.  I'd
> prefer to remove them and simply do memcpy checking.

Folks,

Please don't add extra unneeded checks or fix stuff that does not need
to be fixed. Verify area is not bogus. We need to know total length of
the iovec so we might as well check it in the same loop and not bother
with checking later. 

Thanks
Max

Re: [PATCH 1/5] tun check error on memcpy_fromiovec

[Chris Wright]

* Max Krasnyansky (maxk@qualcomm.com) wrote:
> On Fri, 2004-01-16 at 16:45, Chris Wright wrote:
> > I specifically left those alone.  They have a semi-bogus verify_area()
> > call that is trying to insure the memcpy_fromiovec won't EFAULT.  I'd
> > prefer to remove them and simply do memcpy checking.
> 
> Please don't add extra unneeded checks or fix stuff that does not need
> to be fixed. Verify area is not bogus. We need to know total length of
> the iovec so we might as well check it in the same loop and not bother
> with checking later. 

Yes, I realize it's used to collect total length.  But it does not protect
from userspace controlled buffer whose contents can change.  Continuing
when a fault is possible means the skb->data could be zero'd.  However,
since this is intended to be 0700 root owned device, and the user could
supply same such data directly, it's of fairly low priority to patch.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net