[Ocfs2-devel] [PATCH] ocfs2: fix shift left overflow

[Ocfs2-devel] [PATCH] ocfs2: fix shift left overflow

[Joseph Qi]

cluster pos is defined as u32, when calculate corresponding sector it
should be converted to u64 first, otherwise it may overflow.

Signed-off-by: Joseph Qi <joseph.qi@huawei.com>
---
 fs/ocfs2/aops.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index b57f0c7..b36dcad 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -688,7 +688,7 @@ static int ocfs2_direct_IO_zero_extend(struct ocfs2_super *osb,

 	if (p_cpos && !(ext_flags & OCFS2_EXT_UNWRITTEN)) {
 		u64 s = i_size_read(inode);
-		sector_t sector = (p_cpos << (osb->s_clustersize_bits - 9)) +
+		sector_t sector = ((u64)p_cpos << (osb->s_clustersize_bits - 9)) +
 			(do_div(s, osb->s_clustersize) >> 9);

 		ret = blkdev_issue_zeroout(osb->sb->s_bdev, sector,
@@ -916,7 +916,7 @@ static ssize_t ocfs2_direct_IO_write(struct kiocb *iocb,
 		BUG_ON(!p_cpos || (ext_flags & OCFS2_EXT_UNWRITTEN));

 		ret = blkdev_issue_zeroout(osb->sb->s_bdev,
-				p_cpos << (osb->s_clustersize_bits - 9),
+				(u64)p_cpos << (osb->s_clustersize_bits - 9),
 				zero_len_head >> 9, GFP_NOFS, false);
 		if (ret < 0)
 			mlog_errno(ret);
-- 
1.8.4.3

[Ocfs2-devel] [PATCH] ocfs2: fix shift left overflow

[Andrew Morton]

On Mon, 27 Jul 2015 11:39:16 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:

> cluster pos is defined as u32, when calculate corresponding sector it
> should be converted to u64 first, otherwise it may overflow.

What are the runtime effects of this change?

Please always include this info when fixing bugs so that others can
work out which kernel version(s) should be patched.

[Ocfs2-devel] [PATCH] ocfs2: fix shift left overflow

[Joseph Qi]

On 2015/7/29 4:51, Andrew Morton wrote:
> On Mon, 27 Jul 2015 11:39:16 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:
> 
>> cluster pos is defined as u32, when calculate corresponding sector it
>> should be converted to u64 first, otherwise it may overflow.
> 
> What are the runtime effects of this change?
> 
The issue is when using large volume, for example, 9T volume with 2T already
used, frequently create small files with O_DIRECT and the IO is not cluster
aligned, it may clear sectors in the wrong place.

> Please always include this info when fixing bugs so that others can
> work out which kernel version(s) should be patched.
> 
Sorry, I'll bear it in mind.
> 
> 

[Ocfs2-devel] [PATCH] ocfs2: fix shift left overflow

[Andrew Morton]

On Wed, 29 Jul 2015 08:30:59 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:

> On 2015/7/29 4:51, Andrew Morton wrote:
> > On Mon, 27 Jul 2015 11:39:16 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:
> > 
> >> cluster pos is defined as u32, when calculate corresponding sector it
> >> should be converted to u64 first, otherwise it may overflow.
> > 
> > What are the runtime effects of this change?
> > 
> The issue is when using large volume, for example, 9T volume with 2T already
> used, frequently create small files with O_DIRECT and the IO is not cluster
> aligned, it may clear sectors in the wrong place.

You mean it corrupts the filesystem?

So this wants to be merged asap and backported as far as possible?

[Ocfs2-devel] [PATCH] ocfs2: fix shift left overflow

[Joseph Qi]

On 2015/7/29 8:40, Andrew Morton wrote:
> On Wed, 29 Jul 2015 08:30:59 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:
> 
>> On 2015/7/29 4:51, Andrew Morton wrote:
>>> On Mon, 27 Jul 2015 11:39:16 +0800 Joseph Qi <joseph.qi@huawei.com> wrote:
>>>
>>>> cluster pos is defined as u32, when calculate corresponding sector it
>>>> should be converted to u64 first, otherwise it may overflow.
>>>
>>> What are the runtime effects of this change?
>>>
>> The issue is when using large volume, for example, 9T volume with 2T already
>> used, frequently create small files with O_DIRECT and the IO is not cluster
>> aligned, it may clear sectors in the wrong place.
> 
> You mean it corrupts the filesystem?
> 
> So this wants to be merged asap and backported as far as possible?
> 
Yes, it only affects kernel 4.0+.

>