* [patch] tun: signedness bug in tun_get_user()
@ 2013-08-15 12:52 ` Dan Carpenter
0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2013-08-15 12:52 UTC (permalink / raw)
To: David S. Miller
Cc: Jason Wang, Michael S. Tsirkin, Eric Dumazet, Neil Horman, netdev,
kernel-janitors
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@oracle.com>
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;
+ len -= tun->vnet_hdr_sz;
if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
return -EFAULT;
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [patch] tun: signedness bug in tun_get_user()
2013-08-15 12:52 ` Dan Carpenter
@ 2013-08-15 14:04 ` Michael S. Tsirkin
-1 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-08-15 14:04 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
kernel-janitors
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@oracle.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> 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;
> + len -= tun->vnet_hdr_sz;
>
> if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> return -EFAULT;
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch] tun: signedness bug in tun_get_user()
@ 2013-08-15 14:04 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-08-15 14:04 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
kernel-janitors
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@oracle.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> 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;
> + len -= tun->vnet_hdr_sz;
>
> if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> return -EFAULT;
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch] tun: signedness bug in tun_get_user()
2013-08-15 14:04 ` Michael S. Tsirkin
@ 2013-08-15 14:58 ` Michael S. Tsirkin
-1 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-08-15 14:58 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
kernel-janitors
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@oracle.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.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;
> > + len -= tun->vnet_hdr_sz;
> >
> > if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> > return -EFAULT;
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch] tun: signedness bug in tun_get_user()
@ 2013-08-15 14:58 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-08-15 14:58 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
kernel-janitors
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@oracle.com>
>
> Acked-by: Michael S. Tsirkin <mst@redhat.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;
> > + len -= tun->vnet_hdr_sz;
> >
> > if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> > return -EFAULT;
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch] tun: signedness bug in tun_get_user()
2013-08-15 14:58 ` Michael S. Tsirkin
@ 2013-08-15 15:02 ` Michael S. Tsirkin
-1 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-08-15 15:02 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
kernel-janitors
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@oracle.com>
> >
> > Acked-by: Michael S. Tsirkin <mst@redhat.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.
> > > + len -= tun->vnet_hdr_sz;
> > >
> > > if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> > > return -EFAULT;
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch] tun: signedness bug in tun_get_user()
@ 2013-08-15 15:02 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-08-15 15:02 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
kernel-janitors
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@oracle.com>
> >
> > Acked-by: Michael S. Tsirkin <mst@redhat.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.
> > > + len -= tun->vnet_hdr_sz;
> > >
> > > if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> > > return -EFAULT;
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch] tun: signedness bug in tun_get_user()
2013-08-15 15:02 ` Michael S. Tsirkin
@ 2013-08-15 15:05 ` Michael S. Tsirkin
-1 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-08-15 15:05 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
kernel-janitors
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@oracle.com>
> > >
> > > Acked-by: Michael S. Tsirkin <mst@redhat.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;
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch] tun: signedness bug in tun_get_user()
@ 2013-08-15 15:05 ` Michael S. Tsirkin
0 siblings, 0 replies; 14+ messages in thread
From: Michael S. Tsirkin @ 2013-08-15 15:05 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Jason Wang, Eric Dumazet, Neil Horman, netdev,
kernel-janitors
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@oracle.com>
> > >
> > > Acked-by: Michael S. Tsirkin <mst@redhat.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;
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch] tun: signedness bug in tun_get_user()
2013-08-15 12:52 ` Dan Carpenter
@ 2013-08-15 20:08 ` Neil Horman
-1 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2013-08-15 20:08 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Jason Wang, Michael S. Tsirkin, Eric Dumazet,
netdev, kernel-janitors
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@oracle.com>
>
> 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;
> + len -= tun->vnet_hdr_sz;
>
> if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> return -EFAULT;
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch] tun: signedness bug in tun_get_user()
@ 2013-08-15 20:08 ` Neil Horman
0 siblings, 0 replies; 14+ messages in thread
From: Neil Horman @ 2013-08-15 20:08 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Jason Wang, Michael S. Tsirkin, Eric Dumazet,
netdev, kernel-janitors
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@oracle.com>
>
> 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;
> + len -= tun->vnet_hdr_sz;
>
> if (memcpy_fromiovecend((void *)&gso, iv, offset, sizeof(gso)))
> return -EFAULT;
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [patch] tun: signedness bug in tun_get_user()
2013-08-15 20:08 ` Neil Horman
@ 2013-08-15 21:51 ` David Miller
-1 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2013-08-15 21:51 UTC (permalink / raw)
To: nhorman; +Cc: dan.carpenter, jasowang, mst, edumazet, netdev, kernel-janitors
From: Neil Horman <nhorman@tuxdriver.com>
Date: Thu, 15 Aug 2013 16:08:35 -0400
> 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@oracle.com>
...
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [patch] tun: signedness bug in tun_get_user()
@ 2013-08-15 21:51 ` David Miller
0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2013-08-15 21:51 UTC (permalink / raw)
To: nhorman; +Cc: dan.carpenter, jasowang, mst, edumazet, netdev, kernel-janitors
From: Neil Horman <nhorman@tuxdriver.com>
Date: Thu, 15 Aug 2013 16:08:35 -0400
> 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@oracle.com>
...
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks everyone.
^ permalink raw reply [flat|nested] 14+ messages in thread