ocfs2: Fix memory overflow in cow_by_page.
PATCH 1/1 ocfs2: Fix memory overflow in cow_by_page.
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.
--- fs/ocfs2/refcounttree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
fs/ocfs2/refcounttree.c +1 -1
@@ -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;
Signature · 1 line
1.6.3.3
PATCH 1/1 ocfs2: Fix memory overflow in cow_by_page.
ack
Quoted 25 lines · Tao Ma
In
ocfs2_duplicate_clusters_by_page, we calculatemap_endby shiftingpage_index. But actually in case we meet with a largeoffset(say in a i686 box, poff_t is only 32 bits and page_index=2056240), we will overflow. So change it by addingPAGE_CACHE_SIZEto offset.Signed-off-by Tao Ma <[email protected]>--- fs/ocfs2/refcounttree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)fs/ocfs2/refcounttree.c +1
-1@@ -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;
PATCH 1/1 ocfs2: Fix memory overflow in cow_by_page.
In reply to Tao MaQuoted 16 lines · Tao Ma
In
ocfs2_duplicate_clusters_by_page, we calculatemap_endby shiftingpage_index. But actually in case we meet with a largeoffset(say in a i686 box, poff_t is only 32 bits and page_index=2056240), we will overflow. So change it by addingPAGE_CACHE_SIZEto offset.Signed-off-by Tao Ma <[email protected]>--- fs/ocfs2/refcounttree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)fs/ocfs2/refcounttree.c +0
-0
fs/ocfs2/refcounttree.c static int ocfs2_duplicate_clusters_by_page(handle_t *handle, · line 2948
Quoted 5 lines
fs/ocfs2/refcounttree.c +1
-1@@ -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
Signature · 8 lines
"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
PATCH 1/1 ocfs2: Fix memory overflow in cow_by_page.
Quoted 37 lines · Joel Becker
Quoted 16 lines · Tao Ma
In
ocfs2_duplicate_clusters_by_page, we calculatemap_endby shiftingpage_index. But actually in case we meet with a largeoffset(say in a i686 box, poff_t is only 32 bits and page_index=2056240), we will overflow. So change it by addingPAGE_CACHE_SIZEto offset.Signed-off-by Tao Ma <[email protected]>--- fs/ocfs2/refcounttree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)fs/ocfs2/refcounttree.c +0
-0fs/ocfs2/refcounttree.c static int ocfs2_duplicate_clusters_by_page(handle_t *handle, · line 2948
Quoted 5 lines
fs/ocfs2/refcounttree.c +1
-1@@ -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_endis
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, settingmap_endto 0?
Why aren't we computingmap_endlike we compute end, as aloff_t
value?page_index = offset >> PAGE_CACHE_SHIFT;fs/ocfs2/refcounttree.c +1
-1- 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;
Quoted 1 line
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
PATCH 1/1 ocfs2: Fix memory overflow in cow_by_page.
In reply to Joel BeckerIn 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.
--- fs/ocfs2/refcounttree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
fs/ocfs2/refcounttree.c +1 -1
@@ -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;
Signature · 1 line
1.6.3.3