Re: [PATCH] fs/ext3: use kzalloc instead of kmalloc

[Chen Gang <gang.chen@asianux.com>]

于 2012年12月26日 02:48, Theodore Ts'o 写道:
> On Mon, Dec 24, 2012 at 01:28:53PM +0800, Chen Gang wrote:
>>
>>   better to use kzalloc instead of kmalloc.
>>     if acl_e->e_tag is neither ACL_USER, nor ACL_GROUP.
>>     entry->e_id will not be initialized
>>
>>   we can not say it is a bug, but suggest to initialize it, too.
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> 
> This shouldn't be a problem, since if e_tag is not ACL_USER nor
> ACL_GROUP, the on-disk encoding does not include e_id at all.
> 

  ok, thanks.  it is my fault.

  :-)


> That being said, it looks to me there's another bug hiding here.  The
> size of the extended attribute is calculated by ext3_acl_size(), and
> it looks totally wrong.  For one thing, it caluclates the size of the
> xattr assuming all of the stored encoding ext3_acl_entry_short ---
> which would not be the case if we had a acl entry of type ACL_USER or
> ACL_GROUP.
> 
> But if that were the case, it would mean that we would not be storing
> the full acl entry on disk, which would be a pretty horrible and
> obvious breakage.
> 

  checking the ext3_acl_size, it does not like what you said above.
  but we can say, the design for ext3_acl_size is really not quit well.
    (maybe can cause issue).

 26 static inline size_t ext3_acl_size(int count)
 27 {
 28         if (count <= 4) {
 29                 return sizeof(ext3_acl_header) +
 30                        count * sizeof(ext3_acl_entry_short);
 31         } else {
 32                 return sizeof(ext3_acl_header) +
 33                        4 * sizeof(ext3_acl_entry_short) +
 34                        (count - 4) * sizeof(ext3_acl_entry);
 35         }
 36 }


> I haven't had time to check this yet, but I wanted to flag this so
> hopefully someone else should double check this.....  It would seem to
> me that the better thing to do would be to calculate the size as part
> of the for loop in ext3_acl_to_disk(), and drop ext3_acl_size() from
> acl.h.  (This code exists in ext4 as well, so if we have a bug in
> ext3, we would have a similar bug in ext4.)
> 

  at least, for my idea:
    your design for ext3_acl_size is a standard one.
    it is necessary to use your design instead of original design.

  if you also like me to provide the relative patch, please tell me.

  thanks.


gchen.

> 
> 						- Ted
> 
> 



-- 
Chen Gang

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