lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 15 Aug 2013 18:05:17 +0300 From: "Michael S. Tsirkin" <mst@...hat.com> To: Dan Carpenter <dan.carpenter@...cle.com> Cc: "David S. Miller" <davem@...emloft.net>, Jason Wang <jasowang@...hat.com>, Eric Dumazet <edumazet@...gle.com>, Neil Horman <nhorman@...driver.com>, netdev@...r.kernel.org, kernel-janitors@...r.kernel.org Subject: Re: [patch] tun: signedness bug in tun_get_user() On Thu, Aug 15, 2013 at 06:02:14PM +0300, Michael S. Tsirkin wrote: > On Thu, Aug 15, 2013 at 05:58:40PM +0300, Michael S. Tsirkin wrote: > > On Thu, Aug 15, 2013 at 05:04:49PM +0300, Michael S. Tsirkin wrote: > > > On Thu, Aug 15, 2013 at 03:52:57PM +0300, Dan Carpenter wrote: > > > > The recent fix d9bf5f1309 "tun: compare with 0 instead of total_len" is > > > > not totally correct. Because "len" and "sizeof()" are size_t type, that > > > > means they are never less than zero. > > > > > > > > Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com> > > > > > > Acked-by: Michael S. Tsirkin <mst@...hat.com> > > > > Alternatively how about we revert the original patch? > > This is not the only issue it introduced and it doesn't > > actually fix any bugs. > > > > > > > > > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > > > > index af987f0..7ed13cc 100644 > > > > --- a/drivers/net/tun.c > > > > +++ b/drivers/net/tun.c > > > > @@ -977,8 +977,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, > > > > u32 rxhash; > > > > > > > > if (!(tun->flags & TUN_NO_PI)) { > > > > - if ((len -= sizeof(pi)) < 0) > > > > + if (len < sizeof(pi)) > > > > return -EINVAL; > > > > + len -= sizeof(pi); > > > > > > > > if (memcpy_fromiovecend((void *)&pi, iv, 0, sizeof(pi))) > > > > return -EFAULT; > > > > @@ -986,8 +987,9 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, > > > > } > > > > > > > > if (tun->flags & TUN_VNET_HDR) { > > > > - if ((len -= tun->vnet_hdr_sz) < 0) > > > > + if (len < tun->vnet_hdr_sz) > > > > return -EINVAL; > > And to be even more explicit, this still doesn't handle the > case vnet_hdr_sz < 0 properly. Hmm ENOCOFFEE. User can't make vnet_hdr_sz < 0 - we already catch that. So let's apply Dan's patch, it does fix all issues after all. Sorry about the noise. > > > > > + len -= tun->vnet_hdr_sz; > > > > > > > > if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso))) > > > > return -EFAULT; -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists