Re: [PATCH 3/5] uprobes: fix overflow in vma_address/find_active_uprobe

2012-07-11 Thread Srikar Dronamraju
* Oleg Nesterov  [2012-07-09 12:54:45]:

> On 07/08, Joe Perches wrote:
> >
> > On Sun, 2012-07-08 at 22:30 +0200, Oleg Nesterov wrote:
> > > @@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned 
> > > long bp_vaddr, int *is_swbp)
> > >
> > >   inode = vma->vm_file->f_mapping->host;
> > >   offset = bp_vaddr - vma->vm_start;
> > > - offset += (vma->vm_pgoff << PAGE_SHIFT);
> > > + offset += ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
> >
> > It's be nice to take remove the unnecessary parenthesis
> > and make it consistent with the vaddr use above it too.
> 
> OK, please find v2 below.
> 
> --
> Subject: [PATCH] uprobes: fix overflow in vma_address/find_active_uprobe
> 
> vma->vm_pgoff is "unsigned long", it should be promoted to loff_t
> before the multiplication to avoid the overflow.
> 
> Signed-off-by: Oleg Nesterov 

Acked-by: Srikar Dronamraju 

> ---
>  kernel/events/uprobes.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 47c4e24..6194edb 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -117,7 +117,7 @@ static loff_t vma_address(struct vm_area_struct *vma, 
> loff_t offset)
>   loff_t vaddr;
> 
>   vaddr = vma->vm_start + offset;
> - vaddr -= vma->vm_pgoff << PAGE_SHIFT;
> + vaddr -= (loff_t)vma->vm_pgoff << PAGE_SHIFT;
> 
>   return vaddr;
>  }
> @@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned long 
> bp_vaddr, int *is_swbp)
> 
>   inode = vma->vm_file->f_mapping->host;
>   offset = bp_vaddr - vma->vm_start;
> - offset += (vma->vm_pgoff << PAGE_SHIFT);
> + offset += (loff_t)vma->vm_pgoff << PAGE_SHIFT;
>   uprobe = find_uprobe(inode, offset);
>   }
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] uprobes: fix overflow in vma_address/find_active_uprobe

2012-07-11 Thread Srikar Dronamraju
* Oleg Nesterov o...@redhat.com [2012-07-09 12:54:45]:

 On 07/08, Joe Perches wrote:
 
  On Sun, 2012-07-08 at 22:30 +0200, Oleg Nesterov wrote:
   @@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned 
   long bp_vaddr, int *is_swbp)
  
 inode = vma-vm_file-f_mapping-host;
 offset = bp_vaddr - vma-vm_start;
   - offset += (vma-vm_pgoff  PAGE_SHIFT);
   + offset += ((loff_t)vma-vm_pgoff  PAGE_SHIFT);
 
  It's be nice to take remove the unnecessary parenthesis
  and make it consistent with the vaddr use above it too.
 
 OK, please find v2 below.
 
 --
 Subject: [PATCH] uprobes: fix overflow in vma_address/find_active_uprobe
 
 vma-vm_pgoff is unsigned long, it should be promoted to loff_t
 before the multiplication to avoid the overflow.
 
 Signed-off-by: Oleg Nesterov o...@redhat.com

Acked-by: Srikar Dronamraju sri...@linux.vnet.ibm.com

 ---
  kernel/events/uprobes.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
 index 47c4e24..6194edb 100644
 --- a/kernel/events/uprobes.c
 +++ b/kernel/events/uprobes.c
 @@ -117,7 +117,7 @@ static loff_t vma_address(struct vm_area_struct *vma, 
 loff_t offset)
   loff_t vaddr;
 
   vaddr = vma-vm_start + offset;
 - vaddr -= vma-vm_pgoff  PAGE_SHIFT;
 + vaddr -= (loff_t)vma-vm_pgoff  PAGE_SHIFT;
 
   return vaddr;
  }
 @@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned long 
 bp_vaddr, int *is_swbp)
 
   inode = vma-vm_file-f_mapping-host;
   offset = bp_vaddr - vma-vm_start;
 - offset += (vma-vm_pgoff  PAGE_SHIFT);
 + offset += (loff_t)vma-vm_pgoff  PAGE_SHIFT;
   uprobe = find_uprobe(inode, offset);
   }
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] uprobes: fix overflow in vma_address/find_active_uprobe

2012-07-09 Thread Oleg Nesterov
On 07/08, Joe Perches wrote:
>
> On Sun, 2012-07-08 at 22:30 +0200, Oleg Nesterov wrote:
> > @@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned 
> > long bp_vaddr, int *is_swbp)
> >
> > inode = vma->vm_file->f_mapping->host;
> > offset = bp_vaddr - vma->vm_start;
> > -   offset += (vma->vm_pgoff << PAGE_SHIFT);
> > +   offset += ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
>
> It's be nice to take remove the unnecessary parenthesis
> and make it consistent with the vaddr use above it too.

OK, please find v2 below.

--
Subject: [PATCH] uprobes: fix overflow in vma_address/find_active_uprobe

vma->vm_pgoff is "unsigned long", it should be promoted to loff_t
before the multiplication to avoid the overflow.

Signed-off-by: Oleg Nesterov 
---
 kernel/events/uprobes.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 47c4e24..6194edb 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -117,7 +117,7 @@ static loff_t vma_address(struct vm_area_struct *vma, 
loff_t offset)
loff_t vaddr;
 
vaddr = vma->vm_start + offset;
-   vaddr -= vma->vm_pgoff << PAGE_SHIFT;
+   vaddr -= (loff_t)vma->vm_pgoff << PAGE_SHIFT;
 
return vaddr;
 }
@@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned long 
bp_vaddr, int *is_swbp)
 
inode = vma->vm_file->f_mapping->host;
offset = bp_vaddr - vma->vm_start;
-   offset += (vma->vm_pgoff << PAGE_SHIFT);
+   offset += (loff_t)vma->vm_pgoff << PAGE_SHIFT;
uprobe = find_uprobe(inode, offset);
}
 
-- 
1.5.5.1


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] uprobes: fix overflow in vma_address/find_active_uprobe

2012-07-09 Thread Oleg Nesterov
On 07/08, Joe Perches wrote:

 On Sun, 2012-07-08 at 22:30 +0200, Oleg Nesterov wrote:
  @@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned 
  long bp_vaddr, int *is_swbp)
 
  inode = vma-vm_file-f_mapping-host;
  offset = bp_vaddr - vma-vm_start;
  -   offset += (vma-vm_pgoff  PAGE_SHIFT);
  +   offset += ((loff_t)vma-vm_pgoff  PAGE_SHIFT);

 It's be nice to take remove the unnecessary parenthesis
 and make it consistent with the vaddr use above it too.

OK, please find v2 below.

--
Subject: [PATCH] uprobes: fix overflow in vma_address/find_active_uprobe

vma-vm_pgoff is unsigned long, it should be promoted to loff_t
before the multiplication to avoid the overflow.

Signed-off-by: Oleg Nesterov o...@redhat.com
---
 kernel/events/uprobes.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 47c4e24..6194edb 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -117,7 +117,7 @@ static loff_t vma_address(struct vm_area_struct *vma, 
loff_t offset)
loff_t vaddr;
 
vaddr = vma-vm_start + offset;
-   vaddr -= vma-vm_pgoff  PAGE_SHIFT;
+   vaddr -= (loff_t)vma-vm_pgoff  PAGE_SHIFT;
 
return vaddr;
 }
@@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned long 
bp_vaddr, int *is_swbp)
 
inode = vma-vm_file-f_mapping-host;
offset = bp_vaddr - vma-vm_start;
-   offset += (vma-vm_pgoff  PAGE_SHIFT);
+   offset += (loff_t)vma-vm_pgoff  PAGE_SHIFT;
uprobe = find_uprobe(inode, offset);
}
 
-- 
1.5.5.1


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] uprobes: fix overflow in vma_address/find_active_uprobe

2012-07-08 Thread Joe Perches
On Sun, 2012-07-08 at 22:30 +0200, Oleg Nesterov wrote:
> vma->vm_pgoff is "unsigned long", it should be promoted to loff_t
> before the multiplication to avoid the overflow.
[]
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
[]
> @@ -117,7 +117,7 @@ static loff_t vma_address(struct vm_area_struct *vma, 
> loff_t offset)
>   loff_t vaddr;
>  
>   vaddr = vma->vm_start + offset;
> - vaddr -= vma->vm_pgoff << PAGE_SHIFT;
> + vaddr -= (loff_t)vma->vm_pgoff << PAGE_SHIFT;
>  
>   return vaddr;
>  }
> @@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned long 
> bp_vaddr, int *is_swbp)
>  
>   inode = vma->vm_file->f_mapping->host;
>   offset = bp_vaddr - vma->vm_start;
> - offset += (vma->vm_pgoff << PAGE_SHIFT);
> + offset += ((loff_t)vma->vm_pgoff << PAGE_SHIFT);

It's be nice to take remove the unnecessary parenthesis
and make it consistent with the vaddr use above it too.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 3/5] uprobes: fix overflow in vma_address/find_active_uprobe

2012-07-08 Thread Oleg Nesterov
vma->vm_pgoff is "unsigned long", it should be promoted to loff_t
before the multiplication to avoid the overflow.

Signed-off-by: Oleg Nesterov 
---
 kernel/events/uprobes.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 47c4e24..3826b3b 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -117,7 +117,7 @@ static loff_t vma_address(struct vm_area_struct *vma, 
loff_t offset)
loff_t vaddr;
 
vaddr = vma->vm_start + offset;
-   vaddr -= vma->vm_pgoff << PAGE_SHIFT;
+   vaddr -= (loff_t)vma->vm_pgoff << PAGE_SHIFT;
 
return vaddr;
 }
@@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned long 
bp_vaddr, int *is_swbp)
 
inode = vma->vm_file->f_mapping->host;
offset = bp_vaddr - vma->vm_start;
-   offset += (vma->vm_pgoff << PAGE_SHIFT);
+   offset += ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
uprobe = find_uprobe(inode, offset);
}
 
-- 
1.5.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH 3/5] uprobes: fix overflow in vma_address/find_active_uprobe

2012-07-08 Thread Oleg Nesterov
vma-vm_pgoff is unsigned long, it should be promoted to loff_t
before the multiplication to avoid the overflow.

Signed-off-by: Oleg Nesterov o...@redhat.com
---
 kernel/events/uprobes.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 47c4e24..3826b3b 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -117,7 +117,7 @@ static loff_t vma_address(struct vm_area_struct *vma, 
loff_t offset)
loff_t vaddr;
 
vaddr = vma-vm_start + offset;
-   vaddr -= vma-vm_pgoff  PAGE_SHIFT;
+   vaddr -= (loff_t)vma-vm_pgoff  PAGE_SHIFT;
 
return vaddr;
 }
@@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned long 
bp_vaddr, int *is_swbp)
 
inode = vma-vm_file-f_mapping-host;
offset = bp_vaddr - vma-vm_start;
-   offset += (vma-vm_pgoff  PAGE_SHIFT);
+   offset += ((loff_t)vma-vm_pgoff  PAGE_SHIFT);
uprobe = find_uprobe(inode, offset);
}
 
-- 
1.5.5.1

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 3/5] uprobes: fix overflow in vma_address/find_active_uprobe

2012-07-08 Thread Joe Perches
On Sun, 2012-07-08 at 22:30 +0200, Oleg Nesterov wrote:
 vma-vm_pgoff is unsigned long, it should be promoted to loff_t
 before the multiplication to avoid the overflow.
[]
 diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
[]
 @@ -117,7 +117,7 @@ static loff_t vma_address(struct vm_area_struct *vma, 
 loff_t offset)
   loff_t vaddr;
  
   vaddr = vma-vm_start + offset;
 - vaddr -= vma-vm_pgoff  PAGE_SHIFT;
 + vaddr -= (loff_t)vma-vm_pgoff  PAGE_SHIFT;
  
   return vaddr;
  }
 @@ -1450,7 +1450,7 @@ static struct uprobe *find_active_uprobe(unsigned long 
 bp_vaddr, int *is_swbp)
  
   inode = vma-vm_file-f_mapping-host;
   offset = bp_vaddr - vma-vm_start;
 - offset += (vma-vm_pgoff  PAGE_SHIFT);
 + offset += ((loff_t)vma-vm_pgoff  PAGE_SHIFT);

It's be nice to take remove the unnecessary parenthesis
and make it consistent with the vaddr use above it too.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/