* [Ocfs2-devel] [PATCH] ocfs2: Fix memory overflow in cow_by_page.
@ 2010-01-21 6:59 Tao Ma
2010-01-22 2:54 ` Sunil Mushran
2010-01-26 3:19 ` Joel Becker
0 siblings, 2 replies; 5+ messages in thread
From: Tao Ma @ 2010-01-21 6:59 UTC (permalink / raw)
To: ocfs2-devel
In ocfs2_duplicate_clusters_by_page, we calculate map_end
by shifting page_index. But actually in case we meet with
a large offset(say in a i686 box, poff_t is only 32 bits
and page_index=2056240), we will overflow. So change it
by adding PAGE_CACHE_SIZE to offset.
Signed-off-by: Tao Ma <tao.ma@oracle.com>
---
fs/ocfs2/refcounttree.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
index 74db2be..6db863d 100644
--- a/fs/ocfs2/refcounttree.c
+++ b/fs/ocfs2/refcounttree.c
@@ -2945,7 +2945,7 @@ static int ocfs2_duplicate_clusters_by_page(handle_t *handle,
while (offset < end) {
page_index = offset >> PAGE_CACHE_SHIFT;
- map_end = (page_index + 1) << PAGE_CACHE_SHIFT;
+ map_end = offset + PAGE_CACHE_SIZE;
if (map_end > end)
map_end = end;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: Fix memory overflow in cow_by_page.
2010-01-21 6:59 [Ocfs2-devel] [PATCH] ocfs2: Fix memory overflow in cow_by_page Tao Ma
@ 2010-01-22 2:54 ` Sunil Mushran
2010-01-26 3:19 ` Joel Becker
1 sibling, 0 replies; 5+ messages in thread
From: Sunil Mushran @ 2010-01-22 2:54 UTC (permalink / raw)
To: ocfs2-devel
ack
Tao Ma wrote:
> In ocfs2_duplicate_clusters_by_page, we calculate map_end
> by shifting page_index. But actually in case we meet with
> a large offset(say in a i686 box, poff_t is only 32 bits
> and page_index=2056240), we will overflow. So change it
> by adding PAGE_CACHE_SIZE to offset.
>
> Signed-off-by: Tao Ma <tao.ma@oracle.com>
> ---
> fs/ocfs2/refcounttree.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
> index 74db2be..6db863d 100644
> --- a/fs/ocfs2/refcounttree.c
> +++ b/fs/ocfs2/refcounttree.c
> @@ -2945,7 +2945,7 @@ static int ocfs2_duplicate_clusters_by_page(handle_t *handle,
>
> while (offset < end) {
> page_index = offset >> PAGE_CACHE_SHIFT;
> - map_end = (page_index + 1) << PAGE_CACHE_SHIFT;
> + map_end = offset + PAGE_CACHE_SIZE;
> if (map_end > end)
> map_end = end;
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: Fix memory overflow in cow_by_page.
2010-01-21 6:59 [Ocfs2-devel] [PATCH] ocfs2: Fix memory overflow in cow_by_page Tao Ma
2010-01-22 2:54 ` Sunil Mushran
@ 2010-01-26 3:19 ` Joel Becker
2010-01-26 3:30 ` Tao Ma
2010-01-26 4:24 ` Tao Ma
1 sibling, 2 replies; 5+ messages in thread
From: Joel Becker @ 2010-01-26 3:19 UTC (permalink / raw)
To: ocfs2-devel
On Thu, Jan 21, 2010 at 02:59:26PM +0800, Tao Ma wrote:
> In ocfs2_duplicate_clusters_by_page, we calculate map_end
> by shifting page_index. But actually in case we meet with
> a large offset(say in a i686 box, poff_t is only 32 bits
> and page_index=2056240), we will overflow. So change it
> by adding PAGE_CACHE_SIZE to offset.
>
> Signed-off-by: Tao Ma <tao.ma@oracle.com>
> ---
> fs/ocfs2/refcounttree.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
> index 74db2be..6db863d 100644
> --- a/fs/ocfs2/refcounttree.c
> +++ b/fs/ocfs2/refcounttree.c
> @@ -2945,7 +2945,7 @@ static int ocfs2_duplicate_clusters_by_page(handle_t *handle,
>
> while (offset < end) {
> page_index = offset >> PAGE_CACHE_SHIFT;
> - map_end = (page_index + 1) << PAGE_CACHE_SHIFT;
> + map_end = offset + PAGE_CACHE_SIZE;
First, we can't be computing by offset, because map_end is
supposed to be page bounded, right? Also, what if we have an offset
that is the last page possible? Won't that wrap as well, setting
map_end to 0?
Why aren't we computing map_end like we compute end, as a loff_t
value?
page_index = offset >> PAGE_CACHE_SHIFT;
- map_end = (page_index + 1) << PAGE_CACHE_SHIFT;
+ map_end = ((loff_t)page_index + 1) << PAGE_CACHE_SHIFT;
if (map_end > end)
map_end = end;
The map_end>end check will catch anything too big.
Joel
--
"You don't make the poor richer by making the rich poorer."
- Sir Winston Churchill
Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127
^ permalink raw reply [flat|nested] 5+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: Fix memory overflow in cow_by_page.
2010-01-26 3:19 ` Joel Becker
@ 2010-01-26 3:30 ` Tao Ma
2010-01-26 4:24 ` Tao Ma
1 sibling, 0 replies; 5+ messages in thread
From: Tao Ma @ 2010-01-26 3:30 UTC (permalink / raw)
To: ocfs2-devel
Joel Becker wrote:
> On Thu, Jan 21, 2010 at 02:59:26PM +0800, Tao Ma wrote:
>> In ocfs2_duplicate_clusters_by_page, we calculate map_end
>> by shifting page_index. But actually in case we meet with
>> a large offset(say in a i686 box, poff_t is only 32 bits
>> and page_index=2056240), we will overflow. So change it
>> by adding PAGE_CACHE_SIZE to offset.
>>
>> Signed-off-by: Tao Ma <tao.ma@oracle.com>
>> ---
>> fs/ocfs2/refcounttree.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
>> index 74db2be..6db863d 100644
>> --- a/fs/ocfs2/refcounttree.c
>> +++ b/fs/ocfs2/refcounttree.c
>> @@ -2945,7 +2945,7 @@ static int ocfs2_duplicate_clusters_by_page(handle_t *handle,
>>
>> while (offset < end) {
>> page_index = offset >> PAGE_CACHE_SHIFT;
>> - map_end = (page_index + 1) << PAGE_CACHE_SHIFT;
>> + map_end = offset + PAGE_CACHE_SIZE;
>
> First, we can't be computing by offset, because map_end is
> supposed to be page bounded, right? Also, what if we have an offset
> that is the last page possible? Won't that wrap as well, setting
> map_end to 0?
> Why aren't we computing map_end like we compute end, as a loff_t
> value?
>
> page_index = offset >> PAGE_CACHE_SHIFT;
> - map_end = (page_index + 1) << PAGE_CACHE_SHIFT;
> + map_end = ((loff_t)page_index + 1) << PAGE_CACHE_SHIFT;
>
> if (map_end > end)
> map_end = end;
>
> The map_end>end check will catch anything too big.
oh, you are right.
I only considered the problem of overflow, but forget the original
usage. Sorry.
Regards,
Tao
^ permalink raw reply [flat|nested] 5+ messages in thread* [Ocfs2-devel] [PATCH] ocfs2: Fix memory overflow in cow_by_page.
2010-01-26 3:19 ` Joel Becker
2010-01-26 3:30 ` Tao Ma
@ 2010-01-26 4:24 ` Tao Ma
1 sibling, 0 replies; 5+ messages in thread
From: Tao Ma @ 2010-01-26 4:24 UTC (permalink / raw)
To: ocfs2-devel
In ocfs2_duplicate_clusters_by_page, we calculate map_end
by shifting page_index. But actually in case we meet with
a large offset(say in a i686 box, poff_t is only 32 bits
and page_index=2056240), we will overflow. So change the
type of page_index to loff_t.
Signed-off-by: Tao Ma <tao.ma@oracle.com>
---
fs/ocfs2/refcounttree.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c
index 74db2be..7687bf1 100644
--- a/fs/ocfs2/refcounttree.c
+++ b/fs/ocfs2/refcounttree.c
@@ -2945,7 +2945,7 @@ static int ocfs2_duplicate_clusters_by_page(handle_t *handle,
while (offset < end) {
page_index = offset >> PAGE_CACHE_SHIFT;
- map_end = (page_index + 1) << PAGE_CACHE_SHIFT;
+ map_end = ((loff_t)page_index + 1) << PAGE_CACHE_SHIFT;
if (map_end > end)
map_end = end;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-01-26 4:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-21 6:59 [Ocfs2-devel] [PATCH] ocfs2: Fix memory overflow in cow_by_page Tao Ma
2010-01-22 2:54 ` Sunil Mushran
2010-01-26 3:19 ` Joel Becker
2010-01-26 3:30 ` Tao Ma
2010-01-26 4:24 ` Tao Ma
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.