ocfs2: fix shift left overflow
diff mbox

Message ID 55B5A7E4.6050600@huawei.com
State New
Headers show

Commit Message

Joseph Qi July 27, 2015, 3:39 a.m. UTC
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(-)

Comments

Andrew Morton July 28, 2015, 8:51 p.m. UTC | #1
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.
Joseph Qi July 29, 2015, 12:30 a.m. UTC | #2
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.
> 
>
Andrew Morton July 29, 2015, 12:40 a.m. UTC | #3
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?
Joseph Qi July 29, 2015, 12:59 a.m. UTC | #4
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+.

>

Patch
diff mbox

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);