[Ocfs2-tools-devel] [PATCH 03/44] libocfs2: Add ocfs2_read_refcount_block().

[Tao Ma]

Joel Becker wrote:
> On Thu, Dec 31, 2009 at 10:27:30PM +0800, Tao Ma wrote:
>>>>> +	if (ret == 0 && rb->rf_list.l_next_free_rec > rb->rf_list.l_count)
>>>>> +		ret = OCFS2_ET_CORRUPT_EXTENT_BLOCK;
>> oh, actually here is a bug. rf_list is only valid in case it is a
>> tree, not a leaf refcount block.
>> We can't test this until we trust the flag.
>> So my opinion here is that we remove this check and rename
>> ocfs2_read_refcount_block_no_check to ocfs2_read_refcount_block.
> 
> 	While checking each field for sanity isn't really the
> mission of non-fsck programs anyway.  If the value is bad, though, how
> do we keep folks from walking off the end...ok, the swapping code uses
> ocfs2_swap_barrier().
yeah, so your sap_barrier can resolve swapping issues, but it can't 
prevent user from tramping other memory by using next_free_rec by himself.
> 	I can see why fsck doesn't want the check.  It wants to be able
> to get the block and fix the field.  Good to remember.  But do we want
> other programs to walk off the end?  Same goes for inline data size when
> reading an inode, and other things.  Heck, we don't even check
> next_free_rec when reading inodes.
yes, I just searched the libocfs2, and it seems that only 
read_extent_block has *_nocheck version. So we need a 
ocf2_read_inode_nocheck and let ocfs2_read_inode call it and then check 
next_free_rec. The same goes for xattr tree and value etc.

Regards,
Tao