linux-ext4 - Re: BUG: warning at fs/ext3/inode.c:1016/ext3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <44FDF16D.8040505@us.ibm.com>
Date:	Tue, 05 Sep 2006 14:51:41 -0700
From:	Mingming Cao <cmm@...ibm.com>
To:	Dave Kleikamp <shaggy@...tin.ibm.com>
CC:	Badari Pulavarty <pbadari@...ibm.com>,
	Badari Pulavarty <pbadari@...il.com>,
	Will Simoneau <simoneau@....uri.edu>,
	lkml <linux-kernel@...r.kernel.org>,
	ext4 <linux-ext4@...r.kernel.org>
Subject: Re: BUG: warning at fs/ext3/inode.c:1016/ext3_getblk()

Dave Kleikamp wrote:
> On Tue, 2006-09-05 at 13:14 -0700, Badari Pulavarty wrote:
> 
>>Dave Kleikamp wrote:
> 
> 
>>>I'm having a hard time figuring out exactly what ext3_get_blocks_handle
>>>is trying to return, but it looks to me like if it is allocating one
>>>data block, and needs to allocate an indirect block as well, then it
>>>will return 2 rather than 1.  Is this expected, or am I just confused?
>>>
>>>  
>>
>>It would return "1" in that case.. (data block)
>>
>> > 0 means get_block() suceeded and indicates the number of blocks mapped
>>= 0 lookup failed
>>< 0 mean error case
> 
> 
> Okay, I got confused looking through the code.  I still don't see how
> ext3_get_blocks_handle() should be returning a number greater than
> maxblocks.
> 

yes ext3_get_blocks_handle() will return the number of data blocks 
allocated(not including the indirect/double indirecto blocks), and that 
number should not than maxblocks. In this case, it should return 1 instead.

The ext3_get_blocks_handle() behavior was changed when multiple blocks 
map/allocation was added to ext3 via this function. Previously, the 
behavior of ext3_get_blokc_handle() returns 0 for success case, and 
returns non-zero(nagive) for error case. While with new behavior, the 
success case is the thre returned value should > 0.

How many blocks is being mapped in this case? > 1? or 0? If it failed to 
map the block (ext3_get_blocks_handle() returns 0), ext3_getblk() will 
also WARN_ON().

> 
>>>>I did search for callers of ext3_get_blocks_handle() and found that
>>>>ext3_readdir() seems to do wrong thing all the time with error check :(
>>>>Need to take a closer look..
>>>>
>>>>	err = ext3_get_blocks_handle(NULL, inode, blk, 1,
>>>>                                                &map_bh, 0, 0);
>>>>        if (err > 0) {  <<<< BAD
>>>>                  page_cache_readahead(sb->s_bdev->bd_inode->i_mapping,
>>>>                                &filp->f_ra,
>>>>                                filp,
>>>>                                map_bh.b_blocknr >>
>>>>                                (PAGE_CACHE_SHIFT - inode->i_blkbits),
>>>>                                1);
>>>>                        bh = ext3_bread(NULL, inode, blk, 0, &err);
>>>>       }
>>>>    
>>>
>>>Bad to do what it's doing, or bad to call name the variable "err"?
>>>I think if it looked like this:
>>>
>>>	count = ext3_get_blocks_handle(NULL, inode, blk, 1,
>>>                                                &map_bh, 0, 0);
>>>        if (count > 0) { 
>>>
>>>it would be a lot less confusing.
>>>  
>>
>>I am sorry !! it is doing the right thing :(
> 
> 
> Not your fault.  The variable is very badly named.

-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html