* [PATCH 1/4] ax25 check error on memcpy_fromiovec
@ 2003-12-09 4:23 Chris Wright
2003-12-09 4:24 ` [PATCH 2/4] irda " Chris Wright
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Chris Wright @ 2003-12-09 4:23 UTC (permalink / raw)
To: netdev; +Cc: davem, shemminger
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 */
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 2/4] irda check error on memcpy_fromiovec 2003-12-09 4:23 [PATCH 1/4] ax25 check error on memcpy_fromiovec Chris Wright @ 2003-12-09 4:24 ` Chris Wright 2003-12-09 4:25 ` [PATCH 3/4] netrom " Chris Wright 2003-12-09 5:39 ` [PATCH 1/4] ax25 " David S. Miller 2003-12-19 10:34 ` [PATCH 1/5] tun " maximilian attems 2 siblings, 1 reply; 11+ messages in thread From: Chris Wright @ 2003-12-09 4:24 UTC (permalink / raw) To: netdev; +Cc: davem, shemminger 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) { ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] netrom check error on memcpy_fromiovec 2003-12-09 4:24 ` [PATCH 2/4] irda " Chris Wright @ 2003-12-09 4:25 ` Chris Wright 2003-12-09 4:27 ` [PATCH 4/4] rose " Chris Wright 0 siblings, 1 reply; 11+ messages in thread From: Chris Wright @ 2003-12-09 4:25 UTC (permalink / raw) To: netdev; +Cc: davem, shemminger 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) { ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] rose check error on memcpy_fromiovec 2003-12-09 4:25 ` [PATCH 3/4] netrom " Chris Wright @ 2003-12-09 4:27 ` Chris Wright 0 siblings, 0 replies; 11+ messages in thread From: Chris Wright @ 2003-12-09 4:27 UTC (permalink / raw) To: netdev; +Cc: davem, shemminger 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] ax25 check error on memcpy_fromiovec 2003-12-09 4:23 [PATCH 1/4] ax25 check error on memcpy_fromiovec Chris Wright 2003-12-09 4:24 ` [PATCH 2/4] irda " Chris Wright @ 2003-12-09 5:39 ` David S. Miller 2003-12-09 5:41 ` Chris Wright 2003-12-19 10:34 ` [PATCH 1/5] tun " maximilian attems 2 siblings, 1 reply; 11+ messages in thread From: David S. Miller @ 2003-12-09 5:39 UTC (permalink / raw) To: Chris Wright; +Cc: netdev, shemminger 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? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] ax25 check error on memcpy_fromiovec 2003-12-09 5:39 ` [PATCH 1/4] ax25 " David S. Miller @ 2003-12-09 5:41 ` Chris Wright 0 siblings, 0 replies; 11+ messages in thread From: Chris Wright @ 2003-12-09 5:41 UTC (permalink / raw) To: David S. Miller; +Cc: Chris Wright, netdev, shemminger * 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] tun check error on memcpy_fromiovec 2003-12-09 4:23 [PATCH 1/4] ax25 check error on memcpy_fromiovec Chris Wright 2003-12-09 4:24 ` [PATCH 2/4] irda " Chris Wright 2003-12-09 5:39 ` [PATCH 1/4] ax25 " David S. Miller @ 2003-12-19 10:34 ` maximilian attems 2003-12-19 14:23 ` Jeff Garzik 2004-01-17 0:45 ` Chris Wright 2 siblings, 2 replies; 11+ messages in thread From: maximilian attems @ 2003-12-19 10:34 UTC (permalink / raw) To: Chris Wright; +Cc: netdev [-- 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 --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] tun check error on memcpy_fromiovec 2003-12-19 10:34 ` [PATCH 1/5] tun " maximilian attems @ 2003-12-19 14:23 ` Jeff Garzik 2004-01-17 0:45 ` Chris Wright 1 sibling, 0 replies; 11+ messages in thread From: Jeff Garzik @ 2003-12-19 14:23 UTC (permalink / raw) To: maximilian attems; +Cc: Chris Wright, netdev I'll take a look at merging this... ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] tun check error on memcpy_fromiovec 2003-12-19 10:34 ` [PATCH 1/5] tun " maximilian attems 2003-12-19 14:23 ` Jeff Garzik @ 2004-01-17 0:45 ` Chris Wright 2004-01-21 20:49 ` Max Krasnyansky 1 sibling, 1 reply; 11+ messages in thread From: Chris Wright @ 2004-01-17 0:45 UTC (permalink / raw) To: maximilian attems; +Cc: Chris Wright, netdev * 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] tun check error on memcpy_fromiovec 2004-01-17 0:45 ` Chris Wright @ 2004-01-21 20:49 ` Max Krasnyansky 2004-01-23 2:13 ` Chris Wright 0 siblings, 1 reply; 11+ messages in thread From: Max Krasnyansky @ 2004-01-21 20:49 UTC (permalink / raw) To: Chris Wright; +Cc: maximilian attems, netdev 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] tun check error on memcpy_fromiovec 2004-01-21 20:49 ` Max Krasnyansky @ 2004-01-23 2:13 ` Chris Wright 0 siblings, 0 replies; 11+ messages in thread From: Chris Wright @ 2004-01-23 2:13 UTC (permalink / raw) To: Max Krasnyansky; +Cc: Chris Wright, maximilian attems, netdev * 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2004-01-23 2:13 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2003-12-09 4:23 [PATCH 1/4] ax25 check error on memcpy_fromiovec Chris Wright 2003-12-09 4:24 ` [PATCH 2/4] irda " Chris Wright 2003-12-09 4:25 ` [PATCH 3/4] netrom " Chris Wright 2003-12-09 4:27 ` [PATCH 4/4] rose " Chris Wright 2003-12-09 5:39 ` [PATCH 1/4] ax25 " David S. Miller 2003-12-09 5:41 ` Chris Wright 2003-12-19 10:34 ` [PATCH 1/5] tun " maximilian attems 2003-12-19 14:23 ` Jeff Garzik 2004-01-17 0:45 ` Chris Wright 2004-01-21 20:49 ` Max Krasnyansky 2004-01-23 2:13 ` Chris Wright
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).