On Thu, Jan 17, 2008 at 02:09:41PM +0100, Valerie Clement wrote:
> Aneesh Kumar K.V wrote:
>> On Thu, Jan 17, 2008 at 10:43:40AM +0100, Valerie Clement wrote:
>>> Aneesh Kumar K.V wrote:
>>>> What about this ? I guess we will overflow start = start << bsbits;
>>>>
>>> Hi Aneesh,
>>> your patch below doesn't fix the issue, because as start_off is also
>>> loff_t, start_off = ac->ac_o_ex.fe_logical << bsbits also overflows.
>>>
>>
>> loff_t is 64 bits.
>>
>> typedef __kernel_loff_t loff_t;
>> typedef long long __kernel_loff_t;
>> typedef __u32 ext4_lblk_t;
>> typedef unsigned long long ext4_fsblk_t
>>
>> start_off = ac->ac_o_ex.fe_logical << bsbits;
>>
>> In the above line what we are storing in start_off is the offset in bytes.So it makes
>> sense to use the type loff_t. It is neither logical block nor physical block.
>
> Oh yes, sorry, you're right. I read too quickly.
>
> In fact, it's missing a cast :
> start_off = (loff_t) ac->ac_o_ex.fe_logical << bsbits;
>
> With that change, the test is ok.
Updated patch below.
-aneesh
ext4: Fix overflow in ext4_mb_normalize_request
From: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
kernel BUG at fs/ext4/mballoc.c:3148!
The BUG_ON is:
BUG_ON(size <= 0 || size >= EXT4_BLOCKS_PER_GROUP(ac->ac_sb));
where the value of "size" is 4293920768.
This is due to the overflow of the variable "start" in the
ext4_mb_normalize_request() function.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@xxxxxxxxxxxxxxxxxx>
---
fs/ext4/mballoc.c | 24 +++++++++++-------------
1 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index d8cd81e..d16083c 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -2998,7 +2998,7 @@ static void ext4_mb_normalize_request(struct ext4_allocation_context *ac,
int bsbits, max;
ext4_lblk_t end;
struct list_head *cur;
- loff_t size, orig_size;
+ loff_t size, orig_size, start_off;
ext4_lblk_t start, orig_start;
struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
@@ -3039,7 +3039,7 @@ static void ext4_mb_normalize_request(struct ext4_allocation_context *ac,
/* first, try to predict filesize */
/* XXX: should this table be tunable? */
- start = 0;
+ start_off = 0;
if (size <= 16 * 1024) {
size = 16 * 1024;
} else if (size <= 32 * 1024) {
@@ -3055,26 +3055,24 @@ static void ext4_mb_normalize_request(struct ext4_allocation_context *ac,
} else if (size <= 1024 * 1024) {
size = 1024 * 1024;
} else if (NRL_CHECK_SIZE(size, 4 * 1024 * 1024, max, bsbits)) {
- start = ac->ac_o_ex.fe_logical << bsbits;
- start = (start / (1024 * 1024)) * (1024 * 1024);
+ start_off = ((loff_t)ac->ac_o_ex.fe_logical >>
+ (20 - bsbits)) << 20;
size = 1024 * 1024;
} else if (NRL_CHECK_SIZE(size, 8 * 1024 * 1024, max, bsbits)) {
- start = ac->ac_o_ex.fe_logical << bsbits;
- start = (start / (4 * (1024 * 1024))) * 4 * (1024 * 1024);
+ start_off = ((loff_t)ac->ac_o_ex.fe_logical >>
+ (22 - bsbits)) << 22;
size = 4 * 1024 * 1024;
} else if (NRL_CHECK_SIZE(ac->ac_o_ex.fe_len,
(8<<20)>>bsbits, max, bsbits)) {
- start = ac->ac_o_ex.fe_logical;
- start = start << bsbits;
- start = (start / (8 * (1024 * 1024))) * 8 * (1024 * 1024);
+ start_off = ((loff_t)ac->ac_o_ex.fe_logical >>
+ (23 - bsbits)) << 23;
size = 8 * 1024 * 1024;
} else {
- start = ac->ac_o_ex.fe_logical;
- start = start << bsbits;
- size = ac->ac_o_ex.fe_len << bsbits;
+ start_off = (loff_t)ac->ac_o_ex.fe_logical << bsbits;
+ size = ac->ac_o_ex.fe_len << bsbits;
}
orig_size = size = size >> bsbits;
- orig_start = start = start >> bsbits;
+ orig_start = start = start_off >> bsbits;
/* don't cover already allocated blocks in selected range */
if (ar->pleft && start <= ar->lleft) {
