[Ocfs2-devel] [PATCH 2/2] ocfs2: fix check condition of max inline data

[Tiger Yang]

Hi, Tao,

yes. ocfs2_max_inline_data_with_xattr() in read_inline_data() is more 
critical, I think ocfs2_max_inline_data() is also safe before.

I deliberately left ocfs2_max_inline_data() because in some case, like 
in mknod, di->i_dyn_features have not been set with 
OCFS2_INLINE_XATTR_FL or we couldn't get correct di in somewhere.

thanks,
tiger

Tao Ma wrote:
> Hi tiger,
>     I just searched the code in ocfs2. There are other cases(in 
> ocfs2_read_inline_data) which may have the problem also.
> 
> So I just think that since you have already added inlin-data-xattr 
> support in ocfs2, why not remove the function ocfs2_max_inline_data and 
> use ocfs2_max_inline_data_with_xattr instead in all the cases?  Make it 
> more intelligent?
> 
> I am just worried that this function may be used wrongly afterwards and 
> cause future bugs like this.
> 
> Regards,
> Tao
> 
> Tiger Yang wrote:
>> We should consider the inline xattr when
>> we try to write inline data.
>>
>> Signed-off-by: Tiger Yang <tiger.yang at oracle.com>
>> ---
>>  fs/ocfs2/aops.c |    5 ++++-
>>  1 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
>> index a067a6c..dc6d734 100644
>> --- a/fs/ocfs2/aops.c
>> +++ b/fs/ocfs2/aops.c
>> @@ -1555,6 +1555,7 @@ static int ocfs2_try_to_write_inline_data(struct 
>> address_space *mapping,
>>      int ret, written = 0;
>>      loff_t end = pos + len;
>>      struct ocfs2_inode_info *oi = OCFS2_I(inode);
>> +    struct ocfs2_dinode *di = NULL;
>>  
>>      mlog(0, "Inode %llu, write of %u bytes at off %llu. features: 
>> 0x%x\n",
>>           (unsigned long long)oi->ip_blkno, len, (unsigned long long)pos,
>> @@ -1587,7 +1588,9 @@ static int ocfs2_try_to_write_inline_data(struct 
>> address_space *mapping,
>>      /*
>>       * Check whether the write can fit.
>>       */
>> -    if (mmap_page || end > ocfs2_max_inline_data(inode->i_sb))
>> +    di = (struct ocfs2_dinode *)wc->w_di_bh->b_data;
>> +    if (mmap_page ||
>> +        end > ocfs2_max_inline_data_with_xattr(inode->i_sb, di))
>>          return 0;
>>  
>>  do_inline_write: